Re: [PATCH v3 03/14] virsh: Create macro for common "domain" option

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

 



On Sat, 2016-01-09 at 08:36 -0500, John Ferlan wrote:
> Rather than continually cut-n-paste the strings into each command,
> create a common macro to be used generically.  Note that not all
> '{.name = "domain",' entries are replaced, just those that have the
> common .help string of "domain name, id or uuid" and those that are
> required.
> 
> Other instances won't take all 3 options, but some subset of those
> options as directed by the virshCommandOptDomainBy flags argument or
> in some instances where the domain is not a required option.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  tools/virsh-domain.c | 445 +++++++++------------------------------------------
>  tools/virsh.h        |   7 +
>  2 files changed, 81 insertions(+), 371 deletions(-)

The domain option for the 'undefine' command takes all three forms[1]
even though the help string says otherwise.

I think the code is okay as it is and VIRSH_COMMON_OPT_DOMAIN should
be used there as well: if you agree, please squash in the appropriate
change before pushing. If you think the code should be fixed instead,
do so in a separate patch :)

ACK with the above taken into account.

Cheers.


[1] It's processed using virshCommandOptDomain()
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
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]