Re: [PATCH] virsh: Fix a problem of argv parsing

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

 



On 04/05/2011 06:38 AM, Osier Yang wrote:
> Problem example:
>   # virsh -d 5 vol-create --pool default col.xml
>   vol-create: pool(optdata): default
>   vol-create: pool(optdata): col.xml
>   error: command 'vol-create' requires <file> option
> 
>   It gets same "vshCmdOptDef" for both "--pool default"
>   and "col.xml".
> 
> This patch fixes it by increase "data_ct" when things like
> "--pool default" is successfully parsed, so that could
> get right "vshCmdOptDef" for the other arguments which
> are not with option name together.
> ---
>  tools/virsh.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

NAK to this patch.  It still doesn't detect other issues with argument
parsing, such as 'virsh vol-create --pool default --pool other col.xml'.
 Typically, option parsing should favor the last instance of a given
option, rather than the first, or error out that the option is given too
many times, but your patch doesn't change the status quo of going with
the first instance of the option.

Instead, I've prepared a better patch that not only fixes this issue,
but also addresses a regression I introduced in 0.8.5, where 'virsh
freecell 0' no longer parses correctly.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]