[libvirt] [PATCH 13/19] virsh-domain: Introduce 2 macros for domain options 'interface' and 'mac'

Lin Ma lma at suse.de
Tue Nov 10 10:32:30 UTC 2020


On 2020-11-02 19:40, Michal Privoznik wrote:
> On 11/2/20 9:26 AM, Lin Ma wrote:
>> The macro VIRSH_DOMAIN_OPT_INTERFACE for domain option '--interface',
>> The macro VIRSH_DOMAIN_OPT_MAC for domain option '--mac'.
>> 
>> Signed-off-by: Lin Ma <lma at suse.com>
>> ---
>>   tools/virsh-domain.h | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>> 
>> diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
>> index 0d59c579d4..ac05f983f9 100644
>> --- a/tools/virsh-domain.h
>> +++ b/tools/virsh-domain.h
>> @@ -39,3 +39,21 @@ typedef enum {
>>   VIR_ENUM_DECL(virshDomainHostnameSource);
>>     extern const vshCmdDef domManagementCmds[];
>> +
>> +#define VIRSH_DOMAIN_OPT_INTERFACE(_helpstr, oflags, cflags) \
>> +    {.name = "interface", \
>> +     .type = VSH_OT_STRING, \
>> +     .flags = oflags, \
>> +     .help = _helpstr, \
>> +     .completer = virshDomainInterfaceCompleter, \
>> +     .completer_flags = cflags, \
>> +    }
>> +
>> +#define VIRSH_DOMAIN_OPT_MAC(_helpstr, oflags) \
>> +    {.name = "mac", \
>> +     .type = VSH_OT_STRING, \
>> +     .flags = oflags, \
>> +     .help = _helpstr, \
>> +     .completer = virshDomainInterfaceCompleter, \
>> +     .completer_flags = VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, \
>> +    }
>> 
> 
> So, if we had two distinct completers, as I suggested, then this would
> look a bit different and both macros could accept cflags. We could
> then print MAC addresses only of running/inactive/all interfaces,
> which could be helpful.

The idea about two distinct completers, It's about virsh-interface, But 
in
virsh-domain, Here is only one interface completer 
virshDomainInterfaceCompleter
and it can handle name/mac well with a cflags.

IMO, The question is that we have two form of parameters: "interface" 
and "mac"
in virsh-domain. That's why I created two macros.

If we replace '--mac' by '--interface' in virsh-domain, Then things go 
easy:
With cflags's help, We don't need 2 macros any more, But obviously it 
changed
the existing virsh cmd usage, Probably It's not acceptable. So I created 
macro
VIRSH_DOMAIN_OPT_MAC only for the parameter '--mac', The '--mac' should 
accept
mac address string only, Does a cflags really make sense to this macro?

The discussion about macros perhaps impact the review of the patchset 
v2, so I
removed them in patchset v2, If we need them, I'll post them later.

> I'm stopping my review here. I think you get the gist of my review.
> Looking forward to v2.

Thank you so much for your nice review! I just posted v2 to libvirt ml.

Thank again,
LLin




More information about the libvir-list mailing list