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

Pavel Hrdina posted 7 patches 4 years, 5 months ago
[RFC REPOST 7/7] tools: introduce virsh setappid command
Posted by Pavel Hrdina 4 years, 5 months ago
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 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.")
+    },
+    {.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)
+        return false;
+
+    return true;
+}
+
+
 const vshCmdDef domManagementCmds[] = {
     {.name = "attach-device",
      .handler = cmdAttachDevice,
@@ -14715,5 +14774,11 @@ const vshCmdDef domManagementCmds[] = {
      .info = info_domdirtyrate_calc,
      .flags = 0
     },
+    {.name = "setappid",
+     .handler = cmdSetAppid,
+     .opts = opts_setappid,
+     .info = info_setappid,
+     .flags = 0
+    },
     {.name = NULL}
 };
-- 
2.31.1

Re: [RFC REPOST 7/7] tools: introduce virsh setappid command
Posted by Michal Prívozník 4 years, 5 months ago
On 9/9/21 6:13 PM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  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

Re: [RFC REPOST 7/7] tools: introduce virsh setappid command
Posted by Pavel Hrdina 4 years, 5 months ago
On Fri, Sep 10, 2021 at 01:49:18PM +0200, Michal Prívozník wrote:
> On 9/9/21 6:13 PM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  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

Good point, thanks, using --clear will be probably better and easier for
users.

Pavel

> > +    },
> > +    {.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
>