Re: [RFC REPOST 7/7] tools: introduce virsh setappid command

On 9/9/21 6:13 PM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > docs/manpages/virsh.rst | 14 +++++++++ > tools/virsh-domain.c | 65 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 79 insertions(+) > > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst > index 9561b3f59d..36fc94808d 100644 > --- a/docs/manpages/virsh.rst > +++ b/docs/manpages/virsh.rst > @@ -4515,6 +4515,20 @@ Output the IP address and port number for the VNC display. If the information > is not available the processes will provide an exit code of 1. > > > +setappid > +-------- > + > +**Syntax:** > + > +:: > + > + setappid domain appid > + > +Set the Fibre Channel appid string for domain that is used to create VMID to > +tag the traffic. Accepts only printable characters and maximal length is 128 > +characters. To remove the appid from VM don't pass any appid. > + > + > DEVICE COMMANDS > =============== > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index e5bd1fdd75..07d9c7f954 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -14070,6 +14070,65 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) > } > > > +/* > + * "setappid" command > + */ > +static const vshCmdInfo info_setappid[] = { > + {.name = "help", > + .data = N_("Set domain Fibre Channel appid") > + }, > + {.name = "desc", > + .data = N_("Set the Fibre Channel appid string for domain that is " > + "used to create VMID to tag the traffic. Accepts only " > + "printable characters and maximal length is 128 characters. " > + "To remove the appid from VM don't pass any appid.") I wonder whether removal semantic with an extra flag, e.g. 'virsh setappid --clear' wouldn't be better. If we ever add new option to this function users will be forced to pass current APPID when using the new option so that the APPID doesn't get cleared. For instance: setappid --appid APPID --magic vs setappid --magic

> + }, > + {.name = NULL} > +}; > + > +static const vshCmdOptDef opts_setappid[] = { > + VIRSH_COMMON_OPT_DOMAIN_FULL(0), > + VIRSH_COMMON_OPT_DOMAIN_CONFIG, > + VIRSH_COMMON_OPT_DOMAIN_LIVE, > + VIRSH_COMMON_OPT_DOMAIN_CURRENT, > + {.name = "appid", > + .type = VSH_OT_STRING, > + .help = N_("User provided appid string"), > + }, > + {.name = NULL} > +}; > + > +static bool > +cmdSetAppid(vshControl *ctl, const vshCmd *cmd) > +{ > + g_autoptr(virshDomain) dom = NULL; > + bool config = vshCommandOptBool(cmd, "config"); > + bool live = vshCommandOptBool(cmd, "live"); > + bool current = vshCommandOptBool(cmd, "current"); > + const char *appid = NULL; > + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; > + > + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); > + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); > + > + if (config) > + flags |= VIR_DOMAIN_AFFECT_CONFIG; > + if (live) > + flags |= VIR_DOMAIN_AFFECT_LIVE; > + > + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) > + return false; > + > + if (vshCommandOptStringReq(ctl, cmd, "appid", &appid) < 0) > + return false; > + > + if (virDomainSetFibreChannelAppid(dom, (char *)appid, flags) < 0) Probably a leftover? This typecast shouldn't be needed. > + return false; > + > + return true; > +} Michal