[PATCH v2 25/27] virsh: Simplify @flags handing in cmdSetmem() and cmdSetmaxmem()

[Peter Krempa]

On Thu, Dec 03, 2020 at 13:36:28 +0100, Michal Privoznik wrote:
> What code tries to achieve is that if no flags were provided to
> either 'setmem' or 'setmaxmem' commands then the old (no flags)
> API is called to be able to communicate with older daemons.
> Well, the code can be simplified a bit.

It's not quite the same though.

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  tools/virsh-domain.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 51a9fd90d1..eeeeaa8639 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c

It's better visible in the second hunk ...

> @@ -9090,7 +9087,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
>      bool config = vshCommandOptBool(cmd, "config");
>      bool live = vshCommandOptBool(cmd, "live");
>      bool current = vshCommandOptBool(cmd, "current");
> -    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_MEM_MAXIMUM;
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;

VIR_DOMAIN_AFFECT_CURRENT is 0

>      VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
>      VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> @@ -9099,9 +9096,6 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
>          flags |= VIR_DOMAIN_AFFECT_CONFIG;
>      if (live)
>          flags |= VIR_DOMAIN_AFFECT_LIVE;
> -    /* none of the options were specified */
> -    if (!current && !live && !config)
> -        flags = -1;

In the old code, if --current is used, this condition would be false ...

>  
>      if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
> @@ -9118,13 +9112,13 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
>      }
>      kibibytes = VIR_DIV_UP(bytes, 1024);
>  
> -    if (flags == -1) {
> +    if (flags == 0) {
>          if (virDomainSetMaxMemory(dom, kibibytes) != 0) {
>              vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
>              ret = false;
>          }
>      } else {
> -        if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) {

... and the new API is used.

> +        if (virDomainSetMemoryFlags(dom, kibibytes, flags | VIR_DOMAIN_MEM_MAXIMUM) < 0) {
>              vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
>              ret = false;
>          }

With your change --current does nothing actually. Semantically it should
be the same though. Perhaps justify it better in the commit message.