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

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(a)redhat.com&gt; > --- > 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