Previously, users had to manually specify a TCP port when starting
a pull-mode backup with an NBD server. A TODO comment in
qemuBackupPrepare() noted this limitation and pointed toward using
virPortAllocator, as done for migration, VNC, and SPICE ports.

Add backup_port_min and backup_port_max configuration options to
qemu.conf, defaulting to 10809-10872 (10809 is the IANA-assigned
NBD port; range of 64 matches the migration port pattern).

When a pull-mode backup is started without specifying a TCP port,
a port is now acquired automatically from the configured range via
virPortAllocatorAcquire(). The port is released when the backup
ends or if startup fails.

Signed-off-by: Lucas Amaral <[email protected]>
---
Build-tested and passed full test suite on CentOS Stream 9
(298 OK, 0 failures).

Runtime-validated against an actual patched virtqemud instance
running inside a privileged container (CentOS Stream 9, QEMU 10.1.0)
with a live aarch64/virt guest.

Config parsing validation:
  - Default config (no backup_port_* set): daemon starts OK
  - Custom range (backup_port_min=20000, backup_port_max=20010):
    daemon starts OK
  - Invalid (min > max): daemon correctly refuses to start
  - Invalid (min = 0): daemon correctly refuses to start
  - Invalid (max > 65535): daemon correctly refuses to start

Port auto-allocation (live VM backup):
  - Pull-mode backup WITHOUT port specified:
    backup-begin succeeds, backup-dumpxml shows auto-allocated
    port='10809' from default range.
    Previously this would fail with:
      "<domainbackup> must specify TCP port for now"

  - Pull-mode backup WITH explicit port=10809:
    backup-begin succeeds, backup-dumpxml shows port='10809'.
    Behavior unchanged from before the patch.

Port conflict and exhaustion (range set to 10809-10810, 3 VMs):
  - Auto-allocate on vm1 (got 10809), then manual port=10809
    on vm2: QEMU correctly rejects with "Address already in use"
  - Auto-allocate vm1=10809, vm2=10810 (range full), vm3 attempt:
    "Unable to find an unused port in range 'backup' (10809-10810)"
  - After aborting vm1 backup (releasing 10809), vm3 retries and
    gets 10809: port release and reuse works correctly

 docs/formatbackup.rst              |  3 ++-
 src/qemu/libvirtd_qemu.aug         |  2 ++
 src/qemu/qemu.conf.in              |  9 ++++++++
 src/qemu/qemu_backup.c             | 33 +++++++++++++++++++++---------
 src/qemu/qemu_conf.c               | 25 ++++++++++++++++++++++
 src/qemu/qemu_conf.h               |  6 ++++++
 src/qemu/qemu_domain.h             |  1 +
 src/qemu/qemu_driver.c             |  6 ++++++
 src/qemu/test_libvirtd_qemu.aug.in |  2 ++
 9 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst
index df6392e3bd..24857eaf72 100644
--- a/docs/formatbackup.rst
+++ b/docs/formatbackup.rst
@@ -42,7 +42,8 @@ were supplied). The following child elements and attributes 
are supported:
    ```protocol`` element of a disk 
<formatdomain.html#hard-drives-floppy-disks-cdroms>`__ attached
    via NBD in the domain (such as transport, socket, name, port, or tls),
    necessary to set up an NBD server that exposes the content of each disk at
-   the time the backup is started.
+   the time the backup is started. For TCP transport, if ``port`` is omitted, a
+   port is allocated automatically from the range configured in 
``/etc/libvirt/qemu.conf``.
 
    In addition to the above the NBD server used for backups allows using
    ``transport='fd' fdgroup='NAME'`` where ``NAME`` is the name used with
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 0286582169..eb790d48be 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -146,6 +146,8 @@ module Libvirtd_qemu =
                  | int_entry "migration_port_min"
                  | int_entry "migration_port_max"
                  | str_entry "migration_host"
+                 | int_entry "backup_port_min"
+                 | int_entry "backup_port_max"
 
    let log_entry = bool_entry "log_timestamp"
 
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index 2d1c67034d..5eacd70022 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -969,6 +969,15 @@
 #migration_port_max = 49215
 
 
+# Port range used for automatic allocation of NBD backup server ports.
+# When a pull-mode backup is started without specifying a TCP port, a
+# port from this range will be assigned automatically. The NBD standard
+# port is 10809.
+#
+#backup_port_min = 10809
+#backup_port_max = 10872
+
+
 # Timestamp QEMU's log messages (if QEMU supports it)
 #
 # Defaults to 1.
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 44514d08fc..12ef64b14e 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -32,6 +32,7 @@
 #include "storage_source.h"
 #include "storage_source_conf.h"
 #include "virerror.h"
+#include "virportallocator.h"
 #include "virlog.h"
 #include "virbuffer.h"
 #include "backup_conf.h"
