[PATCH 3/4] virsh: doMigrate: Rework virsh option to migration flag conversion

Peter Krempa posted 4 patches 2 years, 6 months ago
[PATCH 3/4] virsh: doMigrate: Rework virsh option to migration flag conversion
Posted by Peter Krempa 2 years, 6 months ago
Convert the flags declaratively as in the vast majority of cases it's a
simple binary addition if the flag exists.

In one instance there was also an additional check, which was moved up
after the new code, and the error message was fixed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tools/virsh-domain.c | 108 +++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 66 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f8758f18a3..84bf62057b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11082,6 +11082,11 @@ static const vshCmdOptDef opts_migrate[] = {
     {.name = NULL}
 };

+struct doMigrateFlagMapping {
+    const char *optionname;
+    unsigned int migflag;
+};
+
 static void
 doMigrate(void *opaque)
 {
@@ -11099,6 +11104,32 @@ doMigrate(void *opaque)
     unsigned long long ullOpt = 0;
     int rv;
     virConnectPtr dconn = data->dconn;
+    size_t i;
+
+    static const struct doMigrateFlagMapping flagmap[] = {
+        { "live", VIR_MIGRATE_LIVE },
+        { "p2p", VIR_MIGRATE_PEER2PEER },
+        { "tunnelled", VIR_MIGRATE_TUNNELLED },
+        { "persistent", VIR_MIGRATE_PERSIST_DEST },
+        { "undefinesource", VIR_MIGRATE_UNDEFINE_SOURCE },
+        { "copy-storage-all", VIR_MIGRATE_NON_SHARED_DISK },
+        { "copy-storage-inc", VIR_MIGRATE_NON_SHARED_INC },
+        { "copy-storage-synchronous-writes", VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES },
+        { "change-protection", VIR_MIGRATE_CHANGE_PROTECTION },
+        { "unsafe", VIR_MIGRATE_UNSAFE },
+        { "compressed", VIR_MIGRATE_COMPRESSED },
+        { "auto-converge", VIR_MIGRATE_AUTO_CONVERGE },
+        { "rdma-pin-all", VIR_MIGRATE_RDMA_PIN_ALL },
+        { "offline", VIR_MIGRATE_OFFLINE },
+        { "abort-on-error", VIR_MIGRATE_ABORT_ON_ERROR },
+        { "postcopy", VIR_MIGRATE_POSTCOPY },
+        { "postcopy-resume", VIR_MIGRATE_POSTCOPY_RESUME },
+        { "zerocopy", VIR_MIGRATE_ZEROCOPY },
+        { "tls", VIR_MIGRATE_TLS },
+        { "parallel", VIR_MIGRATE_PARALLEL },
+        { "suspend", VIR_MIGRATE_PAUSED },
+    };
+
 #ifndef WIN32
     sigset_t sigmask, oldsigmask;

@@ -11108,6 +11139,17 @@ doMigrate(void *opaque)
         goto out_sig;
 #endif /* !WIN32 */

+    for (i = 0; i < G_N_ELEMENTS(flagmap); i++) {
+        if (vshCommandOptBool(cmd, flagmap[i].optionname))
+            flags |= flagmap[i].migflag;
+    }
+
+    if (flags & VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES &&
+        !(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK))) {
+        vshError(ctl, "'--copy-storage-synchronous-writes' requires one of '--copy-storage-all', '--copy-storage-inc'");
+        goto out;
+    }
+
     if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
         goto out;

@@ -11325,72 +11367,6 @@ doMigrate(void *opaque)
                                 VIR_MIGRATE_PARAM_TLS_DESTINATION, opt) < 0)
         goto save_error;

