[libvirt] [PATCH 1/8] New XML attributes for storage pool source adapter

John Ferlan jferlan at redhat.com
Tue Feb 5 19:48:19 UTC 2013


On 02/04/2013 08:31 AM, 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-compact reason. However, it's mandatory
> for 'fc_host' adapter and any new future adapter types. Attribute 'parent'
> is required for vHBA, but optional for HBA.
> 
> * 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
> * src/storage/storage_backend_scsi.c:
>   - Like above
> * tests/storagepoolxml2xmlin/pool-scsi1.xml:
>   - New test for 'fc_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-scsi1.xml:
>   - New test
> * tests/storagepoolxml2xmltest.c:
>   - Include the test
> ---
>  docs/formatstorage.html.in                 |   13 +++-
>  docs/schemas/storagepool.rng               |   33 +++++++-
>  src/conf/storage_conf.c                    |  123 +++++++++++++++++++++++++--
>  src/conf/storage_conf.h                    |   23 +++++-
>  src/libvirt_private.syms                   |    2 +
>  src/phyp/phyp_driver.c                     |    8 +-
>  src/storage/storage_backend_scsi.c         |    6 +-
>  tests/storagepoolxml2xmlin/pool-scsi1.xml  |   15 ++++
>  tests/storagepoolxml2xmlout/pool-scsi.xml  |    2 +-
>  tests/storagepoolxml2xmlout/pool-scsi1.xml |   18 ++++
>  tests/storagepoolxml2xmltest.c             |    1 +
>  11 files changed, 220 insertions(+), 24 deletions(-)
>  create mode 100644 tests/storagepoolxml2xmlin/pool-scsi1.xml
>  create mode 100644 tests/storagepoolxml2xmlout/pool-scsi1.xml
> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 9f93db8..0849618 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -88,8 +88,17 @@
>          <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.2</span>) specifies the adapter type,

s/type,/type./

> +        valid value is "fc_host" or "scsi_host", defaults to "scsi_host"
> +        if <code>name</code> is specified. To keep back-compact,

Consider the following instead:

Valid values are "fc_host" (vHBA) and "scsi_host"(HBA). If omitted and
the <code>name</code> attribute is specified, then it defaults to
"scsi_host".

NOTE: Based on further description, I'm assuming "fc_host" is "vHBA" and
"scsi_host" is "HBA".  Rather than introduce (v)HBA below, I used this
opportunity to define them here...


s/back-compact/backwards compatibility the/

> +        <code>type</code> is optional for "scsi_host" adapter, but

/s/is optional for "scsi_host"/attribute is optional for the "scsi_host"/

> +        mandatory for "fc_host" adapter.  Attribute <code>wwnn</code> and

s/for/for the/

> +        <code>wwpn</code> (<span class="since">1.0.2</span>) indicates


s/Attribute/Attributes/
s/indicates/indicate/

Consider perhaps: "Attributes wwnn (world wide node name) and wwpn
(world wide port name) are used by the "fc_host" (vHBA) adapter to
uniquely identify the device in the Fibre Channel storage fabric.

Thus "the (v)HBA" below becomes unnecessary.

> +        the (v)HBA. The optional attribute <code>parent</code>

s/optional//

> +        (<span class="since">1.0.2</span>) specifies the parent device of
> +        the (v)HBA. It's optional for HBA, but required for vHBA.

s/of the v(HBA)/

The text is here is confusing because you say it's required for vHBA
here, but declare it optional later. It's completely ignored for
"scsi_host", but can be present for "fc_host". This is where you
introduced vHBA without defining it so it has led to my confusion. If
someone mistakenly supplied parent with scsi_host, then the
virStoragePoolSourceFormat() will not write out the parent.


>          <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 165e276..a3204ec 100644
> --- a/docs/schemas/storagepool.rng
> +++ b/docs/schemas/storagepool.rng
> @@ -275,9 +275,36 @@
>  
>    <define name='sourceinfoadapter'>
>      <element name='adapter'>
> -      <attribute name='name'>
> -        <text/>
> -      </attribute>
> +      <choice>
> +        <group>
> +          <!-- To keep back-compact, 'type' is not mandatory for