@@ -71,16 +72,7 @@ qemuBackupPrepare(virDomainBackupDef *def)
 
         switch (def->server->transport) {
         case VIR_STORAGE_NET_HOST_TRANS_TCP:
-            /* TODO: Update qemu.conf to provide a port range,
-             * probably starting at 10809, for obtaining automatic
-             * port via virPortAllocatorAcquire, as well as store
-             * somewhere if we need to call virPortAllocatorRelease
-             * during BackupEnd. Until then, user must provide port */
-            if (!def->server->port) {
-                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                               _("<domainbackup> must specify TCP port for 
now"));
-                return -1;
-            }
+            /* port is auto-allocated if not set */
             break;
 
         case VIR_STORAGE_NET_HOST_TRANS_UNIX:
@@ -841,6 +833,16 @@ qemuBackupBegin(virDomainObj *vm,
     if (qemuBackupPrepare(def) < 0)
         goto endjob;
 
+    if (pull && def->server &&
+        def->server->transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
+        !def->server->port) {
+        unsigned short port = 0;
+        if (virPortAllocatorAcquire(priv->driver->backupPorts, &port) < 0)
+            goto endjob;
+        def->server->port = port;
+        priv->backupNBDPort = port;
+    }
+
     if (qemuBackupBeginPrepareTLS(vm, cfg, def, &tlsProps, &tlsSecretProps) < 
0)
         goto endjob;
 
@@ -969,6 +971,11 @@ qemuBackupBegin(virDomainObj *vm,
         qemuDomainObjExitMonitor(vm);
     }
 
+    if (ret < 0 && priv->backupNBDPort) {
+        virPortAllocatorRelease(priv->backupNBDPort);
+        priv->backupNBDPort = 0;
+    }
+
     if (ret < 0 && !job_started && priv->backup)
         def = g_steal_pointer(&priv->backup);
 
@@ -1026,6 +1033,12 @@ qemuBackupNotifyBlockjobEndStopNBD(virDomainObj *vm,
     qemuDomainObjExitMonitor(vm);
 
     backup->nbdStopped = true;
+
+    if (priv->backupNBDPort) {
+        virPortAllocatorRelease(priv->backupNBDPort);
+        priv->backupNBDPort = 0;
+        backup->server->port = 0;
+    }
 }
 
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index de6e51177a..99e6a29148 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -74,6 +74,9 @@ VIR_LOG_INIT("qemu.qemu_conf");
 #define QEMU_MIGRATION_PORT_MIN 49152
 #define QEMU_MIGRATION_PORT_MAX 49215
 
+#define QEMU_BACKUP_PORT_MIN 10809
+#define QEMU_BACKUP_PORT_MAX 10872
+
 VIR_ENUM_IMPL(virQEMUSchedCore,
               QEMU_SCHED_CORE_LAST,
               "none",
@@ -265,6 +268,9 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
     cfg->migrationPortMin = QEMU_MIGRATION_PORT_MIN;
     cfg->migrationPortMax = QEMU_MIGRATION_PORT_MAX;
 
+    cfg->backupPortMin = QEMU_BACKUP_PORT_MIN;
+    cfg->backupPortMax = QEMU_BACKUP_PORT_MAX;
+
     /* For privileged driver, try and find hugetlbfs mounts automatically.
      * Non-privileged driver requires admin to create a dir for the
      * user, chown it, and then let user configure it manually. */
@@ -985,6 +991,25 @@ virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfig 
*cfg,
         return -1;
     }
 
+    if (virConfGetValueUInt(conf, "backup_port_min", &cfg->backupPortMin) < 0)
+        return -1;
+    if (cfg->backupPortMin <= 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("%1$s: backup_port_min: port must be greater than 0"),
+                        filename);
+        return -1;
+    }
+
+    if (virConfGetValueUInt(conf, "backup_port_max", &cfg->backupPortMax) < 0)
+        return -1;
+    if (cfg->backupPortMax > 65535 ||
+        cfg->backupPortMax < cfg->backupPortMin) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                        _("%1$s: backup_port_max: port must be between the 
minimal port %2$d and 65535"),
+                       filename, cfg->backupPortMin);
+        return -1;
+    }
+
     if (virConfGetValueString(conf, "migration_host", &cfg->migrateHost) < 0)
         return -1;
     virStringStripIPv6Brackets(cfg->migrateHost);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c284e108a1..2e68f1de14 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -240,6 +240,9 @@ struct _virQEMUDriverConfig {
     unsigned int migrationPortMin;
     unsigned int migrationPortMax;
 
+    unsigned int backupPortMin;
+    unsigned int backupPortMax;
+
     bool logTimestamp;
     bool stdioLogD;
 
@@ -338,6 +341,9 @@ struct _virQEMUDriver {
     /* Immutable pointer, immutable object */
     virPortAllocatorRange *migrationPorts;
 
+    /* Immutable pointer, immutable object */
+    virPortAllocatorRange *backupPorts;
+
     /* Immutable pointer, lockless APIs */
     virSysinfoDef *hostsysinfo;
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 5755d2adb0..1153d8c6e0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -147,6 +147,7 @@ struct _qemuDomainObjPrivate {
     char *origname;
     int nbdPort; /* Port used for migration with NBD */
     unsigned short migrationPort;
+    unsigned short backupNBDPort;
     int preMigrationState;
     unsigned long long preMigrationMemlock; /* Original RLIMIT_MEMLOCK in case
                                                it was changed for the current
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c5eedeedfa..a8034b4f26 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -714,6 +714,12 @@ qemuStateInitialize(bool privileged,
                                   cfg->migrationPortMax)) == NULL)
         goto error;
 
+    if ((qemu_driver->backupPorts =
+         virPortAllocatorRangeNew(_("backup"),
+                                  cfg->backupPortMin,
+                                  cfg->backupPortMax)) == NULL)
+        goto error;
+
     if (qemuSecurityInit(qemu_driver) < 0)
         goto error;
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index 82cfec3b4b..2582c6a09c 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -110,6 +110,8 @@ module Test_libvirtd_qemu =
 { "migration_host" = "host.example.com" }
 { "migration_port_min" = "49152" }
 { "migration_port_max" = "49215" }
+{ "backup_port_min" = "10809" }
+{ "backup_port_max" = "10872" }
 { "log_timestamp" = "0" }
 { "nvram"
     { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" }
-- 
2.52.0

Reply via email to