External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 14:15:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

storage: Don't use a stack copy of the adapter

Browse Source 
https://bugzilla.redhat.com/show_bug.cgi?id=1160926

Passing a copy of the storage pool adapter to a function just changes the
copy of the fields in the particular function and then when returning to
the caller those changes are discarded.  While not yet biting us in the
storage clean-up case, it did cause an issue for the fchost storage pool
startup case, createVport.  The issue was at startup, if no parent is found
in the XML, the code will search for the 'best available' parent and then
store that in the in memory copy of the adapter.  Of course, in this case
it was a copy, so when returning to the virStorageBackendSCSIStartPool that
change was discarded (or lost) from the pool->def->source.adapter which
meant at shutdown (deleteVport), the code assumed no adapter was passed
and skipped the deletion, leaving the vHBA created by libvirt still defined
requiring an additional stop of a nodedev-destroy to remove.

Adjusted the createVport to take virStoragePoolDefPtr instead of the
adapter copy. Then use the virStoragePoolSourceAdapterPtr when processing.
A future patch will need the 'def' anyway, so this just sets up for that.
This commit is contained in:
John Ferlan 2014-11-10 11:34:57 -05:00
parent 42a021c120
commit 5b226fcdc6
3 changed files with 25 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/conf/storage_conf.c
View File

@ -343,15 +343,15 @@ virStorageVolDefFree(virStorageVolDefPtr def)
} }
static void static void
virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapter adapter) virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter)
{ {
if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
VIR_FREE(adapter.data.fchost.wwnn); VIR_FREE(adapter->data.fchost.wwnn);
VIR_FREE(adapter.data.fchost.wwpn); VIR_FREE(adapter->data.fchost.wwpn);
VIR_FREE(adapter.data.fchost.parent); VIR_FREE(adapter->data.fchost.parent);
} else if (adapter.type == } else if (adapter->type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
VIR_FREE(adapter.data.scsi_host.name); VIR_FREE(adapter->data.scsi_host.name);
} }
} }
@ -380,7 +380,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
VIR_FREE(source->devices); VIR_FREE(source->devices);
VIR_FREE(source->dir); VIR_FREE(source->dir);
VIR_FREE(source->name); VIR_FREE(source->name);
virStoragePoolSourceAdapterClear(source->adapter); virStoragePoolSourceAdapterClear(&source->adapter);
VIR_FREE(source->initiator.iqn); VIR_FREE(source->initiator.iqn);
virStorageAuthDefFree(source->auth); virStorageAuthDefFree(source->auth);
VIR_FREE(source->vendor); VIR_FREE(source->vendor);

1
src/conf/storage_conf.h
View File

@ -177,6 +177,7 @@ typedef enum {
VIR_ENUM_DECL(virStoragePoolSourceAdapter) VIR_ENUM_DECL(virStoragePoolSourceAdapter)
typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter; typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter;
typedef virStoragePoolSourceAdapter *virStoragePoolSourceAdapterPtr;
struct _virStoragePoolSourceAdapter { struct _virStoragePoolSourceAdapter {
int type; /* virStoragePoolSourceAdapterType */ int type; /* virStoragePoolSourceAdapterType */

32
src/storage/storage_backend_scsi.c
View File

@ -644,24 +644,25 @@ checkVhbaSCSIHostParent(virConnectPtr conn,
static int static int
createVport(virConnectPtr conn, createVport(virConnectPtr conn,
virStoragePoolSourceAdapter adapter) virStoragePoolDefPtr def)
{ {
virStoragePoolSourceAdapterPtr adapter = &def->source.adapter;
unsigned int parent_host; unsigned int parent_host;
char *name = NULL; char *name = NULL;
if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) if (adapter->type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
return 0; return 0;
/* If a parent was provided, then let's make sure it's vhost capable */ /* If a parent was provided, then let's make sure it's vhost capable */
if (adapter.data.fchost.parent) { if (adapter->data.fchost.parent) {
if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) if (virGetSCSIHostNumber(adapter->data.fchost.parent, &parent_host) < 0)
return -1; return -1;
if (!virIsCapableFCHost(NULL, parent_host)) { if (!virIsCapableFCHost(NULL, parent_host)) {
virReportError(VIR_ERR_XML_ERROR, virReportError(VIR_ERR_XML_ERROR,
_("parent '%s' specified for vHBA " _("parent '%s' specified for vHBA "
"is not vport capable"), "is not vport capable"),
adapter.data.fchost.parent); adapter->data.fchost.parent);
return -1; return -1;
} }
} }
@ -670,35 +671,35 @@ createVport(virConnectPtr conn,
* using the wwnn/wwpn, then a nodedev is already created for * using the wwnn/wwpn, then a nodedev is already created for
* this pool and we don't have to create the vHBA * this pool and we don't have to create the vHBA
*/ */
if ((name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, if ((name = virGetFCHostNameByWWN(NULL, adapter->data.fchost.wwnn,
adapter.data.fchost.wwpn))) { adapter->data.fchost.wwpn))) {
int retval = 0; int retval = 0;
/* If a parent was provided, let's make sure the 'name' we've /* If a parent was provided, let's make sure the 'name' we've
* retrieved has the same parent * retrieved has the same parent
*/ */
if (adapter.data.fchost.parent && if (adapter->data.fchost.parent &&
!checkVhbaSCSIHostParent(conn, name, adapter.data.fchost.parent)) !checkVhbaSCSIHostParent(conn, name, adapter->data.fchost.parent))
retval = -1; retval = -1;
VIR_FREE(name); VIR_FREE(name);
return retval; return retval;
} }
if (!adapter.data.fchost.parent) { if (!adapter->data.fchost.parent) {
if (!(adapter.data.fchost.parent = virFindFCHostCapableVport(NULL))) { if (!(adapter->data.fchost.parent = virFindFCHostCapableVport(NULL))) {
virReportError(VIR_ERR_XML_ERROR, "%s", virReportError(VIR_ERR_XML_ERROR, "%s",
_("'parent' for vHBA not specified, and " _("'parent' for vHBA not specified, and "
"cannot find one on this host")); "cannot find one on this host"));
return -1; return -1;
} }
if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) if (virGetSCSIHostNumber(adapter->data.fchost.parent, &parent_host) < 0)
return -1; return -1;
} }
if (virManageVport(parent_host, adapter.data.fchost.wwpn, if (virManageVport(parent_host, adapter->data.fchost.wwpn,
adapter.data.fchost.wwnn, VPORT_CREATE) < 0) adapter->data.fchost.wwnn, VPORT_CREATE) < 0)
return -1; return -1;
virFileWaitForDevices(); virFileWaitForDevices();
@ -816,8 +817,7 @@ static int
virStorageBackendSCSIStartPool(virConnectPtr conn, virStorageBackendSCSIStartPool(virConnectPtr conn,
virStoragePoolObjPtr pool) virStoragePoolObjPtr pool)
{ {
virStoragePoolSourceAdapter adapter = pool->def->source.adapter; return createVport(conn, pool->def);
return createVport(conn, adapter);
} }
static int static int