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@redhat.com>
---
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)
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/10/2017 04:10 PM, John Ferlan wrote:
> 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@redhat.com>
> ---
> 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));
No test cases that are tripped up by this change in order? (Not saying there need to be, just wondering...)
>
> 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;
These are no longer used globally? (Just making sure)
> 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) {
If you're just getting rid of the "} else" in order to shorten the line, then I'd say leave it in...
> 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)
ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/12/2017 05:53 PM, Laine Stump wrote:
> On 03/10/2017 04:10 PM, John Ferlan wrote:
>> 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@redhat.com>
>> ---
>> 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 @@
[...]
>> 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));
>
> No test cases that are tripped up by this change in order? (Not saying there need to be, just wondering...)
>
No - the managed would probably only appear as "managed='no'", although
managed='yes' is the de facto default and essentially determines whether
or not to destroy the vHBA when the pool is destroyed.
I can move it if really desired/required - I guess I saw it as something
"after" defining parent, parent_wwnn/wwpn, or parent_fabric_wwn - when
adding those I should have put them after parent. Yeah, I know could
have been a separate patch.
>>
>> virBufferAsprintf(buf, " wwnn='%s' wwpn='%s'/>\n",
>> fchost->wwnn, fchost->wwpn);
>> @@ -322,14 +317,14 @@ virStorageAdapterSCSIHostFormat(virBufferPtr buf,
>>
[...]
>> 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;
>
> These are no longer used globally? (Just making sure)
>
Nope - replaced by virStorageAdapterType{From|To}String which are only
local to storage_adapter_conf.c... That's the VIR_ENUM_IMPL change.
>
>> virStoragePoolSourceClear;
>> virStoragePoolSourceDeviceClear;
>> virStoragePoolSourceFindDuplicate;
[...]
>> 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) {
>
> If you're just getting rid of the "} else" in order to shorten the line, then I'd say leave it in...
>
OK
>
>> 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;
>> }
>>
[...]
>
>
> ACK.
>
Before I go off and push all this - just want to double check if you
desire to see the adjustments in a v4 series or do you feel comfortable
enough with a "summary"?
1. The virStorageAdapterPtr and friends had no changes to their names.
2. The function/helper names are now:
virStorageAdapterClearFCHost
virStorageAdapterClear
virStorageAdapterParseXMLFCHost
virStorageAdapterParseXMLSCSIHost
virStorageAdapterParseXMLLegacy **
virStorageAdapterParseXML
virStorageAdapterValidateFCHost
virStorageAdapterValidateSCSIHost
virStorageAdapterValidate
virStorageAdapterFormatFCHost
virStorageAdapterFormatSCSIHost
virStorageAdapterFormat
** No such thing as a short comment ;-)
3. In patch1 rather than the single if statement - there are now two.
The end result is the following check:
if ((fchost->parent_wwnn && !fchost->parent_wwpn)) {
virReportError(VIR_ERR_XML_ERROR,
_("when providing parent_wwnn='%s', the "
"parent_wwpn must also be provided"),
fchost->parent_wwnn);
return -1;
}
if (!fchost->parent_wwnn && fchost->parent_wwpn) {
virReportError(VIR_ERR_XML_ERROR,
_("when providing parent_wwpn='%s', the "
"parent_wwnn must also be provided"),
fchost->parent_wwpn);
return -1;
}
which I'll replicate in a separately posted patch for node_device_conf
4. Using virPCIDeviceAddressPtr in getSCSIHostNumber and getAdapterName.
Ironically the "original" series I had passed along the
virStorageAdapterSCSIHostPtr, but since it's been decreed that a
src/util function cannot include a src/conf header, I had to back that off.
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/15/2017 09:33 AM, John Ferlan wrote:
>
>
> On 03/12/2017 05:53 PM, Laine Stump wrote:
>> On 03/10/2017 04:10 PM, John Ferlan wrote:
>>> 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@redhat.com>
>>> ---
>>> 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 @@
>
> [...]
>
>>> 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));
>>
>> No test cases that are tripped up by this change in order? (Not saying there need to be, just wondering...)
>>
>
> No - the managed would probably only appear as "managed='no'", although
> managed='yes' is the de facto default and essentially determines whether
> or not to destroy the vHBA when the pool is destroyed.
>
> I can move it if really desired/required
Nah, order is supposed to not matter, and any code that relies on the
order of the elements is broken and needs to be fixed. That's the only
reason I thought of the unit tests.
>
> Before I go off and push all this - just want to double check if you
> desire to see the adjustments in a v4 series or do you feel comfortable
> enough with a "summary"?
The summary is fine.
>
> 1. The virStorageAdapterPtr and friends had no changes to their names.
>
> 2. The function/helper names are now:
>
> virStorageAdapterClearFCHost
> virStorageAdapterClear
> virStorageAdapterParseXMLFCHost
> virStorageAdapterParseXMLSCSIHost
> virStorageAdapterParseXMLLegacy **
> virStorageAdapterParseXML
> virStorageAdapterValidateFCHost
> virStorageAdapterValidateSCSIHost
> virStorageAdapterValidate
> virStorageAdapterFormatFCHost
> virStorageAdapterFormatSCSIHost
> virStorageAdapterFormat
>
> ** No such thing as a short comment ;-)
>
> 3. In patch1 rather than the single if statement - there are now two.
> The end result is the following check:
>
> if ((fchost->parent_wwnn && !fchost->parent_wwpn)) {
> virReportError(VIR_ERR_XML_ERROR,
> _("when providing parent_wwnn='%s', the "
> "parent_wwpn must also be provided"),
> fchost->parent_wwnn);
> return -1;
> }
>
> if (!fchost->parent_wwnn && fchost->parent_wwpn) {
> virReportError(VIR_ERR_XML_ERROR,
> _("when providing parent_wwpn='%s', the "
> "parent_wwnn must also be provided"),
> fchost->parent_wwpn);
> return -1;
> }
>
> which I'll replicate in a separately posted patch for node_device_conf
Yeah, that sounds more understandable.
>
> 4. Using virPCIDeviceAddressPtr in getSCSIHostNumber and getAdapterName.
> Ironically the "original" series I had passed along the
> virStorageAdapterSCSIHostPtr, but since it's been decreed that a
> src/util function cannot include a src/conf header, I had to back that off.
But virPCIDeviceAddressPtr is defined in src/util/virpci.c. There are
*other* address types that are defined in conf (e.g.
virDomainDeviceDriveAddress), but not PCI addresses.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[...] >> >> 4. Using virPCIDeviceAddressPtr in getSCSIHostNumber and getAdapterName. >> Ironically the "original" series I had passed along the >> virStorageAdapterSCSIHostPtr, but since it's been decreed that a >> src/util function cannot include a src/conf header, I had to back that off. > > But virPCIDeviceAddressPtr is defined in src/util/virpci.c. There are > *other* address types that are defined in conf (e.g. > virDomainDeviceDriveAddress), but not PCI addresses. > > Well _virPCIDeviceAddress is in virpci.c, but virPCIDeviceAddressPtr is in virpci.h... I could change virSCSIHostGetNameByParentaddr to take a virPCIDeviceAddressPtr and a unique_id - but I'll let that be separate from this series because there's way too much going on already. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/15/2017 01:59 PM, John Ferlan wrote: > [...] > >>> >>> 4. Using virPCIDeviceAddressPtr in getSCSIHostNumber and getAdapterName. >>> Ironically the "original" series I had passed along the >>> virStorageAdapterSCSIHostPtr, but since it's been decreed that a >>> src/util function cannot include a src/conf header, I had to back that off. >> >> But virPCIDeviceAddressPtr is defined in src/util/virpci.c. There are >> *other* address types that are defined in conf (e.g. >> virDomainDeviceDriveAddress), but not PCI addresses. >> >> > > Well _virPCIDeviceAddress is in virpci.c, but virPCIDeviceAddressPtr is > in virpci.h... Yeah, that's what I meant. Just mistyped. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.