[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Allow comma separated list of shutdown/reboot modes with virsh



> The shutdown and reboot commands in virsh allow a comma
> separated list of mode values
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  tools/virsh-domain.c | 25 +++++++++++++++++++++++--
>  tools/virsh.pod      | 16 ++++++++++++----
>  2 files changed, 35 insertions(+), 6 deletions(-)


>  
> -    if (mode) {
> +    if (!(modes = virStringSplit(mode, ",", 0))) {

Any reason you can't use vshStringToArray to do the split?

> +        vshError(ctl, "%s", _("Cannot parse mode string"));
> +        return false;
> +    }
> +
> +    tmp = modes;
> +    while (*tmp) {
> +        mode = *tmp;
>          if (STREQ(mode, "acpi")) {
>              flags |= VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
>          } else if (STREQ(mode, "agent")) {
> @@ -4055,7 +4064,9 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
>                              "'acpi', 'agent', 'initctl' or
>                              'signal'"), mode);
>              return false;

Memory leak...

>          }
> +        tmp++;
>      }
> +    virStringFreeList(modes);

...You need a cleanup label to cover this in all error paths.

> @@ -4118,7 +4137,9 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
>                              "'acpi', 'agent', 'initctl' or
>                              'signal'"), mode);
>              return false;
>          }
> +        tmp++;
>      }
> +    virStringFreeList(modes);

Another leak.

> -=item B<reboot> I<domain> [I<--mode acpi|agent>]
> +=item B<reboot> I<domain> [I<--mode MODE-LIST>]
>  
>  Reboot a domain.  This acts just as if the domain had the B<reboot>
>  command run from the console.  The command returns as soon as it has
> @@ -1149,7 +1149,11 @@ I<on_reboot> parameter in the domain's XML
> definition.
>  
>  By default the hypervisor will try to pick a suitable shutdown
>  method. To specify an alternative method, the I<--mode> parameter
> -can specify C<acpi> or C<agent>.
> +can specify a comma separated list which includes C<acpi>, C<agent>,
> +C<initctl> and C<signal>. The order in which drivers will try each
> +mode is undefined, and not related to the order specified to virsh.
> +For strict control over ordering, use a single mode at a time and
> +repeat the command.

This portion looks good, though :)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]