[libvirt] [PATCH] Allow comma separated list of shutdown/reboot modes with virsh
From: "Daniel P. Berrange" <berrange@redhat.com> 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(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a7ffd2b..3ca246b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -48,6 +48,7 @@ #include "virfile.h" #include "virkeycode.h" #include "virmacaddr.h" +#include "virstring.h" #include "virsh-domain-monitor.h" #include "virterror_internal.h" #include "virtypedparam.h" @@ -4035,13 +4036,21 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) const char *mode = NULL; int flags = 0; int rv; + char **modes, **tmp; if (vshCommandOptString(cmd, "mode", &mode) < 0) { vshError(ctl, "%s", _("Invalid type")); return false; } - if (mode) { + if (!(modes = virStringSplit(mode, ",", 0))) { + 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; } + tmp++; } + virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; @@ -4098,13 +4109,21 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) const char *name; const char *mode = NULL; int flags = 0; + char **modes, **tmp; if (vshCommandOptString(cmd, "mode", &mode) < 0) { vshError(ctl, "%s", _("Invalid type")); return false; } - if (mode) { + if (!(modes = virStringSplit(mode, ",", 0))) { + vshError(ctl, "%s", _("Cannot parse mode string")); + return false; + } + + tmp = modes; + while (*tmp) { + mode = *tmp; if (STREQ(mode, "acpi")) { flags |= VIR_DOMAIN_REBOOT_ACPI_POWER_BTN; } else if (STREQ(mode, "agent")) { @@ -4118,7 +4137,9 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) "'acpi', 'agent', 'initctl' or 'signal'"), mode); return false; } + tmp++; } + virStringFreeList(modes); if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; diff --git a/tools/virsh.pod b/tools/virsh.pod index c901b11..7dde3df 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1137,7 +1137,7 @@ If I<--live> is specified, set scheduler information of a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -=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. =item B<reset> I<domain> @@ -1567,7 +1571,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. -=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<shutdown> I<domain> [I<--mode MODE-LIST>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1584,7 +1588,11 @@ snapshot metadata with B<snapshot-create>. 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. =item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] -- 1.7.11.7
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 :)
On Fri, Nov 30, 2012 at 11:05:25AM -0500, Eric Blake wrote:
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?
I didn't know it existed - utility functions like that shouldn't be in virsh code anyway. Now I know it exists, I'll kill it off in favour of virStringSplit everywhere. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
+ if (!(modes = virStringSplit(mode, ",", 0))) {
Any reason you can't use vshStringToArray to do the split?
I didn't know it existed - utility functions like that shouldn't be in virsh code anyway. Now I know it exists, I'll kill it off in favour of virStringSplit everywhere.
Except that vshStingToArray _also_ does ',,' unescaping, so that: a,b,,c,d is split into "a", "b,c", "d", rather than "a", "b", "", "c", "d" so we'd still need a post-processing path after virStringSplit. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Nov 30, 2012 at 02:01:53PM -0500, Eric Blake wrote:
+ if (!(modes = virStringSplit(mode, ",", 0))) {
Any reason you can't use vshStringToArray to do the split?
I didn't know it existed - utility functions like that shouldn't be in virsh code anyway. Now I know it exists, I'll kill it off in favour of virStringSplit everywhere.
Except that vshStingToArray _also_ does ',,' unescaping, so that:
a,b,,c,d
is split into "a", "b,c", "d", rather than "a", "b", "", "c", "d"
so we'd still need a post-processing path after virStringSplit.
Well, which users of vshStringToArray actually need that feature ? IIUC, the only ones are those which accept arbitrary user strings, which is only 1/2 users in virsh. The rest can use the new API with no practical loss in functionality, since they use fixed strings where ',' is not used. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
so we'd still need a post-processing path after virStringSplit.
Well, which users of vshStringToArray actually need that feature ?
Commit 9d91a18 merged ,, into vshStringToArray out of a single caller, vshParseSnapshotDiskspec; then commit 2cd4d8e added another caller vshParseSnapshotMemspec. All other callers don't need it.
IIUC, the only ones are those which accept arbitrary user strings, which is only 1/2 users in virsh. The rest can use the new API with no practical loss in functionality, since they use fixed strings where ',' is not used.
That works for me. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake