Re: [libvirt] [PATCH v3 14/18] conf: Convert virStoragePoolSourceAdapter to virStorageAdapter

Move the virStoragePoolSourceAdapter from storage_conf.h and rename to virStorageAdapter. Continue with code realignment for brevity and flow. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/storage_adapter_conf.c | 71 ++++++++++++++++++-------------------- src/conf/storage_adapter_conf.h | 51 ++++++++++++++++++++++++--- src/conf/storage_conf.c | 32 ++++++++--------- src/conf/storage_conf.h | 44 ++--------------------- src/libvirt_private.syms | 2 -- src/phyp/phyp_driver.c | 3 +- src/storage/storage_backend_scsi.c | 18 +++++----- src/test/test_driver.c | 5 ++- 8 files changed, 109 insertions(+), 117 deletions(-) diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index 6efe5ae..53c07c7 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -19,7 +19,7 @@ #include <config.h> -#include "storage_adapter_conf.h" +#include "storage_conf.h" #include "viralloc.h" #include "virerror.h" @@ -32,11 +32,10 @@ VIR_LOG_INIT("conf.storage_adapter_conf"); -VIR_ENUM_IMPL(virStoragePoolSourceAdapter, - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST, +VIR_ENUM_IMPL(virStorageAdapter, + VIR_STORAGE_ADAPTER_TYPE_LAST, "default", "scsi_host", "fc_host") - static void virStorageAdapterFCHostClear(virStorageAdapterFCHostPtr fchost) { @@ -50,12 +49,12 @@ virStorageAdapterFCHostClear(virStorageAdapterFCHostPtr fchost) void -virStorageAdapterClear(virStoragePoolSourceAdapterPtr adapter) +virStorageAdapterClear(virStorageAdapterPtr adapter) { - if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) virStorageAdapterFCHostClear(&adapter->data.fchost); - if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) VIR_FREE(adapter->data.scsi_host.name); } @@ -123,7 +122,7 @@ virStorageAdapterSCSIHostParseXML(xmlNodePtr node, static int virStorageAdapterLegacyParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, - virStoragePoolSourceAdapterPtr adapter) + virStorageAdapterPtr adapter) { char *wwnn = virXMLPropString(node, "wwnn"); char *wwpn = virXMLPropString(node, "wwpn"); @@ -154,14 +153,14 @@ virStorageAdapterLegacyParseXML(xmlNodePtr node, * for scsi_host adapter. */ if ((adapter->data.scsi_host.name = virXMLPropString(node, "name"))) - adapter->type = VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST; + adapter->type = VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST; return 0; } int -virStorageAdapterParseXML(virStoragePoolSourcePtr source, +virStorageAdapterParseXML(virStorageAdapterPtr adapter, xmlNodePtr node, xmlXPathContextPtr ctxt) { @@ -172,26 +171,24 @@ virStorageAdapterParseXML(virStoragePoolSourcePtr source, ctxt->node = node; if ((adapter_type = virXMLPropString(node, "type"))) { - if ((source->adapter.type = - virStoragePoolSourceAdapterTypeFromString(adapter_type)) <= 0) { + if ((adapter->type = + virStorageAdapterTypeFromString(adapter_type)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown pool adapter type '%s'"), adapter_type); goto cleanup; } - if (source->adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - if (virStorageAdapterFCHostParseXML(node, &source->adapter.data.fchost) < 0) - goto cleanup; - } else if (source->adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (virStorageAdapterSCSIHostParseXML(node, ctxt, &source->adapter.data.scsi_host) < 0) + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && + (virStorageAdapterFCHostParseXML(node, &adapter->data.fchost)) < 0) goto cleanup; - } + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && + (virStorageAdapterSCSIHostParseXML(node, ctxt, + &adapter->data.scsi_host)) < 0) + goto cleanup; } else { - if (virStorageAdapterLegacyParseXML(node, ctxt, &source->adapter) < 0) + if (virStorageAdapterLegacyParseXML(node, ctxt, adapter) < 0) goto cleanup; } @@ -260,21 +257,19 @@ virStorageAdapterSCSIHostParseValidate(virStorageAdapterSCSIHostPtr scsi_host) int -virStorageAdapterParseValidate(virStoragePoolDefPtr ret) +virStorageAdapterParseValidate(virStorageAdapterPtr adapter) { - if (!ret->source.adapter.type) { + if (!adapter->type) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source adapter")); return -1; } - if (ret->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) - return virStorageAdapterFCHostParseValidate(&ret->source.adapter.data.fchost); + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + return virStorageAdapterFCHostParseValidate(&adapter->data.fchost); - if (ret->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) - return virStorageAdapterSCSIHostParseValidate(&ret->source.adapter.data.scsi_host); + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) + return virStorageAdapterSCSIHostParseValidate(&adapter->data.scsi_host); return 0; } @@ -285,13 +280,13 @@ virStorageAdapterFCHostFormat(virBufferPtr buf, virStorageAdapterFCHostPtr fchost) { virBufferEscapeString(buf, " parent='%s'", fchost->parent); - if (fchost->managed) - virBufferAsprintf(buf, " managed='%s'", - virTristateBoolTypeToString(fchost->managed)); virBufferEscapeString(buf, " parent_wwnn='%s'", fchost->parent_wwnn); virBufferEscapeString(buf, " parent_wwpn='%s'", fchost->parent_wwpn); virBufferEscapeString(buf, " parent_fabric_wwn='%s'", fchost->parent_fabric_wwn); + if (fchost->managed != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " managed='%s'", + virTristateBoolTypeToString(fchost->managed));

virBufferAsprintf(buf, " wwnn='%s' wwpn='%s'/>\n", fchost->wwnn, fchost->wwpn); @@ -322,14 +317,14 @@ virStorageAdapterSCSIHostFormat(virBufferPtr buf, void virStorageAdapterFormat(virBufferPtr buf, - virStoragePoolSourcePtr src) + virStorageAdapterPtr adapter) { virBufferAsprintf(buf, "<adapter type='%s'", - virStoragePoolSourceAdapterTypeToString(src->adapter.type)); + virStorageAdapterTypeToString(adapter->type)); - if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) - virStorageAdapterFCHostFormat(buf, &src->adapter.data.fchost); + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + virStorageAdapterFCHostFormat(buf, &adapter->data.fchost); - if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) - virStorageAdapterSCSIHostFormat(buf, &src->adapter.data.scsi_host); + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) + virStorageAdapterSCSIHostFormat(buf, &adapter->data.scsi_host); } diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h index ec812a1..730a3ca 100644 --- a/src/conf/storage_adapter_conf.h +++ b/src/conf/storage_adapter_conf.h @@ -23,21 +23,62 @@ # include "virpci.h" # include "virxml.h" -# include "storage_conf.h" + +typedef enum { + VIR_STORAGE_ADAPTER_TYPE_DEFAULT = 0, + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST, + VIR_STORAGE_ADAPTER_TYPE_FC_HOST, + + VIR_STORAGE_ADAPTER_TYPE_LAST, +} virStorageAdapterType; +VIR_ENUM_DECL(virStorageAdapter) + +typedef struct _virStorageAdapterSCSIHost virStorageAdapterSCSIHost; +typedef virStorageAdapterSCSIHost *virStorageAdapterSCSIHostPtr; +struct _virStorageAdapterSCSIHost { + char *name; + virPCIDeviceAddress parentaddr; /* host address */ + int unique_id; + bool has_parent; +}; + +typedef struct _virStorageAdapterFCHost virStorageAdapterFCHost; +typedef virStorageAdapterFCHost *virStorageAdapterFCHostPtr; +struct _virStorageAdapterFCHost { + char *parent; + char *parent_wwnn; + char *parent_wwpn; + char *parent_fabric_wwn; + char *wwnn; + char *wwpn; + int managed; /* enum virTristateSwitch */ +}; + +typedef struct _virStorageAdapter virStorageAdapter; +typedef virStorageAdapter *virStorageAdapterPtr; +struct _virStorageAdapter { + int type; /* virStorageAdapterType */ + + union { + virStorageAdapterSCSIHost scsi_host; + virStorageAdapterFCHost fchost; + } data; +}; + void -virStorageAdapterClear(virStoragePoolSourceAdapterPtr adapter); +virStorageAdapterClear(virStorageAdapterPtr adapter); int -virStorageAdapterParseXML(virStoragePoolSourcePtr source, +virStorageAdapterParseXML(virStorageAdapterPtr adapter, xmlNodePtr node, xmlXPathContextPtr ctxt); int -virStorageAdapterParseValidate(virStoragePoolDefPtr ret); +virStorageAdapterParseValidate(virStorageAdapterPtr adapter); void virStorageAdapterFormat(virBufferPtr buf, - virStoragePoolSourcePtr src); + virStorageAdapterPtr adapter); #endif /* __VIR_STORAGE_ADAPTER_CONF_H__ */ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 45dc860..7207605 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -565,7 +565,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, goto cleanup; if ((adapternode = virXPathNode("./adapter", ctxt))) { - if (virStorageAdapterParseXML(source, adapternode, ctxt) < 0) + if (virStorageAdapterParseXML(&source->adapter, adapternode, ctxt) < 0) goto cleanup; } @@ -802,7 +802,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) } if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) && - (virStorageAdapterParseValidate(ret)) < 0) + (virStorageAdapterParseValidate(&ret->source.adapter)) < 0) goto error; /* If DEVICE is the only source type, then its required */ @@ -958,9 +958,9 @@ virStoragePoolSourceFormat(virBufferPtr buf, virBufferEscapeString(buf, "<dir path='%s'/>\n", src->dir); if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) && - (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST || - src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)) - virStorageAdapterFormat(buf, src); + (src->adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST || + src->adapter.type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST)) + virStorageAdapterFormat(buf, &src->adapter); if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) virBufferEscapeString(buf, "<name>%s</name>\n", src->name); @@ -2266,8 +2266,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, int ret = 1; virStoragePoolObjPtr pool = NULL; virStoragePoolObjPtr matchpool = NULL; - virStoragePoolSourceAdapterPtr pool_adapter; - virStoragePoolSourceAdapterPtr def_adapter; + virStorageAdapterPtr pool_adapter; + virStorageAdapterPtr def_adapter; /* Check the pool list for duplicate underlying storage */ for (i = 0; i < pools->count; i++) { @@ -2306,10 +2306,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, pool_adapter = &pool->def->source.adapter; def_adapter = &def->source.adapter; - if (pool_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST && - def_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (pool_adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST && + def_adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { virStorageAdapterFCHostPtr pool_fchost = &pool_adapter->data.fchost; virStorageAdapterFCHostPtr def_fchost = @@ -2319,9 +2317,9 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, STREQ(pool_fchost->wwpn, def_fchost->wwpn)) matchpool = pool; } else if (pool_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST && def_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virStorageAdapterSCSIHostPtr pool_scsi_host = &pool_adapter->data.scsi_host; virStorageAdapterSCSIHostPtr def_scsi_host = @@ -2341,9 +2339,9 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, if (pool_hostnum == def_hostnum) matchpool = pool; } else if (pool_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST && + VIR_STORAGE_ADAPTER_TYPE_FC_HOST && def_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virStorageAdapterFCHostPtr pool_fchost = &pool_adapter->data.fchost; virStorageAdapterSCSIHostPtr def_scsi_host = @@ -2360,9 +2358,9 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, } } else if (pool_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && + VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST && def_adapter->type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { virStorageAdapterSCSIHostPtr pool_scsi_host = &pool_adapter->data.scsi_host; virStorageAdapterFCHostPtr def_fchost = diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 1012e74..6f20111 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "device_conf.h" # include "object_event.h" +# include "storage_adapter_conf.h" # include <libxml/tree.h> @@ -170,47 +171,6 @@ struct _virStoragePoolSourceDevice { } geometry; }; -typedef enum { - 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, -} virStoragePoolSourceAdapterType; -VIR_ENUM_DECL(virStoragePoolSourceAdapter) - -typedef struct _virStorageAdapterSCSIHost virStorageAdapterSCSIHost; -typedef virStorageAdapterSCSIHost *virStorageAdapterSCSIHostPtr; -struct _virStorageAdapterSCSIHost { - char *name; - virPCIDeviceAddress parentaddr; /* host address */ - int unique_id; - bool has_parent; -}; - -typedef struct _virStorageAdapterFCHost virStorageAdapterFCHost; -typedef virStorageAdapterFCHost *virStorageAdapterFCHostPtr; -struct _virStorageAdapterFCHost { - char *parent; - char *parent_wwnn; - char *parent_wwpn; - char *parent_fabric_wwn; - char *wwnn; - char *wwpn; - int managed; /* enum virTristateSwitch */ -}; - -typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter; -typedef virStoragePoolSourceAdapter *virStoragePoolSourceAdapterPtr; -struct _virStoragePoolSourceAdapter { - int type; /* virStoragePoolSourceAdapterType */ - - union { - virStorageAdapterSCSIHost scsi_host; - virStorageAdapterFCHost fchost; - } data; -}; - typedef struct _virStoragePoolSource virStoragePoolSource; typedef virStoragePoolSource *virStoragePoolSourcePtr; struct _virStoragePoolSource { @@ -226,7 +186,7 @@ struct _virStoragePoolSource { char *dir; /* Or an adapter */ - virStoragePoolSourceAdapter adapter; + virStorageAdapter adapter; /* Or a name */ char *name; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6a2bdf2..8a9e71b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -883,8 +883,6 @@ virStoragePoolObjSaveDef; virStoragePoolObjUnlock; virStoragePoolSaveConfig; virStoragePoolSaveState; -virStoragePoolSourceAdapterTypeFromString; -virStoragePoolSourceAdapterTypeToString;

virStoragePoolSourceClear; virStoragePoolSourceDeviceClear; virStoragePoolSourceFindDuplicate; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 7a5df3f..39fa026 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2467,8 +2467,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; - if (source.adapter.type != - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + if (source.adapter.type != VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Only 'scsi_host' adapter is supported")); goto cleanup; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 77a51ff..ff17409 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -176,12 +176,12 @@ virStoragePoolFCRefreshThread(void *opaque) } static char * -getAdapterName(virStoragePoolSourceAdapterPtr adapter) +getAdapterName(virStorageAdapterPtr adapter) { char *name = NULL; char *parentaddr = NULL; - if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host; if (scsi_host->has_parent) { @@ -197,7 +197,9 @@ getAdapterName(virStoragePoolSourceAdapterPtr adapter) } else { ignore_value(VIR_STRDUP(name, scsi_host->name)); } - } else if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + } + + if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) {

virStorageAdapterFCHostPtr fchost = &adapter->data.fchost; if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { @@ -451,7 +453,7 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, * the adapter based on might be not created yet. */ if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { virResetLastError(); return 0; } else { @@ -505,24 +507,24 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } + static int virStorageBackendSCSIStartPool(virConnectPtr conn, virStoragePoolObjPtr pool) { - if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) return createVport(conn, pool->def, pool->configFile, &pool->def->source.adapter.data.fchost); return 0; } + static int virStorageBackendSCSIStopPool(virConnectPtr conn, virStoragePoolObjPtr pool) { - if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) return deleteVport(conn, &pool->def->source.adapter.data.fchost); return 0; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index cf7820a..18792bc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4420,8 +4420,7 @@ testStoragePoolCreateXML(virConnectPtr conn, goto cleanup; def = NULL; - if (pool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { /* In the real code, we'd call virVHBAManageVport followed by * find_new_device, but we cannot do that here since we're not * mocking udev. The mock routine will copy an existing vHBA and @@ -4623,7 +4622,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) privpool->active = 0; if (privpool->def->source.adapter.type == - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { if (testDestroyVport(privconn, privpool->def->source.adapter.data.fchost.wwnn, privpool->def->source.adapter.data.fchost.wwpn) < 0)