[libvirt] [PATCH 1/7] New XML attributes for storage pool source adapter
Osier Yang
jyang at redhat.com
Fri Apr 5 15:18:09 UTC 2013
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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list