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