Re: [PATCH 2/3] qemu: migration: Properly handle reservation of manually specified NBD port
On Tue, Jan 16, 2024 at 04:25:03PM +0100, Peter Krempa wrote:
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.
s/registering/registering it, /
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.
*responsible
+++ b/src/qemu/qemu_migration.c
@@ -627,6 +629,9 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver,
server.port = port;
}
+
+ /* caller will release the port on failure */
+ priv->nbdPort = server.port;
The port is ultimately released even on success, though it will
happen further up the stack. Still, it feels like the comment would
be more accurate and consistent with the function's documentation if
you dropped the "on failure" part.
With the commit message fixed and the comment optionally tweaked,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization