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.f...@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 @@
    &lt;/devices&gt;
    ...</pre>
+ <p>or:</p>
+
+<pre>
+  ...
+  &lt;devices&gt;
+    &lt;hostdev mode='subsystem' type='scsi'&gt;
+      &lt;source&gt;
+        &lt;adapter name='scsi_host0'/&gt;
+        &lt;address type='scsi' bus='0' target='0' unit='0'/&gt;
+      &lt;/source&gt;
+      &lt;readonly/&gt;
+      &lt;address type='scsi' controller='0' bus='0' target='0' unit='0'/&gt;

s/scsi/drive/,

+    &lt;/hostdev&gt;
+  &lt;/devices&gt;
+  ...</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:



diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 405e7d4..1a2c2be 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2238,7 +2238,7 @@
         &lt;address type='scsi' bus='0' target='0' unit='0'/&gt;
       &lt;/source&gt;
       &lt;readonly/&gt;
-      &lt;address type='scsi' controller='0' bus='0' target='0' unit='0'/&gt;
+      &lt;address type='drive' controller='0' bus='0' target='0' unit='0'/&gt;
     &lt;/hostdev&gt;
   &lt;/devices&gt;
   ...</pre>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d640f65..cf955c7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3339,57 +3339,57 @@ virDomainHostdevSubsysScsiDefParseXML(const xmlNodePtr node,
         if (cur->type == XML_ELEMENT_NODE) {
             if (xmlStrEqual(cur->name, BAD_CAST "address")) {
                 if (got_address) {
-                    virReportError(VIR_ERR_XML_ERROR,
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
                                    _("more than one addresses are specified"));
                     goto cleanup;
                 }
-                if (!(bus = virXMLPropString(cur, "bus"))) {
-                    virReportError(VIR_ERR_XML_ERROR,
-                                   _("bus must be specified for scsi hostdev address"));
+
+                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;
                 }
+
                 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,
-                                   _("target must be specified for scsi hostdev address"));
+                                   _("cannot parse bus '%s'"), bus);
                     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,
-                                   _("unit must be specified for scsi hostdev address"));
+                                   _("cannot parse target '%s'"), target);
                     goto cleanup;
                 }
+
                 if (virStrToLong_ui(unit, NULL, 0, &def->source.subsys.u.scsi.unit) < 0) {
                     virReportError(VIR_ERR_INTERNAL_ERROR,
-                                   _("cannot parse unit %s"), unit);
+                                   _("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,
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
                                    _("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"));
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("'adapter' must be specified for scsi hostdev address"));
                     goto cleanup;
                 }
+
                 got_adapter = true;
             } else {
                 virReportError(VIR_ERR_XML_ERROR,
-                               _("unknown scsi source type '%s'"),
+                               _("unsuported element '%s' of scsi hostdev source"),
                                cur->name);
                 goto cleanup;
             }
@@ -3398,8 +3398,8 @@ virDomainHostdevSubsysScsiDefParseXML(const xmlNodePtr node,
     }
 
     if (!got_address || !got_adapter) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       _("'adapter' and 'address' must be specified"));
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Both 'adapter' and 'address' must be specified"));
         goto cleanup;
     }
 
@@ -8420,7 +8420,6 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
 
     if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
         xmlNodePtr cur;
-        unsigned int readonly = 0;
         switch (def->source.subsys.type) {
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
             if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
@@ -8435,14 +8434,13 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
             cur = node->children;
             while (cur != NULL) {
                 if (cur->type == XML_ELEMENT_NODE) {
-                    if (readonly == 0 &&
-                        xmlStrEqual(cur->name, BAD_CAST "readonly"))
-                        readonly = 1;
+                    if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
+                        def->readonly = true;
+                        break;
+                    }
                 }
                 cur = cur->next;
             }
-            def->readonly = readonly;
-
             break;
         }
     }
@@ -11154,18 +11152,6 @@ 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;
-        }
-
         def->hostdevs[def->nhostdevs++] = hostdev;
     }
     VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3fdbf91..9ecea85 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -444,7 +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 */
+    bool readonly; /* readonly is only used for scsi hostdev */
     union {
         virDomainHostdevSubsys subsys;
         virDomainHostdevCaps caps;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml
new file mode 100644
index 0000000..11d1712
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.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'/>
+    <hostdev mode='subsystem' type='scsi' managed='yes'>
+      <source>
+        <adapter name='scsi_host0'/>
+        <address bus='0' target='0' unit='0'/>
+      </source>
+      <readonly/>
+      <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 200d41a..25d2436 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -229,6 +229,7 @@ mymain(void)
 
     DO_TEST("hostdev-usb-address");
     DO_TEST("hostdev-pci-address");
+    DO_TEST("hostdev-scsi-readonly");
     DO_TEST("pci-rom");
 
     DO_TEST("encrypted-disk");
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to