Add a few debug statements to be able to trace lifetime of a reserved/allocated port. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virportallocator.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 6d6f99778e..70393d87ee 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -29,9 +29,12 @@ #include "virthread.h" #include "virerror.h" #include "virutil.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT("util.virportallocator"); + #define VIR_PORT_ALLOCATOR_NUM_PORTS 65536 typedef struct _virPortAllocator virPortAllocator; @@ -228,6 +231,8 @@ virPortAllocatorAcquire(const virPortAllocatorRange *range, return -1; } *port = i; + VIR_DEBUG("port='%u'", *port); + return 0; } } @@ -247,6 +252,8 @@ virPortAllocatorRelease(unsigned short port) if (!pa) return -1; + VIR_DEBUG("port='%u'", port); + if (!port) return 0; @@ -265,6 +272,8 @@ virPortAllocatorSetUsed(unsigned short port) if (!pa) return -1; + VIR_DEBUG("port='%u'", port); + if (!port) return 0; -- 2.43.0

Originally the migration code didn't register the NBD disk port with the port allocator when it was manually specified. Later when commit e74d627bb3bc2684cbe3 refactored the code and started registering the old logic which was clearing 'priv->nbdPort' in case when it was manually specified was not removed. This caused following problems: - the port was not released after successful migration - the port was released even when it was not allocated on failures regarding the NBD server start - the port was not released on other failures of the migration after NBD server startup To address this we remove the assumption that 'priv->nbdPort' is used only for auto-allocated port and fill it only once the port is allocated and make the caller of qemuMigrationDstStartNBDServer responsilbe for releasing it. Fixes: e74d627bb3bc2684cbe3edc1e2f7cc745b4e1ff3 Resolves: https://issues.redhat.com/browse/RHEL-21543 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 25dc16a9e9..182e51f0f0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -527,6 +527,8 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm, * arguments in 'migrate' monitor command. * Error is reported here. * + * Caller is responsible for releasing 'priv->nbdPort' from the port allocator. + * * Returns 0 on success, -1 otherwise. */ static int @@ -627,6 +629,9 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, server.port = port; } + + /* caller will release the port on failure */ + priv->nbdPort = server.port; } if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_MIGRATION_IN) < 0) @@ -643,14 +648,9 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, qemuDomainObjExitMonitor(vm); } - if (server.transport == VIR_STORAGE_NET_HOST_TRANS_TCP) - priv->nbdPort = server.port; - ret = 0; cleanup: - if (ret < 0) - virPortAllocatorRelease(server.port); return ret; exit_monitor: @@ -3261,11 +3261,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, virDomainAuditStart(vm, "migrated", false); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_MIGRATION_IN, stopFlags); - /* release if port is auto selected which is not the case if - * it is given in parameters - */ - if (nbdPort == 0) - virPortAllocatorRelease(priv->nbdPort); + virPortAllocatorRelease(priv->nbdPort); priv->nbdPort = 0; } goto cleanup; @@ -3425,11 +3421,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, if (autoPort) priv->migrationPort = port; - /* in this case port is not auto selected and we don't need to manage it - * anymore after cookie is baked - */ - if (nbdPort != 0) - priv->nbdPort = 0; + ret = 0; cleanup: -- 2.43.0

There's nothing under the 'cleanup:' label thus the whole code can be simplified. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 182e51f0f0..832cbd1c17 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -541,7 +541,6 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, const char *nbdURI, const char *tls_alias) { - int ret = -1; qemuDomainObjPrivate *priv = vm->privateData; size_t i; virStorageNetHostDef server = { @@ -610,22 +609,22 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("Cannot migrate empty or read-only disk %1$s"), disk->dst); - goto cleanup; + return -1; } if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) - goto cleanup; + return -1; if (!server_started && server.transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { if (server.port) { if (virPortAllocatorSetUsed(server.port) < 0) - goto cleanup; + return -1; } else { unsigned short port = 0; if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) - goto cleanup; + return -1; server.port = port; } @@ -635,7 +634,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, } if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_MIGRATION_IN) < 0) - goto cleanup; + return -1; if (!server_started) { if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) @@ -648,14 +647,11 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, qemuDomainObjExitMonitor(vm); } - ret = 0; - - cleanup: - return ret; + return 0; exit_monitor: qemuDomainObjExitMonitor(vm); - goto cleanup; + return -1; } -- 2.43.0