s/compact/compat/

> +           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>

The text description implies otherwise, but the code certainly declares
this 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 7a39998..ddb2945 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,18 @@ 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 +340,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 +505,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
>      virStoragePoolOptionsPtr options;
>      char *name = NULL;
>      char *port = NULL;
> +    char *adapter_type = NULL;
>      int n;
>  
>      relnode = ctxt->node;
> @@ -580,7 +597,45 @@ 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 {
> +        if (virXPathString("string(./adapter/@wwnn)", ctxt) ||
> +            virXPathString("string(./adapter/@wwpn)", ctxt)) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("'type' for 'fc_host' adapter must be specified"));

Message is confusing, consider "Use of 'wwnn' and 'wwpn' attributes not
valid for 'scsi_host" adapter." or "Use of 'wwnn' and 'wwpn' attributes
requires the 'fc_host' adapter 'type'".

> +            goto cleanup;
> +        }
I believe you will leak here if either of these is true.  Thus you'll
need some sort of
    wwnn = virXPath...
    wwpn = virXPath...
    if (wwnn || wwpn) {
        VIR_FREE(wwnn);
        VIR_FREE(wwpn);
> +
> +        /* To keep back-compact, 'type' is not required to specify

s/back-compact/backwards compatibility/

> +         * 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 +673,7 @@ cleanup:
>      VIR_FREE(port);
>      VIR_FREE(authType);
>      VIR_FREE(nodeset);
> +    VIR_FREE(adapter_type);
>      return ret;
>  }
>  
> @@ -819,11 +875,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) {

The fchost.parent is optional here - following the code, but not the docs.

> +            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;
> +        } 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 +1031,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);

Since parent is possible NULL you won't write anything, right?

> +            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 +1948,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 4a84b2b..c0c6b91 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -777,6 +777,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 74f04ff..a12a430 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -2058,7 +2058,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 =

So this code only works for scsi_host then?  Since data.name is not
valid for fc_host.

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

Same comment

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

and again

>  
>      if (system_type == HMC)
>          virBufferAddChar(&buf, '\'');
> @@ -2761,7 +2761,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 =

ditto

>           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) {
>          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;
>      }
I'll make the broader assumption that something with _scsi in the
filename means it only works for scsi.

> diff --git a/tests/storagepoolxml2xmlin/pool-scsi1.xml b/tests/storagepoolxml2xmlin/pool-scsi1.xml
> new file mode 100644
> index 0000000..1e9826d
> --- /dev/null
> +++ b/tests/storagepoolxml2xmlin/pool-scsi1.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/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'/>
>    </source>
>    <target>
>      <path>/dev/disk/by-path</path>

I think you shouldn't change this one - it becomes the backwards
compatibility check.  Then create a "pool-scsi-type-scsi.xml" using the
new syntax (or something close/similar to indicate the "scsi_host"
syntax is in use).

> diff --git a/tests/storagepoolxml2xmlout/pool-scsi1.xml b/tests/storagepoolxml2xmlout/pool-scsi1.xml
> new file mode 100644
> index 0000000..fb1079f
> --- /dev/null
> +++ b/tests/storagepoolxml2xmlout/pool-scsi1.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>

Change the name from "pool-scsi1.xml" -> "pool-scsi-type-fc.xml"

I think that'll be clearer

> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
> index 8cac978..c04eb69 100644
> --- a/tests/storagepoolxml2xmltest.c
> +++ b/tests/storagepoolxml2xmltest.c
> @@ -90,6 +90,7 @@ mymain(void)
>      DO_TEST("pool-iscsi-auth");
>      DO_TEST("pool-netfs");
>      DO_TEST("pool-scsi");
> +    DO_TEST("pool-scsi1");
>      DO_TEST("pool-mpath");
>      DO_TEST("pool-iscsi-multiiqn");
>      DO_TEST("pool-iscsi-vendor-product");
> 

Then change/add the names here too.


John




More information about the libvir-list mailing list