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