Re: virt-install: Add warn if 'single-use' options are defined multiple times?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/4/2018 11:55 AM, Cole Robinson wrote:
On 10/04/2018 12:29 PM, Mark Kanda wrote:


On 10/1/2018 12:16 PM, Cole Robinson wrote:
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,

Yes, this does look better. However, IMO, the user should be warned if they do something like '--memory 123,maxmem=456 --memory 555' because it's not clear if they want '123' or '555'.

I was thinking of defining and using a custom argpase action to warn the user - see attached patch (incomplete - likely missing many single-use args).

What do you think of this approach?


I dunno... are there any other command line tools that warn in this case?

I don't know. In addition, I was a bit surprised to find python argparse doesn't have some facility to prevent options from being specified multiple times. Perhaps this lends credence to idea that this isn't something other command line tools typically do..

I can't think of any offhand. The warning may save users a bit of time if they accidentally specify an option twice and get unexpected results, but it will also be noise for other users.

For example, I know it's a common pattern for people to treat virt-install effectively as 'write once' and stuff a working invocation in script. So say a user has a simple script like

#!/bin/bash
virt-install --name test --memory 2048 --disk size=10 --cpu host-model "$@"

They invoke it like 'myscript --location $URL' or similar. But later on, or for some invocations, they want to specify more memory (myscript --memory 4096...) or a different cpu config (myscript --cpu host-passthrough). Now they see a warning even though it's entirely intended. So it's not a total win either way IMO,

Can you describe the case where this bit you? Maybe it will help me understand better


Similar to what you mentioned, we stumbled upon this when we made a mistake in a script which invokes 'virt-install'. We had a 'single-use' option defined twice, and were left a bit puzzled as to why it wasn't working as intended. TBH, as you can probably imagine, it really wasn't a big deal to chase down and fix. However, I still believe it would be good to warn users in this situation.

That being said, I don't feel strongly about it, so if there is resistance to adding such a thing, I'm okay with that.

Thanks,

-Mark

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list



[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux