[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