[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 21/25] conf: Introduce sgio for hostdev



On 05/03/2013 02:07 PM, Osier Yang wrote:
> "sgio" is only valid for scsi host device.
> ---
>  docs/formatdomain.html.in                          |  7 ++++-
>  docs/schemas/domaincommon.rng                      |  8 +++++
>  src/conf/domain_conf.c                             | 34 ++++++++++++++++++---
>  src/conf/domain_conf.h                             |  1 +
>  .../qemuxml2argv-hostdev-scsi-sgio.xml             | 35 ++++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  6 files changed, 81 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-sgio.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6437d6d..89d11e7 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2319,7 +2319,12 @@
>          and <code>virNodeDeviceReAttach</code> (or <code>virsh
>          nodedev-reattach</code>) after hot-unplug or stopping the
>          guest. For SCSI device, user is responsible to make sure the device
> -        is not used by host.</dd>
> +        is not used by host.
> +        The optional <code>sgio</code> (<span class="since">since 1.0.6</span>)
> +        attribute indicates whether the kernel will filter unprivileged
> +        SG_IO commands for the disk, valid settings are "filtered" or
> +        "unfiltered". Defaults to "filtered".
> +      </dd>
>        <dt><code>source</code></dt>
>        <dd>The source element describes the device as seen from the host.
>        The USB device can either be addressed by vendor / product id using the
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index ca79e67..0a59c7a 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3179,6 +3179,14 @@
>      <attribute name="type">
>        <value>scsi</value>
>      </attribute>
> +    <optional>
> +      <attribute name="sgio">
> +        <choice>
> +          <value>filtered</value>
> +          <value>unfiltered</value>
> +        </choice>
> +      </attribute>
> +    </optional>
>      <element name="source">
>        <interleave>
>          <ref name="sourceinfoadapter"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9823b9c..d604e5b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3784,6 +3784,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>  {
>      xmlNodePtr sourcenode;
>      char *managed = NULL;
> +    char *sgio = NULL;
>      char *backendStr = NULL;
>      int backend;
>      int ret = -1;
> @@ -3798,6 +3799,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>              def->managed = true;
>      }
>  
> +    sgio = virXMLPropString(node, "sgio");
> +
>      /* @type is passed in from the caller rather than read from the
>       * xml document, because it is specified in different places for
>       * different kinds of defs - it is an attribute of
> @@ -3834,6 +3837,22 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>          goto error;
>      }
>  
> +    if (sgio) {
> +        if (def->source.subsys.type !=
> +            VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("sgio is only supported for scsi host device"));
> +            goto error;
> +        }
> +
> +        if ((def->source.subsys.u.scsi.sgio =
> +             virDomainDeviceSGIOTypeFromString(sgio)) <= 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("unknown sgio mode '%s'"), sgio);
> +            goto error;
> +        }
> +    }
> +
>      switch (def->source.subsys.type) {
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
>          if (virDomainHostdevSubsysPciDefParseXML(sourcenode, def, flags) < 0)
> @@ -3872,6 +3891,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>      ret = 0;
>  error:
>      VIR_FREE(managed);
> +    VIR_FREE(sgio);
>      VIR_FREE(backendStr);
>      return ret;
>  }
> @@ -15464,11 +15484,17 @@ virDomainHostdevDefFormat(virBufferPtr buf,
>  
>      virBufferAsprintf(buf, "    <hostdev mode='%s' type='%s'",
>                        mode, type);
> -    if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> -        virBufferAsprintf(buf, " managed='%s'>\n",
> +    if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> +        virBufferAsprintf(buf, " managed='%s'",
>                            def->managed ? "yes" : "no");
> -    else
> -        virBufferAddLit(buf, ">\n");
> +
> +        if (def->source.subsys.type ==
> +            VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
> +            def->source.subsys.u.scsi.sgio)
> +            virBufferAsprintf(buf, " sgio='%s'",
> +                              virDomainDeviceSGIOTypeToString(def->source.subsys.u.scsi.sgio));
> +    }
> +    virBufferAddLit(buf, ">\n");
>  
>      virBufferAdjustIndent(buf, 6);
>      switch (def->mode) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 59cf6a7..8d67e67 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -423,6 +423,7 @@ struct _virDomainHostdevSubsys {
>              unsigned bus;
>              unsigned target;
>              unsigned unit;
> +            int sgio; /* enum virDomainDeviceSGIO */
>          } scsi;
>      } u;
>  };
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-sgio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-sgio.xml
> new file mode 100644
> index 0000000..21404ee
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-sgio.xml
> @@ -0,0 +1,35 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest2</name>
> +  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='scsi' index='0' model='virtio-scsi'/>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <hostdev mode='subsystem' type='scsi' managed='yes' sgio='unfiltered'>
> +      <source>
> +        <adapter name='scsi_host0'/>
> +        <address bus='0' target='0' unit='0'/>
> +      </source>
> +      <address type='drive' controller='0' bus='0' target='4' unit='8'/>
> +    </hostdev>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index d37a657..04be8dd 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -289,6 +289,7 @@ mymain(void)
>      DO_TEST("hostdev-scsi-virtio-scsi");
>      DO_TEST("hostdev-scsi-readonly");
>      DO_TEST("hostdev-scsi-shareable");
> +    DO_TEST("hostdev-scsi-sgio");

NIT: should this be "scsi-unfiltered-sgio" since the default is filtered
and you're testing the unfiltered mode...  Probably changes the xml file
name too.

It's not a deal breaker...  ACK otherwise.

John

>  
>      virObjectUnref(driver.caps);
>      virObjectUnref(driver.xmlopt);
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]