From 1efb7944878d2a6610e358faef0fefbdb9174817 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 13 Mar 2024 22:52:12 +0100
Subject: [PATCH] virsh: Fix "positional" argument annotations for 'migrate'
 command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Annotate arguments which can be unintentionally parsed positionally.
(See previous commits for explanation.)

Annotate '--migrateuri', '--graphicsuri', '--listen-address', '-dname',
'--timeout', '--xml', '--migrate-disks' and '--disks port' as
'unwanted_positional'. These were declared in chronological order per
git history.

All others are annotated with VSH_OFLAG_REQ_OPT which makes the parser
require the '--optionname'. This is due to the fact that '--disks-uri'
was introduced later and put in front of others declared earlier
breaking the order they would be accepted, thus changing the behaviour
between versions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 tools/virsh-domain.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c8b0896eb6..a558afe273 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10653,26 +10653,31 @@ static const vshCmdOptDef opts_migrate[] = {
     },
     {.name = "migrateuri",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshCompleteEmpty,
      .help = N_("migration URI, usually can be omitted")
     },
     {.name = "graphicsuri",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshCompleteEmpty,
      .help = N_("graphics URI to be used for seamless graphics migration")
     },
     {.name = "listen-address",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshCompleteEmpty,
      .help = N_("listen address that destination should bind to for incoming migration")
     },
     {.name = "dname",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshCompleteEmpty,
      .help = N_("rename to new name during migration (if supported)")
     },
     {.name = "timeout",
      .type = VSH_OT_INT,
+     .unwanted_positional = true,
      .help = N_("run action specified by --timeout-* option (suspend by "
                 "default) if live migration exceeds timeout (in seconds)")
     },
@@ -10686,54 +10691,66 @@ static const vshCmdOptDef opts_migrate[] = {
     },
     {.name = "xml",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshCompletePathLocalExisting,
      .help = N_("filename containing updated XML for the target")
     },
     {.name = "migrate-disks",
      .type = VSH_OT_STRING,
+     .unwanted_positional = true,
      .completer = virshDomainMigrateDisksCompleter,
      .help = N_("comma separated list of disks to be migrated")
     },
     {.name = "disks-port",
      .type = VSH_OT_INT,
+     .unwanted_positional = true,
      .help = N_("port to use by target server for incoming disks migration")
     },
     {.name = "disks-uri",
      .type = VSH_OT_STRING,
+     .flags = VSH_OFLAG_REQ_OPT,
      .completer = virshCompleteEmpty,
      .help = N_("URI to use for disks migration (overrides --disks-port)")
     },
     {.name = "comp-methods",
      .type = VSH_OT_STRING,
+     .flags = VSH_OFLAG_REQ_OPT,
      .completer = virshDomainMigrateCompMethodsCompleter,
      .help = N_("comma separated list of compression methods to be used")
     },
     {.name = "comp-mt-level",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("compress level for multithread compression")
     },
     {.name = "comp-mt-threads",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("number of compression threads for multithread compression")
     },
     {.name = "comp-mt-dthreads",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("number of decompression threads for multithread compression")
     },
     {.name = "comp-xbzrle-cache",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("page cache size for xbzrle compression")
     },
     {.name = "auto-converge-initial",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("initial CPU throttling rate for auto-convergence")
     },
     {.name = "auto-converge-increment",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("CPU throttling rate increment for auto-convergence")
     },
     {.name = "persistent-xml",
      .type = VSH_OT_STRING,
+     .flags = VSH_OFLAG_REQ_OPT,
      .completer = virshCompletePathLocalExisting,
      .help = N_("filename containing updated persistent XML for the target")
     },
@@ -10743,31 +10760,38 @@ static const vshCmdOptDef opts_migrate[] = {
     },
     {.name = "postcopy-bandwidth",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("post-copy migration bandwidth limit in MiB/s")
     },
     {.name = "parallel",
      .type = VSH_OT_BOOL,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("enable parallel migration")
     },
     {.name = "parallel-connections",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("number of connections for parallel migration")
     },
     {.name = "bandwidth",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("migration bandwidth limit in MiB/s")
     },
     {.name = "tls-destination",
      .type = VSH_OT_STRING,
+     .flags = VSH_OFLAG_REQ_OPT,
      .completer = virshCompleteEmpty,
      .help = N_("override the destination host name used for TLS verification")
     },
     {.name = "comp-zlib-level",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("compress level for zlib compression")
     },
     {.name = "comp-zstd-level",
      .type = VSH_OT_INT,
+     .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("compress level for zstd compression")
     },
     {.name = NULL}