[libvirt] [PATCH v2] virsh: Emit error for VSH_OT_DATA without VSH_OFLAG_REQ

11 Dec 2014

Commit 6b9964 enforces checking invalid use of VSH_OT_STRING with
VSH_OFLAG_REQ. This commit tries to do the same thing to stop using
VSH_OT_DATA without VSH_OFLAG_REQ and also fix existing misuse.

Signed-off-by: Hao Liu <hliu@redhat.com>
---
V2:
    Add missing fix of dommemstat --period option.

 tools/virsh-domain-monitor.c |  4 +--
 tools/virsh-domain.c         | 68 ++++++++++++++++++++++----------------------
 tools/virsh-network.c        |  6 ++--
 tools/virsh-pool.c           | 18 ++++++------
 tools/virsh-snapshot.c       | 20 ++++++-------
 tools/virsh.c                | 13 +++++++--
 6 files changed, 68 insertions(+), 61 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 259400f..ace2c4a 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -271,7 +271,7 @@ static const vshCmdOptDef opts_dommemstat[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "period",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("period in seconds to set collection")
     },
@@ -845,7 +845,7 @@ static const vshCmdOptDef opts_domblkstat[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "device",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_EMPTY_OK,
      .help = N_("block device")
     },
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 750411b..6733cfa 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -755,27 +755,27 @@ static const vshCmdOptDef opts_attach_interface[] = {
      .help = N_("source of network interface")
     },
     {.name = "target",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("target network name")
     },
     {.name = "mac",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("MAC address")
     },
     {.name = "script",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("script used to bridge network interface")
     },
     {.name = "model",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("model type")
     },
     {.name = "inbound",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("control domain's incoming traffics")
     },
     {.name = "outbound",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("control domain's outgoing traffics")
     },
     {.name = "persistent",
@@ -1749,7 +1749,7 @@ static const vshCmdOptDef opts_block_commit[] = {
      .help = N_("bandwidth limit in MiB/s")
     },
     {.name = "base",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("path of base file to commit into (default bottom of chain)")
     },
     {.name = "shallow",
@@ -1757,7 +1757,7 @@ static const vshCmdOptDef opts_block_commit[] = {
      .help = N_("use backing file of top as base")
     },
     {.name = "top",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("path of top file to commit from (default top of chain)")
     },
     {.name = "active",
@@ -1977,7 +1977,7 @@ static const vshCmdOptDef opts_block_copy[] = {
      .help = N_("fully-qualified path of source disk")
     },
     {.name = "dest",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("path of the copy to create")
     },
     {.name = "bandwidth",
@@ -2025,11 +2025,11 @@ static const vshCmdOptDef opts_block_copy[] = {
      .help = N_("with --wait, don't wait for cancel to finish")
     },
     {.name = "xml",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("filename containing XML description of the copy destination")
     },
     {.name = "format",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("format of the destination file")
     },
     {.name = "granularity",
@@ -2519,7 +2519,7 @@ static const vshCmdOptDef opts_block_pull[] = {
      .help = N_("bandwidth limit in MiB/s")
     },
     {.name = "base",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("path of backing file in chain for a partial pull")
     },
     {.name = "wait",
@@ -3046,11 +3046,11 @@ static const vshCmdOptDef opts_domiftune[] = {
      .help = N_("interface device (MAC Address)")
     },
     {.name = "inbound",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("control domain's incoming traffics")
     },
     {.name = "outbound",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("control domain's outgoing traffics")
     },
     {.name = "config",
@@ -3425,7 +3425,7 @@ static const vshCmdOptDef opts_undefine[] = {
      .help = N_("remove domain managed state file")
     },
     {.name = "storage",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("remove associated storage volumes (comma separated list of "
                 "targets or source paths) (see domblklist)")
     },
@@ -5019,7 +5019,7 @@ static const vshCmdOptDef opts_dump[] = {
      .help = N_("dump domain's memory only")
     },
     {.name = "format",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("specify the format of memory-only dump")
     },
     {.name = NULL}
@@ -5173,7 +5173,7 @@ static const vshCmdOptDef opts_screenshot[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "file",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("where to store the screenshot")
     },
     {.name = "screen",
@@ -6217,7 +6217,7 @@ static const vshCmdOptDef opts_vcpupin[] = {
      .help = N_("vcpu number")
     },
     {.name = "cpulist",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_EMPTY_OK,
      .help = N_("host cpu number(s) to set, or omit option to query")
     },
@@ -6498,7 +6498,7 @@ static const vshCmdOptDef opts_emulatorpin[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "cpulist",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_EMPTY_OK,
      .help = N_("host cpu number(s) to set, or omit option to query")
     },
@@ -7537,11 +7537,11 @@ static const vshCmdOptDef opts_metadata[] = {
      .help = N_("use an editor to change the metadata")
     },
     {.name = "key",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("key to be used as a namespace identifier"),
     },
     {.name = "set",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("new metadata to set"),
     },
     {.name = "remove",
@@ -8326,12 +8326,12 @@ static const vshCmdOptDef opts_numatune[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "mode",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("NUMA mode, one of strict, preferred and interleave \n"
                 "or a number from the virDomainNumatuneMemMode enum")
     },
     {.name = "nodeset",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("NUMA node selections to set")
     },
     {.name = "config",
@@ -8602,11 +8602,11 @@ static const vshCmdInfo info_qemu_monitor_event[] = {
 
 static const vshCmdOptDef opts_qemu_monitor_event[] = {
     {.name = "domain",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("filter by domain name, id or uuid")
     },
     {.name = "event",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("filter by event name")
     },
     {.name = "pretty",
@@ -9416,19 +9416,19 @@ static const vshCmdOptDef opts_migrate[] = {
      .help = N_("abort on soft errors during migration")
     },
     {.name = "migrateuri",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("migration URI, usually can be omitted")
     },
     {.name = "graphicsuri",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("graphics URI to be used for seamless graphics migration")
     },
     {.name = "listen-address",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("listen address that destination should bind to for incoming migration")
     },
     {.name = "dname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("rename to new name during migration (if supported)")
     },
     {.name = "timeout",
@@ -9914,7 +9914,7 @@ static const vshCmdOptDef opts_domdisplay[] = {
      .help = N_("includes the password into the connection URI if available")
     },
     {.name = "type",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("select particular graphical display "
                 "(e.g. \"vnc\", \"spice\", \"rdp\")")
     },
@@ -11761,11 +11761,11 @@ static const vshCmdInfo info_event[] = {
 
 static const vshCmdOptDef opts_event[] = {
     {.name = "domain",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("filter by domain name, id, or uuid")
     },
     {.name = "event",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("which event type to wait for")
     },
     {.name = "all",
@@ -11923,7 +11923,7 @@ static const vshCmdOptDef opts_change_media[] = {
      .help = N_("Fully-qualified path or target of disk device")
     },
     {.name = "source",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("source of the media")
     },
     {.name = "eject",
@@ -12074,7 +12074,7 @@ static const vshCmdOptDef opts_domfstrim[] = {
                 "free ranges smaller than this (Bytes)")
     },
     {.name = "mountpoint",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("which mount point to trim")
     },
     {.name = NULL}
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 9dc52e5..5f8743c 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1191,11 +1191,11 @@ static const vshCmdInfo info_network_event[] = {
 
 static const vshCmdOptDef opts_network_event[] = {
     {.name = "network",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("filter by network name or uuid")
     },
     {.name = "event",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("which event type to wait for")
     },
     {.name = "loop",
@@ -1305,7 +1305,7 @@ static const vshCmdOptDef opts_network_dhcp_leases[] = {
      .help = N_("network name or uuid")
     },
     {.name = "mac",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_NONE,
      .help = N_("MAC address")
     },
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index a9482c4..addbccc 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -200,23 +200,23 @@ static const vshCmdOptDef opts_pool_X_as[] = {
      .help = N_("print XML document, but don't define/create")
     },
     {.name = "source-host",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("source-host for underlying storage")
     },
     {.name = "source-path",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("source path for underlying storage")
     },
     {.name = "source-dev",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("source device for underlying storage")
     },
     {.name = "source-name",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("source name for underlying storage")
     },
     {.name = "target",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("target for underlying storage")
     },
     {.name = "source-format",
@@ -1399,15 +1399,15 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = {
      .help = N_("type of storage pool sources to find")
     },
     {.name = "host",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("optional host to query")
     },
     {.name = "port",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("optional port to query")
     },
     {.name = "initiator",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("optional initiator IQN to use for query")
     },
     {.name = NULL}
@@ -1489,7 +1489,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = {
      .help = N_("type of storage pool sources to discover")
     },
     {.name = "srcSpec",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("optional file of source xml to query for pools")
     },
     {.name = NULL}
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 9fe7751..1bb74a6 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -128,7 +128,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "xmlfile",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("domain snapshot XML")
     },
     {.name = "redefine",
@@ -333,11 +333,11 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "name",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("name of snapshot")
     },
     {.name = "description",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("description of snapshot")
     },
     {.name = "print-xml",
@@ -529,7 +529,7 @@ static const vshCmdOptDef opts_snapshot_edit[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("snapshot name")
     },
     {.name = "current",
@@ -658,7 +658,7 @@ static const vshCmdOptDef opts_snapshot_current[] = {
      .help = N_("include security sensitive information in XML dump")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("name of existing snapshot to make current")
     },
     {.name = NULL}
@@ -886,7 +886,7 @@ static const vshCmdOptDef opts_snapshot_info[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("snapshot name")
     },
     {.name = "current",
@@ -1491,7 +1491,7 @@ static const vshCmdOptDef opts_snapshot_list[] = {
      .help = N_("list snapshots in a tree")
     },
     {.name = "from",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("limit list to children of given snapshot")
     },
     {.name = "current",
@@ -1775,7 +1775,7 @@ static const vshCmdOptDef opts_snapshot_parent[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("find parent of snapshot name")
     },
     {.name = "current",
@@ -1843,7 +1843,7 @@ static const vshCmdOptDef opts_snapshot_revert[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("snapshot name")
     },
     {.name = "current",
@@ -1936,7 +1936,7 @@ static const vshCmdOptDef opts_snapshot_delete[] = {
      .help = N_("domain name, id or uuid")
     },
     {.name = "snapshotname",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("snapshot name")
     },
     {.name = "current",
diff --git a/tools/virsh.c b/tools/virsh.c
index 0ead9ae..aba34ce 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -435,7 +435,7 @@ static const vshCmdInfo info_connect[] = {
 
 static const vshCmdOptDef opts_connect[] = {
     {.name = "name",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_EMPTY_OK,
      .help = N_("hypervisor connection URI")
     },
@@ -594,7 +594,7 @@ static const vshCmdInfo info_help[] = {
 
 static const vshCmdOptDef opts_help[] = {
     {.name = "command",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("Prints global help, command specific help, or help for a group of related commands")
     },
     {.name = NULL}
@@ -854,7 +854,7 @@ static const vshCmdInfo info_cd[] = {
 
 static const vshCmdOptDef opts_cd[] = {
     {.name = "dir",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .help = N_("directory to switch to (default: home or else root)")
     },
     {.name = NULL}
@@ -1392,6 +1392,13 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
                     snprintf(buf, sizeof(buf), _("--%s <string>"), opt->name);
                     break;
                 case VSH_OT_DATA:
+                    /* OT_DATA should always be VSH_OFLAG_REQ */
+                    if (!(opt->flags & VSH_OFLAG_REQ)) {
+                        vshError(ctl,
+                                 _("internal error: bad options in command: '%s'"),
+                                 def->name);
+                        return false;
+                    }
                     snprintf(buf, sizeof(buf), _("[--%s] <string>"),
                              opt->name);
                     break;
-- 
1.8.3.1

    

Hao Liu

Ján Tomko

tags

participants (2)