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@xxxxxxxxxx> > --- > 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...> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list