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

Re: [libvirt] [PATCH 1/7] New XML attributes for storage pool source adapter



On 05/04/13 22:53, John Ferlan wrote:
On 03/25/2013 12:43 PM, Osier Yang wrote:
This introduces 4 new attributes for storage pool source adapter.
E.g.

<adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>

Attribute 'type' can be either 'scsi_host' or 'fc_host', and defaults
to 'scsi_host' if attribute 'name' is specified. I.e. It's optional
for 'scsi_host' adapter, for back-compat reason. However, mandatory
for 'fc_host' adapter and any new future adapter types. Attribute
'parent' is to specify the parent for the fc_host adapter.

* docs/formatstorage.html.in:
   - Add documents for the 4 new attrs
* docs/schemas/storagepool.rng:
   - Add RNG schema
* src/conf/storage_conf.c:
   - Parse and format the new XMLs
* src/conf/storage_conf.h:
   - New struct virStoragePoolSourceAdapter, replace "char *adapter" with it;
   - New enum virStoragePoolSourceAdapterType
* src/libvirt_private.syms:
   - Export TypeToString and TypeFromString
* src/phyp/phyp_driver.c:
   - Replace "adapter" with "adapter.data.name", which is member of the union
     of the new struct virStoragePoolSourceAdapter now. Later patch will
     add the checking, as "adapter.data.name" is only valid for "scsi_host"
     adapter.
* src/storage/storage_backend_scsi.c:
   - Like above
* tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml:
* tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml:
   - New test for 'fc_host' and "scsi_host" adapter
* tests/storagepoolxml2xmlout/pool-scsi.xml:
   - Change the expected output, as the 'type' defaults to 'scsi_host' if 'name"
     specified now
* tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.xml:
* tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml:
   - New test
* tests/storagepoolxml2xmltest.c:
   - Include the test
---
  docs/formatstorage.html.in                         |  16 ++-
  docs/schemas/storagepool.rng                       |  33 +++++-
  src/conf/storage_conf.c                            | 132 +++++++++++++++++++--
  src/conf/storage_conf.h                            |  23 +++-
  src/libvirt_private.syms                           |   2 +
  src/phyp/phyp_driver.c                             |   8 +-
  src/storage/storage_backend_scsi.c                 |   6 +-
  .../pool-scsi-type-fc-host.xml                     |  15 +++
  .../pool-scsi-type-scsi-host.xml                   |  15 +++
  .../pool-scsi-type-fc-host.xml                     |  18 +++
  .../pool-scsi-type-scsi-host.xml                   |  18 +++
  tests/storagepoolxml2xmlout/pool-scsi.xml          |   2 +-
  tests/storagepoolxml2xmltest.c                     |   2 +
  13 files changed, 266 insertions(+), 24 deletions(-)
  create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml
  create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml
  create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml
  create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.xml

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 9b68738..eff3016 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -88,8 +88,20 @@
          <span class="since">Since 0.4.1</span></dd>
        <dt><code>adapter</code></dt>
        <dd>Provides the source for pools backed by SCSI adapters. May
-        only occur once. Contains a single attribute <code>name</code>
-        which is the SCSI adapter name (ex. "host1").
+        only occur once. Attribute <code>name</code> is the SCSI adapter
+        name (ex. "host1"). Attribute <code>type</code>
+        (<span class="since">1.0.4</span>) specifies the adapter type.
1.0.5 now right

+        Valid value are "fc_host" and "scsi_host".  If omitted and
s/value/values/

+        the <code>name</code> attribute is specified, then it defaults to
+        "scsi_host". To keep backwards compatibility, the attribute
+        <code>type</code> is optional for the "scsi_host" adapter, but
+        mandatory for the "fc_host" adapter.  Attributes <code>wwnn</code>
+        (Word Wide Node Name) and <code>wwpn</code> (Word Wide Port Name)
+        (<span class="since">1.0.4</span>) are used by the "fc_host" adapter
1.0.5

+        to uniquely indentify the device in the Fibre Channel storage farbic
s/indentify/identify
s/farbic/fabric


+        (the device can be either a HBA or vHBA). The optional attribute
s/vHBA). The/vHBA).  Both wwnn and wwpn must be specified. The/

The point being - although both are optional, if one is specified, then
the other must be specified as well.

+        <code>parent</code> (<span class="since">1.0.4</span>) specifies the
1.0.5

+        parent device for the "fc_host" adapter.
Does it need to be said that wwwn, wwpn, and parent have no meaning for
scsi_host?

Yes.

Also could you add a note about how "parent" is used - that
is why would someone want/need to specify.

XML example will be good.  I will add when pushing.


One other perhaps confusing element is that "name" is only valid for
"scsi_host". I think rather discussing he attribute name first, start
with the type. Then add the to keep backwards compatibility, if only the
attribute name is provided for the adapter element, the "type" field
will be assumed/filled in.

Another thought would be how someone would go about getting the
wwwn/wwpn values - not sure if we are supposed to document, but it might
be useful.  Although thinking about it - I suppose there could be
different tools/ways to get that value across different OS's.

We have commands like nodedev-dumpxml, which can get the
wwnn/wwpn, I can mention it. But it's only supported for Linux platform.
For other platforms, I think it's just overkill for us to document here.


          <span class="since">Since 0.6.2</span></dd>
        <dt><code>host</code></dt>
        <dd>Provides the source for pools backed by storage from a
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 0cc0406..43283d2 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -276,9 +276,36 @@
<define name='sourceinfoadapter'>
      <element name='adapter'>
-      <attribute name='name'>
-        <text/>
-      </attribute>
+      <choice>
+        <group>
+          <!-- To keep back-compat, 'type' is not mandatory for
+           scsi_host adapter -->
+          <optional>
+            <attribute name='type'>
+              <value>scsi_host</value>
+            </attribute>
+          </optional>
+          <attribute name='name'>
+            <text/>
+          </attribute>
+        </group>
+        <group>
+          <attribute name='type'>
+            <value>fc_host</value>
+          </attribute>
+          <optional>
+            <attribute name='parent'>
+              <text/>
+            </attribute>
+          </optional>
+          <attribute name='wwnn'>
+            <ref name='wwn'/>
+          </attribute>
+          <attribute name='wwpn'>
+            <ref name='wwn'/>
+          </attribute>
+        </group>
+      </choice>
        <empty/>
      </element>
    </define>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9134a22..deb4221 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -90,6 +90,10 @@ VIR_ENUM_IMPL(virStoragePartedFsType,
                "ext2", "ext2",
                "extended")
+VIR_ENUM_IMPL(virStoragePoolSourceAdapterType,
+              VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST,
+              "default", "scsi_host", "fc_host")
+
  typedef const char *(*virStorageVolFormatToString)(int format);
  typedef int (*virStorageVolFormatFromString)(const char *format);
@@ -304,6 +308,19 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
      VIR_FREE(def);
  }
+static void
+virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapter adapter)
+{
+    if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+        VIR_FREE(adapter.data.fchost.wwnn);
+        VIR_FREE(adapter.data.fchost.wwpn);
+        VIR_FREE(adapter.data.fchost.parent);
+    } else if (adapter.type ==
+               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+        VIR_FREE(adapter.data.name);
+    }
+}
+
  void
  virStoragePoolSourceClear(virStoragePoolSourcePtr source)
  {
@@ -324,7 +341,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
      VIR_FREE(source->devices);
      VIR_FREE(source->dir);
      VIR_FREE(source->name);
-    VIR_FREE(source->adapter);
+    virStoragePoolSourceAdapterClear(source->adapter);
      VIR_FREE(source->initiator.iqn);
      VIR_FREE(source->vendor);
      VIR_FREE(source->product);
@@ -489,6 +506,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
      virStoragePoolOptionsPtr options;
      char *name = NULL;
      char *port = NULL;
+    char *adapter_type = NULL;
      int n;
relnode = ctxt->node;
@@ -580,7 +598,53 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
      }