-    if (vshCommandOptBool(cmd, "live"))
-        flags |= VIR_MIGRATE_LIVE;
-    if (vshCommandOptBool(cmd, "p2p"))
-        flags |= VIR_MIGRATE_PEER2PEER;
-    if (vshCommandOptBool(cmd, "tunnelled"))
-        flags |= VIR_MIGRATE_TUNNELLED;
-
-    if (vshCommandOptBool(cmd, "persistent"))
-        flags |= VIR_MIGRATE_PERSIST_DEST;
-    if (vshCommandOptBool(cmd, "undefinesource"))
-        flags |= VIR_MIGRATE_UNDEFINE_SOURCE;
-
-    if (vshCommandOptBool(cmd, "suspend"))
-        flags |= VIR_MIGRATE_PAUSED;
-
-    if (vshCommandOptBool(cmd, "copy-storage-all"))
-        flags |= VIR_MIGRATE_NON_SHARED_DISK;
-
-    if (vshCommandOptBool(cmd, "copy-storage-inc"))
-        flags |= VIR_MIGRATE_NON_SHARED_INC;
-
-    if (vshCommandOptBool(cmd, "copy-storage-synchronous-writes")) {
-        if (!(flags & VIR_MIGRATE_NON_SHARED_DISK) &&
-            !(flags & VIR_MIGRATE_NON_SHARED_INC)) {
-            vshError(ctl, "'--copy-storage-synchronous-writes' requires one of '--copy-storage-all', 'copy-storage-inc'");
-            goto out;
-        }
-        flags |= VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES;
-    }
-
-    if (vshCommandOptBool(cmd, "change-protection"))
-        flags |= VIR_MIGRATE_CHANGE_PROTECTION;
-
-    if (vshCommandOptBool(cmd, "unsafe"))
-        flags |= VIR_MIGRATE_UNSAFE;
-
-    if (vshCommandOptBool(cmd, "compressed"))
-        flags |= VIR_MIGRATE_COMPRESSED;
-
-    if (vshCommandOptBool(cmd, "auto-converge"))
-        flags |= VIR_MIGRATE_AUTO_CONVERGE;
-
-    if (vshCommandOptBool(cmd, "rdma-pin-all"))
-        flags |= VIR_MIGRATE_RDMA_PIN_ALL;
-
-    if (vshCommandOptBool(cmd, "offline"))
-        flags |= VIR_MIGRATE_OFFLINE;
-
-    if (vshCommandOptBool(cmd, "abort-on-error"))
-        flags |= VIR_MIGRATE_ABORT_ON_ERROR;
-
-    if (vshCommandOptBool(cmd, "postcopy"))
-        flags |= VIR_MIGRATE_POSTCOPY;
-
-    if (vshCommandOptBool(cmd, "postcopy-resume"))
-        flags |= VIR_MIGRATE_POSTCOPY_RESUME;
-
-    if (vshCommandOptBool(cmd, "zerocopy"))
-        flags |= VIR_MIGRATE_ZEROCOPY;
-
-    if (vshCommandOptBool(cmd, "tls"))
-        flags |= VIR_MIGRATE_TLS;
-
-    if (vshCommandOptBool(cmd, "parallel"))
-        flags |= VIR_MIGRATE_PARALLEL;
-
     if (flags & VIR_MIGRATE_PEER2PEER || vshCommandOptBool(cmd, "direct")) {
         if (virDomainMigrateToURI3(dom, desturi, params, nparams, flags) == 0)
             data->ret = 0;
-- 
2.41.0
Re: [PATCH 3/4] virsh: doMigrate: Rework virsh option to migration flag conversion
Posted by Ján Tomko 2 years, 6 months ago
On a Wednesday in 2023, Peter Krempa wrote:
>Convert the flags declaratively as in the vast majority of cases it's a
>simple binary addition if the flag exists.
>
>In one instance there was also an additional check, which was moved up
>after the new code, and the error message was fixed.
>

I don't see the difference between the old and new messages.

>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> tools/virsh-domain.c | 108 +++++++++++++++++--------------------------
> 1 file changed, 42 insertions(+), 66 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano