[libvirt] [PATCH v2.5 03/10] conf: Introduce scsi hostdev
Osier Yang
jyang at redhat.com
Wed Apr 10 08:59:56 UTC 2013
On 09/04/13 10:32, Han Cheng wrote:
> Add scsi hostdev, it looks like:
>
> <hostdev mode='subsystem' type='scsi'>
> <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>
>
> The only parameter in -drive affects scsi-generic is readonly. Introduce
> <readonly/> to <hostdevsubsysscsi>.
>
> Signed-off-by: Han Cheng <hanc.fnst at cn.fujitsu.com>
> ---
> docs/formatdomain.html.in | 38 +++++++--
> docs/schemas/domaincommon.rng | 29 +++++++
> src/conf/domain_audit.c | 10 +++
> src/conf/domain_conf.c | 181 ++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 8 ++
> 5 files changed, 257 insertions(+), 9 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6a6bdbc..f8aff6a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2190,13 +2190,13 @@
>
> <h4><a name="elementsHostDev">Host device assignment</a></h4>
>
> - <h5><a href="elementsHostDevSubsys">USB / PCI devices</a></h5>
> + <h5><a href="elementsHostDevSubsys">USB / PCI / SCSI devices</a></h5>
>
> <p>
> - USB and PCI devices attached to the host can be passed through
> + USB, PCI and SCSI devices attached to the host can be passed through
> to the guest using the <code>hostdev</code> element.
> - <span class="since">since after 0.4.4 for USB and 0.6.0 for PCI
> - (KVM only)</span>:
> + <span class="since">since after 0.4.4 for USB, 0.6.0 for PCI(KVM only)
> + and 1.0.5 for SCSI(KVM only)</span>:
> </p>
>
> <pre>
> @@ -2227,12 +2227,29 @@
> </devices>
> ...</pre>
>
> + <p>or:</p>
> +
> +<pre>
> + ...
> + <devices>
> + <hostdev mode='subsystem' type='scsi'>
> + <source>
> + <adapter name='scsi_host0'/>
> + <address type='scsi' bus='0' target='0' unit='0'/>
> + </source>
> + <readonly/>
> + <address type='scsi' controller='0' bus='0' target='0' unit='0'/>
s/scsi/drive/,
> + </hostdev>
> + </devices>
> + ...</pre>
> +
> <dl>
> <dt><code>hostdev</code></dt>
> <dd>The <code>hostdev</code> element is the main container for describing
> host devices. For usb device passthrough <code>mode</code> is always
> - "subsystem" and <code>type</code> is "usb" for a USB device and "pci"
> - for a PCI device. When <code>managed</code> is "yes" for a PCI
> + "subsystem" and <code>type</code> is "usb" for a USB device, "pci"
> + for a PCI device and "scsi" for a SCSI device. When
> + <code>managed</code> is "yes" for a PCI
> device, it is detached from the host before being passed on to
> the guest, and reattached to the host after the guest exits.
> If <code>managed</code> is omitted or "no", and for USB
> @@ -2242,13 +2259,16 @@
> hot-plugging the device,
> and <code>virNodeDeviceReAttach</code> (or <code>virsh
> nodedev-reattach</code>) after hot-unplug or stopping the
> - guest.</dd>
> + guest. For SCSI device, user is responsible to make sure the device
> + is not used by host.</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
> <code>vendor</code> and <code>product</code> elements or by the device's
> address on the hosts using the <code>address</code> element. PCI devices
> on the other hand can only be described by their <code>address</code>.
> + SCSI devices can be described by <code>adapter</code> and
> + <code>address</code>.
>
> <span class="since">Since 1.0.0</span>, the <code>source</code> element
> of USB devices may contain <code>startupPolicy</code> attribute which can
> @@ -2275,6 +2295,9 @@
> <code>id</code> attribute that specifies the USB vendor and product id.
> The ids can be given in decimal, hexadecimal (starting with 0x) or
> octal (starting with 0) form.</dd>
> + <dt><code>readonly</code></dt>
> + <dd>Indicates that the device is readonly, only valid for SCSI device.
> + <span class="since">Since 1.0.5</span></dd>
> <dt><code>boot</code></dt>
> <dd>Specifies that the device is bootable. The <code>order</code>
> attribute determines the order in which devices will be tried during
> @@ -2283,6 +2306,7 @@
> <a href="#elementsOSBIOS">BIOS bootloader</a> section.
> <span class="since">Since 0.8.8</span> for PCI devices,
> <span class="since">Since 1.0.1</span> for USB devices.
> + <span class="since">Since 1.0.5</span> for SCSI devices.
> <dt><code>rom</code></dt>
> <dd>The <code>rom</code> element is used to change how a PCI
> device's ROM is presented to the guest. The optional <code>bar</code>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 2c31f76..d4d2bb9 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2988,6 +2988,7 @@
> <choice>
> <ref name="hostdevsubsyspci"/>
> <ref name="hostdevsubsysusb"/>
> + <ref name="hostdevsubsysscsi"/>
> </choice>
> </define>
>
> @@ -3043,6 +3044,23 @@
> </element>
> </define>
>
> + <define name="hostdevsubsysscsi">
> + <attribute name="type">
> + <value>scsi</value>
> + </attribute>
> + <element name="source">
> + <ref name="sourceinfoadapter"/>
> + <element name="address">
> + <ref name="scsiaddress"/>
> + </element>
> + </element>
> + <optional>
> + <element name='readonly'>
> + <empty/>
> + </element>
> + </optional>
> + </define>
> +
> <define name="hostdevcapsstorage">
> <attribute name="type">
> <value>storage</value>
> @@ -3098,6 +3116,17 @@
> </attribute>
> </element>
> </define>
> + <define name="scsiaddress">
> + <attribute name="bus">
> + <ref name="driveBus"/>
> + </attribute>
> + <attribute name="target">
> + <ref name="driveTarget"/>
> + </attribute>
> + <attribute name="unit">
> + <ref name="driveUnit"/>
> + </attribute>
> + </define>
> <define name="usbportaddress">
> <attribute name="bus">
> <ref name="usbAddr"/>
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index a776058..2fb5989 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -398,6 +398,16 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev,
> goto cleanup;
> }
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + if (virAsprintf(&address, "%s:%d:%d:%d",
> + hostdev->source.subsys.u.scsi.adapter,
> + hostdev->source.subsys.u.scsi.bus,
> + hostdev->source.subsys.u.scsi.target,
> + hostdev->source.subsys.u.scsi.unit) < 0) {
> + VIR_WARN("OOM while encoding audit message");
> + goto cleanup;
> + }
> + break;
> default:
> VIR_WARN("Unexpected hostdev type while encoding audit message: %d",
> hostdev->source.subsys.type);
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 492e0b7..d640f65 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -578,7 +578,8 @@ VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
>
> VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
> "usb",
> - "pci")
> + "pci",
> + "scsi")
>
> VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
> "storage",
> @@ -1604,7 +1605,8 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
> if (def->parent.type == VIR_DOMAIN_DEVICE_NONE)
> virDomainDeviceInfoFree(def->info);
>
> - if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) {
> + switch (def->mode) {
> + case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
> switch (def->source.caps.type) {
> case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
> VIR_FREE(def->source.caps.u.storage.block);
> @@ -1616,6 +1618,11 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
> VIR_FREE(def->source.caps.u.net.iface);
> break;
> }
> + break;
> + case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS:
> + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)
> + VIR_FREE(def->source.subsys.u.scsi.adapter);
> + break;
> }
> }
>
> @@ -3319,6 +3326,92 @@ virDomainParseLegacyDeviceAddress(char *devaddr,
> }
>
> static int
> +virDomainHostdevSubsysScsiDefParseXML(const xmlNodePtr node,
> + virDomainHostdevDefPtr def)
> +{
> + int ret = -1;
> + bool got_address = false, got_adapter = false;
> + xmlNodePtr cur;
> + char *bus = NULL, *target = NULL, *unit = NULL;
> +
> + cur = node->children;
> + while (cur != NULL) {
> + if (cur->type == XML_ELEMENT_NODE) {
> + if (xmlStrEqual(cur->name, BAD_CAST "address")) {
> + if (got_address) {
> + virReportError(VIR_ERR_XML_ERROR,
The format string "%s" is missed.
> + _("more than one addresses are specified"));
> + goto cleanup;
> + }
> + if (!(bus = virXMLPropString(cur, "bus"))) {
> + virReportError(VIR_ERR_XML_ERROR,
Same.
> + _("bus must be specified for scsi hostdev address"));
> + goto cleanup;
> + }
> + if (virStrToLong_ui(bus, NULL, 0, &def->source.subsys.u.scsi.bus) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse bus %s"), bus);
> + goto cleanup;
> + }
> + if (!(target = virXMLPropString(cur, "target"))) {
> + virReportError(VIR_ERR_XML_ERROR,
Same.
> + _("target must be specified for scsi hostdev address"));
> + goto cleanup;
> + }
> + if (virStrToLong_ui(target, NULL, 0, &def->source.subsys.u.scsi.target) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse target %s"), target);
> + goto cleanup;
> + }
> + if (!(unit = virXMLPropString(cur, "unit"))) {
> + virReportError(VIR_ERR_XML_ERROR,
Same.
> + _("unit must be specified for scsi hostdev address"));
> + goto cleanup;
> + }
Since all of 'bus', 'target', and "unit" are mandatory, I changed the
checking like
below:
if (!(bus = virXMLPropString(cur, "bus")) ||
!(target = virXMLPropString(cur, "target")) ||
!(unit = virXMLPropString(cur, "unit"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("'bus', 'target', and 'unit' must
be specified "
"for scsi hostdev address"));
goto cleanup;
}
This tells the user what requires once.
> + if (virStrToLong_ui(unit, NULL, 0, &def->source.subsys.u.scsi.unit) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse unit %s"), unit);
> + goto cleanup;
> + }
> + got_address = true;
> + } else if (xmlStrEqual(cur->name, BAD_CAST "adapter")) {
> + if (got_adapter) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("more than one adapters are specified"));
> + goto cleanup;
> + }
> + if (!(def->source.subsys.u.scsi.adapter =
> + virXMLPropString(cur, "name"))) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("adapter must be specified for scsi hostdev address"));
> + goto cleanup;
> + }
> + got_adapter = true;
> + } else {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown scsi source type '%s'"),
I changed this into:
"unsuported element '%s' of scsi hostdev source"
> + cur->name);
> + goto cleanup;
> + }
> + }
> + cur = cur->next;
> + }
> +
> + if (!got_address || !got_adapter) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("'adapter' and 'address' must be specified"));
And:
Both 'adapter' and ......
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + VIR_FREE(bus);
> + VIR_FREE(target);
> + VIR_FREE(unit);
> + return ret;
> +}
> +
> +static int
> virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
> virDomainHostdevDefPtr def)
> {
> @@ -3613,6 +3706,10 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> if (virDomainHostdevSubsysUsbDefParseXML(sourcenode, def) < 0)
> goto error;
> break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + if (virDomainHostdevSubsysScsiDefParseXML(sourcenode, def) < 0)
> + goto error;
> + break;
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("address type='%s' not supported in hostdev interfaces"),
> @@ -8322,6 +8419,8 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
> }
>
> if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> + xmlNodePtr cur;
> + unsigned int readonly = 0;
I will change this to boolean.
> switch (def->source.subsys.type) {
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> @@ -8330,6 +8429,20 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
> _("PCI host devices must use 'pci' address type"));
> goto error;
> }
> +
> + break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + cur = node->children;
> + while (cur != NULL) {
> + if (cur->type == XML_ELEMENT_NODE) {
> + if (readonly == 0 &&
> + xmlStrEqual(cur->name, BAD_CAST "readonly"))
> + readonly = 1;
> + }
> + cur = cur->next;
> + }
> + def->readonly = readonly;
> +
> break;
> }
> }
> @@ -8858,6 +8971,17 @@ virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a,
> return 0;
> }
>
> +static int
> +virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr a,
> + virDomainHostdevDefPtr b)
> +{
> + if (STREQ(a->source.subsys.u.scsi.adapter, b->source.subsys.u.scsi.adapter) &&
> + a->source.subsys.u.scsi.bus == b->source.subsys.u.scsi.bus &&
> + a->source.subsys.u.scsi.target == b->source.subsys.u.scsi.target &&
> + a->source.subsys.u.scsi.unit == b->source.subsys.u.scsi.unit)
> + return 1;
> + return 0;
> +}
>
> static int
> virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
> @@ -8871,6 +8995,8 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
> return virDomainHostdevMatchSubsysPCI(a, b);
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> return virDomainHostdevMatchSubsysUSB(a, b);
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + return virDomainHostdevMatchSubsysSCSI(a, b);
> }
> return 0;
> }
> @@ -11028,6 +11154,18 @@ virDomainDefParseXML(xmlDocPtr xml,
> goto error;
> }
>
> + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
> + hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> + /* We define default mapping to be 1 controller, 1 bus,
> + * 1 target and many units. And we reserve first 16 unit for
> + * disk usage in virDomainDiskDefAssignAddress */
> + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
> + hostdev->info->addr.drive.controller = 0;
> + hostdev->info->addr.drive.bus = 0;
> + hostdev->info->addr.drive.target = 0;
> + hostdev->info->addr.drive.unit = 16 + i;
Why do we need to set the default values here? Per the "address" is
mandatory.
> + }
> +
> def->hostdevs[def->nhostdevs++] = hostdev;
> }
> VIR_FREE(nodes);
> @@ -12600,6 +12738,30 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def)
> return 0;
> }
>
> +static int
> +virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
> +{
> + /* Look for any hostdev scsi dev */
> + int i;
> + int maxController = -1;
> + virDomainHostdevDefPtr hostdev;
> +
> + for (i = 0; i < def->nhostdevs; i++) {
> + hostdev = def->hostdevs[i];
> + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
> + (int)hostdev->info->addr.drive.controller > maxController) {
> + maxController = hostdev->info->addr.drive.controller;
> + }
> + }
> +
> + for (i = 0; i <= maxController; i++) {
> + if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, i) < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
>
> /*
> * Based on the declared <address/> info for any devices,
> @@ -12636,6 +12798,9 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
> if (virDomainDefMaybeAddSmartcardController(def) < 0)
> return -1;
>
> + if (virDomainDefMaybeAddHostdevSCSIcontroller(def) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -13487,6 +13652,15 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
> virBufferAdjustIndent(buf, 2);
> switch (def->source.subsys.type)
> {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + virBufferAsprintf(buf, "<adapter name='%s'/>\n",
> + def->source.subsys.u.scsi.adapter);
> + virBufferAsprintf(buf, "<address %sbus='%d' target='%d' unit='%d'/>\n",
> + includeTypeInAddr ? "type='scsi' " : "",
> + def->source.subsys.u.scsi.bus,
> + def->source.subsys.u.scsi.target,
> + def->source.subsys.u.scsi.unit);
> + break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> if (def->source.subsys.u.usb.vendor) {
> virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n",
> @@ -14742,6 +14916,9 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS:
> if (virDomainHostdevDefFormatSubsys(buf, def, flags, false) < 0)
> return -1;
> + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
> + def->readonly)
> + virBufferAsprintf(buf, "<readonly/>\n");
> break;
> case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
> if (virDomainHostdevDefFormatCaps(buf, def) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 08b8e48..3fdbf91 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -380,6 +380,7 @@ enum virDomainHostdevMode {
> enum virDomainHostdevSubsysType {
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB,
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI,
> + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
>
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
> };
> @@ -399,6 +400,12 @@ struct _virDomainHostdevSubsys {
> unsigned vendor;
> unsigned product;
> } usb;
> + struct {
> + char *adapter;
> + unsigned bus;
> + unsigned target;
> + unsigned unit;
> + } scsi;
> virDevicePCIAddress pci; /* host address */
> } u;
> };
> @@ -437,6 +444,7 @@ struct _virDomainHostdevDef {
> int startupPolicy; /* enum virDomainStartupPolicy */
> unsigned int managed : 1;
> unsigned int missing : 1;
> + unsigned int readonly : 1; /* readonly is only used for scsi hostdev */
Changed into boolean.
The diff after I fixed the pointed out problems, with the xml2xml test
in 10/10
merged:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3.diff
Type: text/x-patch
Size: 9294 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130410/5f544332/attachment-0001.bin>
More information about the libvir-list
mailing list