source->dir = virXPathString("string(./dir/@path)", ctxt);
-    source->adapter = virXPathString("string(./adapter/@name)", ctxt);
+
+    if ((adapter_type = virXPathString("string(./adapter/@type)", ctxt))) {
+        if ((source->adapter.type =
+             virStoragePoolSourceAdapterTypeTypeFromString(adapter_type)) <= 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("Unknown pool adapter type '%s'"),
+                           adapter_type);
+            goto cleanup;
+        }
+
+        if (source->adapter.type ==
+            VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            source->adapter.data.fchost.parent =
+                virXPathString("string(./adapter/@parent)", ctxt);
+            source->adapter.data.fchost.wwnn =
+                virXPathString("string(./adapter/@wwnn)", ctxt);
+            source->adapter.data.fchost.wwpn =
+                virXPathString("string(./adapter/@wwpn)", ctxt);
+        } else if (source->adapter.type ==
+                   VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            source->adapter.data.name =
+                virXPathString("string(./adapter/@name)", ctxt);
+        }
+    } else {
+        char *wwnn = NULL;
+        char *wwpn = NULL;
+
+        wwnn = virXPathString("string(./adapter/@wwnn)", ctxt);
+        wwpn = virXPathString("string(./adapter/@wwpn)", ctxt);
+
+        if (wwnn || wwpn) {
+            VIR_FREE(wwnn);
+            VIR_FREE(wwpn);
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Use of 'wwnn' and 'wwpn' attributes "
+                             "requires the 'fc_host' adapter 'type'"));
+            goto cleanup;
+        }
Same for parent too, right?

Yes. I missed it. Will add.

  Conversely, if 'name' is present on a
fc_host type line, then it's bogus too.

So the questions then become - do we care? does it need to be errored or
just messaged?  or can it just be quietly ignored?   Not sure what the
"norm" is historically...  Since we don't check in the "if" portion of
this, then one could easily say - this is superfluous.

It's historically, actually we ignore a lot everywhere when parsing. My
opinion is except the fatal error, such as "type" is not specified, but
"wwnn/wwpn" are specified. Others can be just ignored if the required
atrributes are correctly parsed. E.g. for a "fc_host" adpater, if the
wwnn/wwpn  are correctly parsed, we don't care about if the "name"
is specified or not, it's just ignored. I agreed with doing so, because
if not, the XML parsing code will be huge, and it's overkill in my opinion.


+
+        /* To keep back-compat, 'type' is not required to specify
+         * for scsi_host adapter.
+         */
+        if ((source->adapter.data.name =
+             virXPathString("string(./adapter/@name)", ctxt)))
+            source->adapter.type =
+                VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST;
+    }
authType = virXPathString("string(./auth/@type)", ctxt);
      if (authType == NULL) {
@@ -618,6 +682,7 @@ cleanup:
      VIR_FREE(port);
      VIR_FREE(authType);
      VIR_FREE(nodeset);
+    VIR_FREE(adapter_type);
      return ret;
  }
@@ -819,11 +884,33 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {
      }
if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) {
-        if (!ret->source.adapter) {
-            virReportError(VIR_ERR_XML_ERROR,
-                           "%s", _("missing storage pool source adapter name"));
+        if (!ret->source.adapter.type) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("missing storage pool source adapter"));
              goto cleanup;
          }
+
+        if (ret->source.adapter.type ==
+            VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            if (!ret->source.adapter.data.fchost.wwnn ||
+                !ret->source.adapter.data.fchost.wwpn) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("'wwnn' and 'wwpn' must be specified for adapter "
+                                 "type 'fchost'"));
+                goto cleanup;
+            }
+
+            if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) ||
+                !virValidateWWN(ret->source.adapter.data.fchost.wwpn))
+                goto cleanup;
No validation of parent?  Makes me wonder at this point of the review
what is it's purpose...

See the rng schema, parent is a "<text/>", so no need to validate.


+        } else if (ret->source.adapter.type ==
+                   VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            if (!ret->source.adapter.data.name) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               "%s", _("missing storage pool source adapter name"));
+                goto cleanup;
+            }
+        }
      }
