On 11/10/2014 05:45 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1160926
Introduce a 'managed' attribute to allow libvirt to decide whether to delete a vHBA vport created via external means such as nodedev-create. The code currently decides whether to delete the vHBA based solely on whether the parent was provided at creation time. However, that may not be the desired action, so rather than delete and force someone to create another vHBA via an additional nodedev-create allow the configuration of the storage pool to decide the desired action.
During createVport when libvirt does the VPORT_CREATE, set the managed value to YES if not already set to indicate to the deleteVport code that it should delete the vHBA when the pool is destroyed.
If libvirtd is restarted all the memory only state was lost, so for a persistent storage pool, use the virStoragePoolSaveConfig in order to write out the managed value.
Because we're now saving the current configuration, we need to be sure to not save the parent in the output XML if it was undefined at start. Saving the name would cause future starts to always use the same parent which is not the expected result when not providing a parent. By not providing a parent, libvirt is expected to find the best available vHBA port for each subsequent (re)start.
At deleteVport, use the new managed value to decide whether to execute the VPORT_DELETE. Since we no longer save the parent in memory or in XML when provided, if it was not provided, then we have to look it up.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatstorage.html.in | 13 +++ docs/schemas/basictypes.rng | 5 ++ src/conf/storage_conf.c | 17 ++++ src/conf/storage_conf.h | 1 + src/storage/storage_backend_scsi.c | 93 +++++++++++++++++----- .../pool-scsi-type-fc-host-managed.xml | 15 ++++ .../pool-scsi-type-fc-host-managed.xml | 18 +++++ tests/storagepoolxml2xmltest.c | 1 + 8 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-fc-host-managed.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-fc-host-managed.xml
<...snip...> Consider the following squished into deleteVport as testing asked about a condition where someone does a 'virsh nodedev-destroy' on the vHBA we've created resulting in the virGetFCHostNameByWWN() rightfully returning NULL (just like it would in the createVport case when the wwnn/wwpn doesn't exist). Allowing virsh nodedev-destroy to remove an "active" vHBA is perhaps a different issue... The desire was to get a real error message instead of: destroy the 'fc_host' pool # virsh pool-destroy fc-pool error: Failed to destroy pool fc-pool error: An error occurred, but the cause is unknown #
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 41ea67a..b1602ea 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c <...snip...> static int -deleteVport(virStoragePoolSourceAdapter adapter) +deleteVport(virConnectPtr conn, + virStoragePoolSourceAdapter adapter) { unsigned int parent_host; char *name = NULL; + char *vhba_parent = NULL; int ret = -1;
if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) return 0;
- /* It must be a HBA instead of a vHBA as long as "parent" - * is NULL. "createVport" guaranteed "parent" for a vHBA - * cannot be NULL, it's either specified in XML, or detected - * automatically. - */ - if (!adapter.data.fchost.parent) + VIR_DEBUG("conn=%p parent='%s', managed='%d' wwnn='%s' wwpn='%s'", + conn, NULLSTR(adapter.data.fchost.parent), + adapter.data.fchost.managed, + adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn); + + /* If we're not managing the deletion of the vHBA, then just return */ + if (adapter.data.fchost.managed != VIR_TRISTATE_BOOL_YES) return 0;
+ /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, adapter.data.fchost.wwpn))) - return -1; - - if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup;
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_sc index b1602ea..3f61610 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -764,8 +764,12 @@ deleteVport(virConnectPtr conn, /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) + adapter.data.fchost.wwpn))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find fc_host for wwnn='%s' and wwpn='%s'"), + adapter.data.fchost.wwnn, adapter.data.fchost.wwpn); goto cleanup; + } /* If at startup time we provided a parent, then use that to * get the parent_host value; otherwise, we have to determine
+ /* If at startup time we provided a parent, then use that to + * get the parent_host value; otherwise, we have to determine + * the parent scsi_host which we did not save at startup time + */ + if (adapter.data.fchost.parent) { + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + goto cleanup; + } else { + if (!(vhba_parent = getVhbaSCSIHostParent(conn, name))) + goto cleanup; + + if (virGetSCSIHostNumber(vhba_parent, &parent_host) < 0) + goto cleanup; + } + if (virManageVport(parent_host, adapter.data.fchost.wwpn, adapter.data.fchost.wwnn, VPORT_DELETE) < 0) goto cleanup; @@ -737,6 +789,7 @@ deleteVport(virStoragePoolSourceAdapter adapter) ret = 0; cleanup: VIR_FREE(name); + VIR_FREE(vhba_parent); return ret; }
<...snip...>