On 09/14/2018 04:28 PM, Mark Kanda wrote:
Hi all,
I recently discovered, for 'single-use' command line options,
virt-install silently ignores all but the last definition. For example,
if the command line has '--memory X' and later '--memory Y', then 'X' is
silently ignored.
I think virt-install should warn the user if 'single use' options are
specified multiple times, and I'd like to implement a fix.
However, before I implement such a fix, I'd like to know if this is 'by
design', or if anyone would otherwise object to such a check..
Sorry for the late response. This isn't really intentional and more just
a side effect of how our argparse options are initialized. I hacked up
the attached diff that will essentially merge all ex. --memory options,
so --memory 123,maxmem=456 --memory 555 will be equivalent to --memory
555,maxmem=456. Is that fine for you needs?
I think it's a bit nicer and has benefits if you want to pass in an
extra option to a virt-install script or similar
Thanks,
Cole
>From 25eb5e1a50ae136511c43dc2e0ae50786bdc169c Mon Sep 17 00:00:00 2001
Message-Id: <25eb5e1a50ae136511c43dc2e0ae50786bdc169c.1538414182.git.crobinso@xxxxxxxxxx>
From: Cole Robinson <crobinso@xxxxxxxxxx>
Date: Mon, 1 Oct 2018 13:14:43 -0400
Subject: [PATCH virt-manager] cli: Merge multiple instances of single use opts
So --memory 123,maxmem=456 --memory 555 is equivalent to
--memory 555,maxmem=45
Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
---
virt-install | 17 +++++++++--------
virtinst/cli.py | 33 +++++++++++++++++----------------
2 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/virt-install b/virt-install
index 0415151f..7dc691f7 100755
--- a/virt-install
+++ b/virt-install
@@ -103,9 +103,9 @@ def convert_old_init(options):
if not options.init:
return
if not options.boot:
- options.boot = ""
- options.boot += ",init=%s" % options.init
- logging.debug("Converted old --init to --boot %s", options.boot)
+ options.boot = [""]
+ options.boot[-1] += ",init=%s" % options.init
+ logging.debug("Converted old --init to --boot %s", options.boot[-1])
def _do_convert_old_disks(options):
@@ -192,9 +192,9 @@ def convert_old_cpuset(options):
if not options.cpuset:
return
if not options.vcpus:
- options.vcpus = ""
- options.vcpus += ",cpuset=%s" % options.cpuset
- logging.debug("Generated compat cpuset: --vcpus %s", options.vcpus)
+ options.vcpus = [""]
+ options.vcpus[-1] += ",cpuset=%s" % options.cpuset
+ logging.debug("Generated compat cpuset: --vcpus %s", options.vcpus[-1])
def convert_old_networks(options):
@@ -286,7 +286,7 @@ def convert_old_graphics(options):
def convert_old_features(options):
- if getattr(options, "features", None):
+ if options.features:
return
opts = ""
@@ -296,7 +296,8 @@ def convert_old_features(options):
if opts:
opts += ","
opts += "apic=off"
- options.features = opts or None
+ if opts:
+ options.features = [opts]
##################################
diff --git a/virtinst/cli.py b/virtinst/cli.py
index 1d5c8a71..96289a66 100644
--- a/virtinst/cli.py
+++ b/virtinst/cli.py
@@ -536,7 +536,7 @@ def add_misc_options(grp, prompt=False, replace=False,
"create devices or define the guest."))
if prompt:
- grp.add_argument("--check",
+ grp.add_argument("--check", action="append",
help=_("Enable or disable validation checks. Example:\n"
"--check path_in_use=off\n"
"--check all=off"))
@@ -547,14 +547,14 @@ def add_misc_options(grp, prompt=False, replace=False,
def add_metadata_option(grp):
- grp.add_argument("--metadata",
+ grp.add_argument("--metadata", action="append",
help=_("Configure guest metadata. Ex:\n"
"--metadata name=foo,title=\"My pretty title\",uuid=...\n"
"--metadata description=\"My nice long description\""))
def add_memory_option(grp, backcompat=False):
- grp.add_argument("--memory",
+ grp.add_argument("--memory", action="append",
help=_("Configure guest memory allocation. Ex:\n"
"--memory 1024 (in MiB)\n"
"--memory 512,maxmemory=1024\n"
@@ -566,7 +566,7 @@ def add_memory_option(grp, backcompat=False):
def vcpu_cli_options(grp, backcompat=True, editexample=False):
- grp.add_argument("--vcpus",
+ grp.add_argument("--vcpus", action="append",
help=_("Number of vcpus to configure for your guest. Ex:\n"
"--vcpus 5\n"
"--vcpus 5,maxcpus=10,cpuset=1-4,6,8\n"
@@ -575,7 +575,7 @@ def vcpu_cli_options(grp, backcompat=True, editexample=False):
extramsg = "--cpu host"
if editexample:
extramsg = "--cpu host-model,clearxml=yes"
- grp.add_argument("--cpu",
+ grp.add_argument("--cpu", action="append",
help=_("CPU model and features. Ex:\n"
"--cpu coreduo,+x2apic\n"
"--cpu host-passthrough\n") + extramsg)
@@ -676,9 +676,9 @@ def add_device_options(devg, sound_back_compat=False):
def add_guest_xml_options(geng):
geng.add_argument("--security", action="append",
help=_("Set domain security driver configuration."))
- geng.add_argument("--cputune",
+ geng.add_argument("--cputune", action="append",
help=_("Tune CPU parameters for the domain process."))
- geng.add_argument("--numatune",
+ geng.add_argument("--numatune", action="append",
help=_("Tune NUMA policy for the domain process."))
geng.add_argument("--memtune", action="append",
help=_("Tune memory policy for the domain process."))
@@ -687,16 +687,16 @@ def add_guest_xml_options(geng):
geng.add_argument("--memorybacking", action="append",
help=_("Set memory backing policy for the domain process. Ex:\n"
"--memorybacking hugepages=on"))
- geng.add_argument("--features",
+ geng.add_argument("--features", action="append",
help=_("Set domain <features> XML. Ex:\n"
"--features acpi=off\n"
"--features apic=on,eoi=on"))
- geng.add_argument("--clock",
+ geng.add_argument("--clock", action="append",
help=_("Set domain <clock> XML. Ex:\n"
"--clock offset=localtime,rtc_tickpolicy=catchup"))
- geng.add_argument("--pm",
+ geng.add_argument("--pm", action="append",
help=_("Configure VM power management features"))
- geng.add_argument("--events",
+ geng.add_argument("--events", action="append",
help=_("Configure VM lifecycle management policy"))
geng.add_argument("--resource", action="append",
help=_("Configure VM resource partitioning (cgroups)"))
@@ -714,11 +714,11 @@ def add_guest_xml_options(geng):
def add_boot_options(insg):
- insg.add_argument("--boot",
+ insg.add_argument("--boot", action="append",
help=_("Configure guest boot settings. Ex:\n"
"--boot hd,cdrom,menu=on\n"
"--boot init=/sbin/init (for containers)"))
- insg.add_argument("--idmap",
+ insg.add_argument("--idmap", action="append",
help=_("Enable user namespace for LXC container. Ex:\n"
"--idmap uid_start=0,uid_target=1000,uid_count=10"))
@@ -1286,10 +1286,11 @@ ParseCLICheck.add_arg(None, "path_exists", is_onoff=True,
ParseCLICheck.add_arg("all_checks", "all", is_onoff=True)
-def parse_check(checkstr):
+def parse_check(checks):
# Overwrite this for each parse
- parser = ParseCLICheck(None, checkstr)
- parser.parse(get_global_state())
+ for checkstr in util.listify(checks):
+ parser = ParseCLICheck(None, checkstr)
+ parser.parse(get_global_state())
######################
--
2.19.0
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