/* If DEVICE is the only source type, then its required */
@@ -953,9 +1040,23 @@ virStoragePoolSourceFormat(virBufferPtr buf,
      if ((options->flags & VIR_STORAGE_POOL_SOURCE_DIR) &&
          src->dir)
          virBufferAsprintf(buf,"    <dir path='%s'/>\n", src->dir);
-    if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) &&
-        src->adapter)
-        virBufferAsprintf(buf,"    <adapter name='%s'/>\n", src->adapter);
+    if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) {
+        if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
+            src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)
+            virBufferAsprintf(buf, "    <adapter type='%s'",
+                              virStoragePoolSourceAdapterTypeTypeToString(src->adapter.type));
+
+        if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            virBufferEscapeString(buf, " parent='%s'",
+                                  src->adapter.data.fchost.parent);
+            virBufferAsprintf(buf," wwnn='%s' wwpn='%s'/>\n",
+                              src->adapter.data.fchost.wwnn,
+                              src->adapter.data.fchost.wwpn);
+        } else if (src->adapter.type ==
+                 VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            virBufferAsprintf(buf," name='%s'/>\n", src->adapter.data.name);
+        }
+    }
      if ((options->flags & VIR_STORAGE_POOL_SOURCE_NAME) &&
          src->name)
          virBufferAsprintf(buf,"    <name>%s</name>\n", src->name);
@@ -1856,8 +1957,19 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
                  matchpool = pool;
              break;
          case VIR_STORAGE_POOL_SCSI:
-            if (STREQ(pool->def->source.adapter, def->source.adapter))
-                matchpool = pool;
+            if (pool->def->source.adapter.type ==
+                VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+                if (STREQ(pool->def->source.adapter.data.fchost.wwnn,
+                          def->source.adapter.data.fchost.wwnn) &&
+                    STREQ(pool->def->source.adapter.data.fchost.wwpn,
+                          def->source.adapter.data.fchost.wwpn))
+                    matchpool = pool;
+            } else if (pool->def->source.adapter.type ==
+                       VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST){
+                if (STREQ(pool->def->source.adapter.data.name,
+                          def->source.adapter.data.name))
+                    matchpool = pool;
+            }
              break;
          case VIR_STORAGE_POOL_ISCSI:
          {
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index ad16eca..60b8034 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -233,7 +233,28 @@ struct _virStoragePoolSourceDevice {
      } geometry;
  };
+enum virStoragePoolSourceAdapterType {
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_DEFAULT = 0,
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST,
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST,
+ VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST,
+};
+VIR_ENUM_DECL(virStoragePoolSourceAdapterType)
+
+typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter;
+struct _virStoragePoolSourceAdapter {
+    int type; /* enum virStoragePoolSourceAdapterType */
+
+    union {
+        char *name;
+        struct {
+            char *parent;
+            char *wwnn;
+            char *wwpn;
+        } fchost;
+    } data;
+};
typedef struct _virStoragePoolSource virStoragePoolSource;
  typedef virStoragePoolSource *virStoragePoolSourcePtr;
