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.