July 2017 - Devel - libvirt List Archives

[libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing
by John Ferlan 25 Jul '17

25 Jul '17

Disallow providing the wwnn/wwpn of the HBA in the adapter XML: <adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn' wwpn='HBA_wwpn'/> This should be considered a configuration error since a vHBA would not be created. In order to use the HBA as the backing the following XML should be used: <adapter type='scsi_host' name='scsi_hostN'/> So add a check prior to the checkParent call to validate that the provided wwnn/wwpn resolves to a vHBA and not an HBA. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- This used to be patch 4 of the series: https://www.redhat.com/archives/libvir-list/2017-July/msg00837.html which while the series was in it's 3rd review cycle, this particular patch was new to the series and thus becomes a v2 since the three other patches from the series were pushed. docs/formatstorage.html.in | 27 +++++++++-------- src/storage/storage_backend_scsi.c | 62 +++++++++++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 4946ddf..27578e8 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -207,18 +207,21 @@ </dl> <dl> <dt><code>wwnn</code> and <code>wwpn</code></dt> - <dd>The "World Wide Node Name" (<code>wwnn</code>) and "World Wide - Port Name" (<code>wwpn</code>) are used by the "fc_host" adapter - to uniquely identify the device in the Fibre Channel storage fabric - (the device can be either a HBA or vHBA). Both wwnn and wwpn should - be specified. Use the command 'virsh nodedev-dumpxml' to determine - how to set the values for the wwnn/wwpn of a (v)HBA. The wwnn and - wwpn have very specific numerical format requirements based on the - hypervisor being used, thus care should be taken if you decide to - generate your own to follow the standards; otherwise, the pool - will fail to start with an opaque error message indicating failure - to write to the vport_create file during vport create/delete due - to "No such file or directory". + <dd>The required "World Wide Node Name" (<code>wwnn</code>) and + "World Wide Port Name" (<code>wwpn</code>) are used by the + "fc_host" adapter to uniquely identify the vHBA device in the + Fibre Channel storage fabric. If the vHBA device already exists + as a Node Device, then libvirt will use it; otherwise, the vHBA + will be created using the provided values. It is considered a + configuration error use the values from the HBA as those would + be for a "scsi_host" <code>type</code> pool instead. The + <code>wwnn</code> and <code>wwpn</code> have very specific + format requirements based on the hypervisor being used, thus + care should be taken if you decide to generate your own to + follow the standards; otherwise, the pool will fail to start + with an opaque error message indicating failure to write to + the vport_create file during vport create/delete due to + "No such file or directory". <span class="since">Since 1.0.4</span> </dd> </dl> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index af12889..8892822 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -211,21 +211,66 @@ getAdapterName(virStorageAdapterPtr adapter) } +/** + * @name: Name from a wwnn/wwpn lookup + * + * Validate that the @name fetched from the wwnn/wwpn is a vHBA and not + * an HBA as that should be a configuration error. It's only possible to + * use an existing wwnn/wwpn of a vHBA because that's what someone would + * have created using the node device create functionality. Using the + * HBA "just because" it has a wwnn/wwpn and the characteristics of a + * vHBA is just not valid + * + * Returns the @scsi_host_name to be used or NULL on errror + */ +static char * +checkName(const char *name) +{ + char *scsi_host_name = NULL; + unsigned int host_num; + + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + return NULL; + + if (virSCSIHostGetNumber(scsi_host_name, &host_num) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("host name '%s' is not properly formatted"), + name); + goto error; + } + + /* If scsi_host_name is vport capable, then it's an HBA. This is + * a configuration error as the wwnn/wwpn should only be for a vHBA */ + if (virVHBAIsVportCapable(NULL, host_num)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the wwnn/wwpn for '%s' are assigned to an HBA"), + scsi_host_name); + goto error; + } + + return scsi_host_name; + + error: + VIR_FREE(scsi_host_name); + return NULL; +} + + /* * Using the host# name found via wwnn/wwpn lookup in the fc_host * sysfs tree to get the parent 'scsi_host#' to ensure it matches. */ static bool checkParent(virConnectPtr conn, - const char *name, + const char *scsi_host_name, const char *parent_name) { unsigned int host_num; - char *scsi_host_name = NULL; char *vhba_parent = NULL; bool retval = false; - VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name); + VIR_DEBUG("conn=%p, scsi_host_name=%s, parent_name=%s", + conn, scsi_host_name, parent_name); /* autostarted pool - assume we're OK */ if (!conn) @@ -245,9 +290,6 @@ checkParent(virConnectPtr conn, goto cleanup; } - if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) - goto cleanup; - if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) goto cleanup; @@ -255,7 +297,7 @@ checkParent(virConnectPtr conn, virReportError(VIR_ERR_XML_ERROR, _("Parent attribute '%s' does not match parent '%s' " "determined for the '%s' wwnn/wwpn lookup."), - parent_name, vhba_parent, name); + parent_name, vhba_parent, scsi_host_name); goto cleanup; } @@ -263,7 +305,6 @@ checkParent(virConnectPtr conn, cleanup: VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); return retval; } @@ -275,6 +316,7 @@ createVport(virConnectPtr conn, virStorageAdapterFCHostPtr fchost) { char *name = NULL; + char *scsi_host_name = NULL; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; int ret = -1; @@ -288,6 +330,9 @@ createVport(virConnectPtr conn, * this pool and we don't have to create the vHBA */ if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { + if (!(scsi_host_name = checkName(name))) + goto cleanup; + /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent. If not this will cause failure. */ if (!fchost->parent || checkParent(conn, name, fchost->parent)) @@ -333,6 +378,7 @@ createVport(virConnectPtr conn, cleanup: VIR_FREE(name); + VIR_FREE(scsi_host_name); return ret; } -- 2.9.4

2 5

[libvirt] [PATCH 0/4] Clean up machineName-related code
by Martin Kletzander 25 Jul '17

25 Jul '17

Just in case we'll want to use the "cleaned-up" machineName for anything else in the future, I decided to modify the code so that it is always generated, saved and cleaned-up. While doing that I identified some code duplication, leak and a TODO/XXX comment that are all fixed with this series. And it only adds four lines overall! Martin Kletzander (4): conf: Pass config.priv to xmlopt->privateData.alloc qemu: Save qemu driver in qemuDomainObjPrivateData lxc: Make lxcProcessStop callable even without PID being available Move machineName generation from virsystemd into domain_conf src/bhyve/bhyve_domain.c | 2 +- src/conf/domain_conf.c | 65 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 7 ++++- src/libvirt_private.syms | 2 +- src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_cgroup.c | 5 +--- src/lxc/lxc_domain.c | 21 ++++++++++++++- src/lxc/lxc_domain.h | 3 +++ src/lxc/lxc_process.c | 57 +++++++++++----------------------------- src/qemu/qemu_cgroup.c | 24 ++++------------- src/qemu/qemu_domain.c | 24 ++++++++++++++++- src/qemu/qemu_domain.h | 5 ++++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 11 +++++--- src/uml/uml_driver.c | 2 +- src/util/vircgroup.c | 15 ++++------- src/util/vircgroup.h | 14 +++++----- src/util/virsystemd.c | 62 ------------------------------------------- src/util/virsystemd.h | 5 ---- src/vmware/vmware_driver.c | 2 +- src/vz/vz_utils.c | 2 +- tests/virsystemdtest.c | 4 +-- 22 files changed, 170 insertions(+), 166 deletions(-) -- 2.13.3

2 9

[libvirt] [PATCH v3 0/3] More virsecretobj changes to prepare for common object
by John Ferlan 25 Jul '17

25 Jul '17

v2: https://www.redhat.com/archives/libvir-list/2017-July/msg00520.html Changes in v3: Former patch 1, dropped Former patch 2, pushed Former patch 3, adjusted: -> Use @objDef instead of @objdef -> Handle the if (backup) leak shown during review Former patch 4, split -> Patch 3 handles the virsecretobj call to virSecretObjListRemove -> Patch 4 handles the secret_driver calls to virSecretObjListRemove John Ferlan (3): secret: Properly handle @def after virSecretObjAdd in driver secret: Fix memory leak in virSecretLoad secret: Handle object list removal and deletion properly src/conf/virsecretobj.c | 14 ++++++-------- src/secret/secret_driver.c | 31 ++++++++++++++++++------------- 2 files changed, 24 insertions(+), 21 deletions(-) -- 2.9.4

2 4

[libvirt] [PATCH v2 0/4] More virsecretobj changes to prepare for common object
by John Ferlan 25 Jul '17

25 Jul '17

v1: https://www.redhat.com/archives/libvir-list/2017-June/msg00168.html but review was done more recently in 2017-July... Changes since v1... ... Pushed v1 patches 1-3 since they were ACK'd ... Altered old patch 4, new patch 1 to use if/else processing ... Altered old patch 5, new patch 2 to account for merge conflict (old patch 5 was ACK'd) ... Dropped old patch 6 ... Altered old patch 7, new patch 3 to use VIR_STEAL_PTR and updated the commit message to also describe the @def usage problem ... Altered old patch 8, new patch 4 to change the commit message to have more details John Ferlan (4): secret: Clean up virSecretObjListAdd processing secret: Remove need for local configFile and base64File in ObjectAdd secret: Properly handle @def after virSecretObjAdd in driver secret: Handle object list removal and deletion properly src/conf/virsecretobj.c | 53 ++++++++++++++++++++-------------------------- src/secret/secret_driver.c | 28 ++++++++++++++---------- 2 files changed, 40 insertions(+), 41 deletions(-) -- 2.9.4

2 15

[libvirt] [PATCH] Revert "virthread: Introduce virRWLockInitPreferWriter"
by Michal Privoznik 25 Jul '17

25 Jul '17

This reverts commit 328bd24443d2a345a5832ee48ebba0208f8036ea. As it turns out, this is not portable and very Linux & glibc specific. Worse, this may lead to not starving writers on Linux but everywhere else. Revert this and if the starvation occurs resolve it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libvirt_private.syms | 1 - src/util/virobject.c | 2 +- src/util/virthread.c | 35 ----------------------------------- src/util/virthread.h | 1 - 4 files changed, 1 insertion(+), 38 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3cc6f4c6e..d98417678 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2733,7 +2733,6 @@ virMutexUnlock; virOnce; virRWLockDestroy; virRWLockInit; -virRWLockInitPreferWriter; virRWLockRead; virRWLockUnlock; virRWLockWrite; diff --git a/src/util/virobject.c b/src/util/virobject.c index 4236abfef..b1bb378b4 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -275,7 +275,7 @@ virObjectRWLockableNew(virClassPtr klass) if (!(obj = virObjectNew(klass))) return NULL; - if (virRWLockInitPreferWriter(&obj->lock) < 0) { + if (virRWLockInit(&obj->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to initialize RW lock")); virObjectUnref(obj); diff --git a/src/util/virthread.c b/src/util/virthread.c index a8dd72f8b..6c495158f 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -95,15 +95,6 @@ void virMutexUnlock(virMutexPtr m) } -/** - * virRWLockInit: - * @m: rwlock to init - * - * Initializes RW lock using pthread default attributes (which - * is PTHREAD_RWLOCK_PREFER_READER_NP). - * - * Returns 0 on success, -1 otherwise. - */ int virRWLockInit(virRWLockPtr m) { int ret; @@ -115,32 +106,6 @@ int virRWLockInit(virRWLockPtr m) return 0; } - -/** - * virRWLockInitPreferWriter: - * @m: rwlock to init - * - * Initializes RW lock which prefers writers over readers. - * - * Returns 0 on success, -1 otherwise. - */ -int virRWLockInitPreferWriter(virRWLockPtr m) -{ - int ret; - pthread_rwlockattr_t attr; - - pthread_rwlockattr_init(&attr); - pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP); - ret = pthread_rwlock_init(&m->lock, &attr); - pthread_rwlockattr_destroy(&attr); - if (ret != 0) { - errno = ret; - return -1; - } - return 0; -} - - void virRWLockDestroy(virRWLockPtr m) { pthread_rwlock_destroy(&m->lock); diff --git a/src/util/virthread.h b/src/util/virthread.h index 18b785af2..e466d9bf0 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -136,7 +136,6 @@ void virMutexUnlock(virMutexPtr m); int virRWLockInit(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; -int virRWLockInitPreferWriter(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; void virRWLockDestroy(virRWLockPtr m); void virRWLockRead(virRWLockPtr m); -- 2.13.0

2 1

[libvirt] [PATCH] Generate unique virtio-serial socket file names
by seg＠us.ibm.com 25 Jul '17

25 Jul '17

It's possible to have more than one unix channel with type=virtio-serial. We need to generate a unique name for each channel. Currently, we use "unknown.sock" for all of them. Signed-off-by: Scott Garfinkle <seg(a)us.ibm.com>

2 2

[libvirt] [PATCH] qemu: fix migration fail of an auto-placement vm after attached memory to it
by Yi Wang 24 Jul '17

24 Jul '17

This patch fix this condition: -vm has the "auto" placement in vcpu -hot-plug memory with source node "1-3" through attach-device command -migrate the vm to a host with only 2 numa node And the migration will fail with error: "error: unsupported configuration: NUMA node 2 is unavailable" Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn> Signed-off-by: Xi Xu <xu.xi8(a)zte.com.cn> --- src/qemu/qemu_process.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7b708be..dcc564c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5259,6 +5259,16 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; } + VIR_DEBUG("Updating memory source nodes"); + for (i = 0; i < vm->def->nmems; i++) { + virDomainMemoryDefPtr mem = vm->def->mems[i]; + if (priv->autoNodeset && mem && mem->sourceNodes) { + virBitmapFree(mem->sourceNodes); + if (!(mem->sourceNodes = virBitmapNewCopy(priv->autoNodeset))) + goto cleanup; + } + } + /* Whether we should use virtlogd as stdio handler for character * devices source backend. */ if (cfg->stdioLogD && -- 1.8.3.1

4 4

[libvirt] [PATCH v3 0/4] Fix a couple issues found w/ vHBA logic
by John Ferlan 24 Jul '17

24 Jul '17

v2: https://www.redhat.com/archives/libvir-list/2017-July/msg00661.html Changes since v2: * Patch 1 - make the requested logic adjustment * Patch 2 - no change * Patch 3 - make this *just* the check that the provided @parent_name is a valid fc_host (whether it's vHBA or HBA). * Patch 4 - Alter the checkParent logic to cause a failure if the passed @name (from the wwnn/wwpn's provided) are for an HBA instead of a vHBA. Make this an invalid configuration. Modify the docs to describe that the provided wwnn/wwpn must be for an existing vHBA, are to be used to create a vHBA, but cannot be from an existing HBA. NB: No change to the Destroy code since it's possible one of these exists and there needs to be a way to shut it down. The failure could turn into a "conversion" of sorts as well by changing the type to 'scsi_host', saving the generated scsi_host name, removing the parent and wwnn & wwpn fields. Tested using my vHBA system with various valid and invalid configurations. John Ferlan (4): storage: Fix existing parent check for vHBA creation storage: Remove @conn from virNodeDeviceCreateVport storage: Check if provided parent is vHBA capable storage: Disallow usage of the HBA for a fc_host backing docs/formatstorage.html.in | 27 ++++++----- src/conf/node_device_conf.c | 63 ++----------------------- src/conf/node_device_conf.h | 3 +- src/storage/storage_backend_scsi.c | 94 +++++++++++++++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 75 deletions(-) -- 2.9.4

2 7

[libvirt] [PATCH v2 0/2] Fix backwards migration of pSeries guests
by Andrea Bolognani 24 Jul '17

24 Jul '17

Changes from [v1]: * don't hide the controller model check inside a function and propely explain the reason not to format it; * rename a confusingly named function. [v1] https://www.redhat.com/archives/libvir-list/2017-July/msg00705.html Andrea Bolognani (2): conf: Fix backwards migration of pSeries guests conf: Rename virDomainControllerIsPCIHostBridge() to IsPSeriesPHB() src/conf/domain_conf.c | 29 ++++++++++++++++++++++++++--- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 2 +- 6 files changed, 31 insertions(+), 8 deletions(-) -- 2.7.5

2 4

[libvirt] [PATCH v2 0/3] implement migrate-getmaxdowntime command
by seg＠us.ibm.com 24 Jul '17

24 Jul '17

Currently, the maximum tolerable downtime for a domain being migrated is write-only. This patch implements a way to query that value nondestructively. Changes from [v1]: * Fixed formatting and style problems * Add additional JSON error check * Better patch submission mail, I hope [v1} https://www.redhat.com/archives/libvir-list/2017-July/msg00908.html Scott Garfinkle (3): migrate-getmaxdowntime command qemu side migrate-getmaxdowntime public symbols and table indices migrate-getmaxdowntime local/libvirt enablement src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 40 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ src/libvirt_public.syms | 4 ++++ src/remote/remote_protocol.x | 16 +++++++++++++++- src/remote_protocol-structs | 8 ++++++++ include/libvirt/libvirt-domain.h | 4 ++++ src/driver-hypervisor.h | 6 ++++++ src/libvirt-domain.c | 45 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 12 +++++++++++ src/remote/remote_driver.c | 1 + tools/virsh-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 18 ++++++++++++++++ 14 files changed, 240 insertions(+), 1 deletion(-)

2 4