@@ -250,7 +271,7 @@ struct _virStoragePoolSource {
      char *dir;
/* Or an adapter */
-    char *adapter;
+    virStoragePoolSourceAdapter adapter;
/* Or a name */
      char *name;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a2c4a54..39c02ad 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -614,6 +614,8 @@ virStoragePoolObjLock;
  virStoragePoolObjRemove;
  virStoragePoolObjSaveDef;
  virStoragePoolObjUnlock;
+virStoragePoolSourceAdapterTypeTypeFromString;
+virStoragePoolSourceAdapterTypeTypeToString;
  virStoragePoolSourceClear;
  virStoragePoolSourceFindDuplicate;
  virStoragePoolSourceFindDuplicateDevices;
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 59cc1ca..3a97364 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -2062,7 +2062,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool,
      spdef->source.ndevice = 1;
/*XXX source adapter not working properly, should show hdiskX */
-    if ((spdef->source.adapter =
+    if ((spdef->source.adapter.data.name =
Is it safe to go directly here without checking the type?

Same question and same reply in v1, :-)

It's sorted out in later patch, this match just add the xml support, and make
sure the build is not broken.


           phypGetStoragePoolDevice(pool->conn, pool->name)) == NULL) {
          VIR_ERROR(_("Unable to determine storage pools's source adapter."));
          goto err;
@@ -2284,7 +2284,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags)
pool.source.ndevice = 1; - if ((pool.source.adapter =
+    if ((pool.source.adapter.data.name =
Same question

           phypGetStoragePoolDevice(sp->conn, sp->name)) == NULL) {
          VIR_ERROR(_("Unable to determine storage sps's source adapter."));
          goto cleanup;
@@ -2524,7 +2524,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def)
                            managed_system, vios_id);
virBufferAsprintf(&buf, "mksp -f %schild %s", def->name,
-                      source.adapter);
+                      source.adapter.data.name);
Again

if (system_type == HMC)
          virBufferAddChar(&buf, '\'');
@@ -2765,7 +2765,7 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool, unsigned int flags)
      def.source.ndevice = 1;
/*XXX source adapter not working properly, should show hdiskX */
-    if ((def.source.adapter =
+    if ((def.source.adapter.data.name =
Again

           phypGetStoragePoolDevice(pool->conn, pool->name)) == NULL) {
          VIR_ERROR(_("Unable to determine storage pools's source adapter."));
          goto err;
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 90bbf59..c1c163e 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -639,7 +639,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
      char *path;
*isActive = false;
-    if (virAsprintf(&path, "/sys/class/scsi_host/%s", pool->def->source.adapter) < 0) {
+    if (virAsprintf(&path, "/sys/class/scsi_host/%s", pool->def->source.adapter.data.name) < 0) {
This one seems OK since it's a SCSI function

          virReportOOMError();
          return -1;
      }
@@ -661,9 +661,9 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
pool->def->allocation = pool->def->capacity = pool->def->available = 0; - if (sscanf(pool->def->source.adapter, "host%u", &host) != 1) {
+    if (sscanf(pool->def->source.adapter.data.name, "host%u", &host) != 1) {
          VIR_DEBUG("Failed to get host number from '%s'",
-                    pool->def->source.adapter);
+                    pool->def->source.adapter.data.name);
          retval = -1;
          goto out;
      }
diff --git a/tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml b/tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml
new file mode 100644
index 0000000..1e9826d
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml
@@ -0,0 +1,15 @@
+<pool type='scsi'>
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <source>
+    <adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml b/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml
new file mode 100644
index 0000000..4f48133
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml
@@ -0,0 +1,15 @@
+<pool type="scsi">
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <source>
+    <adapter type='scsi_host' name="host0"/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml b/tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml
new file mode 100644
index 0000000..fb1079f
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml
@@ -0,0 +1,18 @@
+<pool type='scsi'>
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <capacity unit='bytes'>0</capacity>
+  <allocation unit='bytes'>0</allocation>
+  <available unit='bytes'>0</available>
+  <source>
+    <adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.xml b/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.xml
new file mode 100644
index 0000000..faf5342
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.xml
@@ -0,0 +1,18 @@
+<pool type='scsi'>
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <capacity unit='bytes'>0</capacity>
+  <allocation unit='bytes'>0</allocation>
+  <available unit='bytes'>0</available>
+  <source>
+    <adapter type='scsi_host' name='host0'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-scsi.xml b/tests/storagepoolxml2xmlout/pool-scsi.xml
index 321dc2b..faf5342 100644
--- a/tests/storagepoolxml2xmlout/pool-scsi.xml
+++ b/tests/storagepoolxml2xmlout/pool-scsi.xml
@@ -5,7 +5,7 @@
    <allocation unit='bytes'>0</allocation>
    <available unit='bytes'>0</available>
    <source>
-    <adapter name='host0'/>
+    <adapter type='scsi_host' name='host0'/>
Should there exist a test/config which keeps that back compat option?
That is should we make sure that if only the "name" is provided that the
right things happen.

This is the *xml2xmlout* data.  Now we always output the adpater "type"
when formating.


    </source>
    <target>
      <path>/dev/disk/by-path</path>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 8cac978..9be63c5 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -90,6 +90,8 @@ mymain(void)
      DO_TEST("pool-iscsi-auth");
      DO_TEST("pool-netfs");
      DO_TEST("pool-scsi");
+    DO_TEST("pool-scsi-type-scsi-host");
+    DO_TEST("pool-scsi-type-fc-host");
      DO_TEST("pool-mpath");
      DO_TEST("pool-iscsi-multiiqn");
      DO_TEST("pool-iscsi-vendor-product");


ACK - with suggestions handled...

John

--
libvir-list mailing list
libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list


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