9:34 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The shutdown and reboot commands in virsh allow a comma
separated list of mode values
Signed-off-by: Daniel P. Berrange <berrange(a)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
10:05 a.m.
New subject: [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(a)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 :)
12:45 p.m.
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(a)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 :|
1:01 p.m.
> > + 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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:03 p.m.
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 :|
1:19 p.m.
> 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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4376
days inactive
4376
days old
5 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake