5:14 p.m.
This small series is a V2 of
https://www.redhat.com/archives/libvir-list/2014-April/msg00837.html
Patch 1 adds VIR_DOMAIN_SHUTDOWN_PARAVIRT and
VIR_REBOOT_SHUTDOWN_PARAVIRT flags to the API as requested by danpb.
Patch 2 makes use of the shutdown flags in libxlDomainShutdownFlags.
Patch 3 does the same for libxlDomainReboot.
Jim Fehlig (3):
Introduce a new flag for controlling shutdown/reboot
libxl: support PARAVIRT and ACPI shutdown flags
libxl: support PARAVIRT and ACPI reboot flags
include/libvirt/libvirt.h.in | 2 ++
src/libxl/libxl_driver.c | 64 +++++++++++++++++++++++++++++++++++---------
tools/virsh-domain.c | 14 +++++++---
tools/virsh.pod | 8 +++---
4 files changed, 68 insertions(+), 20 deletions(-)
--
1.8.1.4
5:14 p.m.
New subject: [libvirt] [PATCH V2 1/3] Introduce a new flag for controlling shutdown/reboot
Add a new flag to virDomain{Reboot,Shutdown}FlagValues to allow
shutting down and rebooting a domain via the Xen paravirt control
interface.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
include/libvirt/libvirt.h.in | 2 ++
tools/virsh-domain.c | 14 ++++++++++----
tools/virsh.pod | 8 ++++----
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 930b7e8..2c7565a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1652,6 +1652,7 @@ typedef enum {
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1), /* Use guest agent */
VIR_DOMAIN_SHUTDOWN_INITCTL = (1 << 2), /* Use initctl */
VIR_DOMAIN_SHUTDOWN_SIGNAL = (1 << 3), /* Send a signal */
+ VIR_DOMAIN_SHUTDOWN_PARAVIRT = (1 << 4), /* Use paravirt guest control
*/
} virDomainShutdownFlagValues;
int virDomainShutdown (virDomainPtr domain);
@@ -1664,6 +1665,7 @@ typedef enum {
VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1), /* Use guest agent */
VIR_DOMAIN_REBOOT_INITCTL = (1 << 2), /* Use initctl */
VIR_DOMAIN_REBOOT_SIGNAL = (1 << 3), /* Send a signal */
+ VIR_DOMAIN_REBOOT_PARAVIRT = (1 << 4), /* Use paravirt guest control */
} virDomainRebootFlagValues;
int virDomainReboot (virDomainPtr domain,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..3a7c260 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4837,7 +4837,7 @@ static const vshCmdOptDef opts_shutdown[] = {
},
{.name = "mode",
.type = VSH_OT_STRING,
- .help = N_("shutdown mode: acpi|agent|initctl|signal")
+ .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
},
{.name = NULL}
};
@@ -4872,9 +4872,12 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SHUTDOWN_INITCTL;
} else if (STREQ(mode, "signal")) {
flags |= VIR_DOMAIN_SHUTDOWN_SIGNAL;
+ } else if (STREQ(mode, "paravirt")) {
+ flags |= VIR_DOMAIN_SHUTDOWN_PARAVIRT;
} else {
vshError(ctl, _("Unknown mode %s value, expecting "
- "'acpi', 'agent', 'initctl' or
'signal'"), mode);
+ "'acpi', 'agent', 'initctl',
'signal', "
+ "or 'paravirt'"), mode);
goto cleanup;
}
tmp++;
@@ -4923,7 +4926,7 @@ static const vshCmdOptDef opts_reboot[] = {
},
{.name = "mode",
.type = VSH_OT_STRING,
- .help = N_("shutdown mode: acpi|agent|initctl|signal")
+ .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt")
},
{.name = NULL}
};
@@ -4957,9 +4960,12 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_REBOOT_INITCTL;
} else if (STREQ(mode, "signal")) {
flags |= VIR_DOMAIN_REBOOT_SIGNAL;
+ } else if (STREQ(mode, "paravirt")) {
+ flags |= VIR_DOMAIN_REBOOT_PARAVIRT;
} else {
vshError(ctl, _("Unknown mode %s value, expecting "
- "'acpi', 'agent', 'initctl' or
'signal'"), mode);
+ "'acpi', 'agent', 'initctl',
'signal' "
+ "or 'paravirt'"), mode);
goto cleanup;
}
tmp++;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index abd2e93..9104804 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1302,8 +1302,8 @@ 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 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.
+C<initctl>, C<signal> and C<paravirt>. 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.
@@ -1781,8 +1781,8 @@ 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 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.
+C<initctl>, C<signal> and C<paravirt>. 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.
--
1.8.1.4
4:41 a.m.
New subject: [libvirt] [PATCH V2 1/3] Introduce a new flag for controlling shutdown/reboot
On Thu, May 01, 2014 at 04:14:37PM -0600, Jim Fehlig wrote:
Add a new flag to virDomain{Reboot,Shutdown}FlagValues to allow
shutting down and rebooting a domain via the Xen paravirt control
interface.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
include/libvirt/libvirt.h.in | 2 ++
tools/virsh-domain.c | 14 ++++++++++----
tools/virsh.pod | 8 ++++----
3 files changed, 16 insertions(+), 8 deletions(-)
ACK for merge post-1.2.4 release
Regards,
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 :|
5:14 p.m.
New subject: [libvirt] [PATCH V2 2/3] libxl: support PARAVIRT and ACPI shutdown flags
Add support for VIR_DOMAIN_SHUTDOWN_PARAVIRT and
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flags in
libxlDomainShutdownFlags().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index dcc3ac2..28e8512 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -873,7 +873,11 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
int ret = -1;
libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
+ VIR_DOMAIN_SHUTDOWN_PARAVIRT, -1);
+ if (flags == 0)
+ flags = VIR_DOMAIN_SHUTDOWN_PARAVIRT |
+ VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -888,18 +892,32 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
}
priv = vm->privateData;
- if (libxl_domain_shutdown(priv->ctx, vm->def->id) != 0) {
+ if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) {
+ ret = libxl_domain_shutdown(priv->ctx, vm->def->id);
+ if (ret == 0)
+ goto cleanup;
+
+ if (ret != ERROR_NOPARAVIRT) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to shutdown domain '%d' with
libxenlight"),
+ vm->def->id);
+ ret = -1;
+ goto cleanup;
+ }
+ }
+
+ if (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) {
+ ret = libxl_send_trigger(priv->ctx, vm->def->id,
+ LIBXL_TRIGGER_POWER, 0);
+ if (ret == 0)
+ goto cleanup;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to shutdown domain '%d' with
libxenlight"),
vm->def->id);
- goto cleanup;
+ ret = -1;
}
- /* vm is marked shutoff (or removed from domains list if not persistent)
- * in shutdown event handler.
- */
- ret = 0;
-
cleanup:
if (vm)
virObjectUnlock(vm);
--
1.8.1.4
3:24 a.m.
New subject: [libvirt] [Xen-devel] [PATCH V2 2/3] libxl: support PARAVIRT and ACPI shutdown flags
On Thu, 2014-05-01 at 16:14 -0600, Jim Fehlig wrote:
Add support for VIR_DOMAIN_SHUTDOWN_PARAVIRT and
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flags in
libxlDomainShutdownFlags().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
For the libxl use: Acked-by: Ian Campbell <ian.campbell(a)citrix.com>
4:40 a.m.
New subject: [libvirt] [PATCH V2 2/3] libxl: support PARAVIRT and ACPI shutdown flags
On Thu, May 01, 2014 at 04:14:38PM -0600, Jim Fehlig wrote:
Add support for VIR_DOMAIN_SHUTDOWN_PARAVIRT and
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN flags in
libxlDomainShutdownFlags().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index dcc3ac2..28e8512 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -873,7 +873,11 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
int ret = -1;
libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
+ VIR_DOMAIN_SHUTDOWN_PARAVIRT, -1);
+ if (flags == 0)
+ flags = VIR_DOMAIN_SHUTDOWN_PARAVIRT |
+ VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -888,18 +892,32 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
}
priv = vm->privateData;
- if (libxl_domain_shutdown(priv->ctx, vm->def->id) != 0) {
+ if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) {
+ ret = libxl_domain_shutdown(priv->ctx, vm->def->id);
+ if (ret == 0)
+ goto cleanup;
+
+ if (ret != ERROR_NOPARAVIRT) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to shutdown domain '%d' with
libxenlight"),
+ vm->def->id);
+ ret = -1;
+ goto cleanup;
+ }
I think you need 'ret = -1' here too, otherwise if domain shutdown
fails, but flags does not request ACPI, then we can leak
ret == ERROR_NOPARAVIRT back to the public API.
+ }
+
+ if (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) {
+ ret = libxl_send_trigger(priv->ctx, vm->def->id,
+ LIBXL_TRIGGER_POWER, 0);
+ if (ret == 0)
+ goto cleanup;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to shutdown domain '%d' with
libxenlight"),
vm->def->id);
- goto cleanup;
+ ret = -1;
}
- /* vm is marked shutoff (or removed from domains list if not persistent)
- * in shutdown event handler.
- */
- ret = 0;
-
cleanup:
if (vm)
virObjectUnlock(vm);
ACK to the patch with that fix added (post 1.2.4 release of course)
Regards,
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 :|
5:14 p.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
Add support for VIR_DOMAIN_REBOOT_PARAVIRT and
VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in
libxlDomainReboot().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 28e8512..6c63251 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -938,7 +938,11 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
int ret = -1;
libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
+ VIR_DOMAIN_REBOOT_PARAVIRT, -1);
+ if (flags == 0)
+ flags = VIR_DOMAIN_REBOOT_PARAVIRT |
+ VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -953,13 +957,31 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
}
priv = vm->privateData;
- if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) {
+ if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
+ ret = libxl_domain_reboot(priv->ctx, vm->def->id);
+ if (ret == 0)
+ goto cleanup;
+
+ if (ret != ERROR_NOPARAVIRT) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reboot domain '%d' with
libxenlight"),
+ vm->def->id);
+ ret = -1;
+ goto cleanup;
+ }
+ }
+
+ if (flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) {
+ ret = libxl_send_trigger(priv->ctx, vm->def->id,
+ LIBXL_TRIGGER_RESET, 0);
+ if (ret == 0)
+ goto cleanup;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to reboot domain '%d' with
libxenlight"),
vm->def->id);
- goto cleanup;
+ ret = -1;
}
- ret = 0;
cleanup:
if (vm)
--
1.8.1.4
3:24 a.m.
New subject: [libvirt] [Xen-devel] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
On Thu, 2014-05-01 at 16:14 -0600, Jim Fehlig wrote:
Add support for VIR_DOMAIN_REBOOT_PARAVIRT and
VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in
libxlDomainReboot().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
libxl side looks correct; Acked-by: Ian Campbell <ian.campbell(a)citrix.com>
4:46 a.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
On Thu, May 01, 2014 at 04:14:39PM -0600, Jim Fehlig wrote:
Add support for VIR_DOMAIN_REBOOT_PARAVIRT and
VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in
libxlDomainReboot().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 28e8512..6c63251 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -938,7 +938,11 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
int ret = -1;
libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
+ VIR_DOMAIN_REBOOT_PARAVIRT, -1);
+ if (flags == 0)
+ flags = VIR_DOMAIN_REBOOT_PARAVIRT |
+ VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -953,13 +957,31 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
}
priv = vm->privateData;
- if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) {
+ if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
+ ret = libxl_domain_reboot(priv->ctx, vm->def->id);
+ if (ret == 0)
+ goto cleanup;
+
+ if (ret != ERROR_NOPARAVIRT) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reboot domain '%d' with
libxenlight"),
+ vm->def->id);
+ ret = -1;
+ goto cleanup;
+ }
+ }
+
+ if (flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) {
+ ret = libxl_send_trigger(priv->ctx, vm->def->id,
+ LIBXL_TRIGGER_RESET, 0);
What does this trigger in ACPI ? IIUC, it'll do a hard reset of the
board, which is not the same as a controlled reboot which this API
wants. There isn't any ACPI button that I know of that guests will
interpret todo a controlled reboot, so in the QEMU driver we actually
just send a normal ACPI shutdown event. We have QEMU configured with
the '-no-shutdown' flag so when it finishes doing an controlled APCI
shutdown, we can then reset the board and start the CPUs again, which
gives the illusion of a controlled reboot.
Given that Xen has a decent paravirt reboot facility I'd probably
just not bother with trying to fake the controlled reboot via ACPI.
We only did this with QEMU since we didn't have any alternative to
make reboot work with QEMU at the time, since the QEMU guest agent
did not exist then.
A hard reset of the machine board is something you can implement
in the virDomainReset() API if you like though.
+ if (ret == 0)
+ goto cleanup;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to reboot domain '%d' with
libxenlight"),
vm->def->id);
- goto cleanup;
+ ret = -1;
}
- ret = 0;
cleanup:
if (vm)
Regards,
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 :|
9:01 a.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 04:14:39PM -0600, Jim Fehlig wrote:
> Add support for VIR_DOMAIN_REBOOT_PARAVIRT and
> VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in
> libxlDomainReboot().
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 28e8512..6c63251 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -938,7 +938,11 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
> int ret = -1;
> libxlDomainObjPrivatePtr priv;
>
> - virCheckFlags(0, -1);
> + virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
> + VIR_DOMAIN_REBOOT_PARAVIRT, -1);
> + if (flags == 0)
> + flags = VIR_DOMAIN_REBOOT_PARAVIRT |
> + VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
>
> if (!(vm = libxlDomObjFromDomain(dom)))
> goto cleanup;
> @@ -953,13 +957,31 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
> }
>
> priv = vm->privateData;
> - if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) {
> + if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
> + ret = libxl_domain_reboot(priv->ctx, vm->def->id);
> + if (ret == 0)
> + goto cleanup;
> +
> + if (ret != ERROR_NOPARAVIRT) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to reboot domain '%d' with
libxenlight"),
> + vm->def->id);
> + ret = -1;
> + goto cleanup;
> + }
> + }
> +
> + if (flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) {
> + ret = libxl_send_trigger(priv->ctx, vm->def->id,
> + LIBXL_TRIGGER_RESET, 0);
>
What does this trigger in ACPI ? IIUC, it'll do a hard reset of the
board,
Yes, I think you are right. Similar to pushing the reset button.
which is not the same as a controlled reboot which this API
wants. There isn't any ACPI button that I know of that guests will
interpret todo a controlled reboot, so in the QEMU driver we actually
just send a normal ACPI shutdown event. We have QEMU configured with
the '-no-shutdown' flag so when it finishes doing an controlled APCI
shutdown, we can then reset the board and start the CPUs again, which
gives the illusion of a controlled reboot.
Given that Xen has a decent paravirt reboot facility I'd probably
just not bother with trying to fake the controlled reboot via ACPI.
Ok, that sounds reasonable to me. I'll drop this patch when pushing the
others, post 1.2.4. Should 1/3 retain the VIR_DOMAIN_REBOOT_PARAVIRT
addition tovirDomainRebootFlagValues?
We only did this with QEMU since we didn't have any alternative
to
make reboot work with QEMU at the time, since the QEMU guest agent
did not exist then.
A hard reset of the machine board is something you can implement
in the virDomainReset() API if you like though.
Ah, yes, good point. domainReset is missing in the libxl driver.
Regards,
Jim
9:14 a.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
On Fri, May 02, 2014 at 08:01:00AM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
> On Thu, May 01, 2014 at 04:14:39PM -0600, Jim Fehlig wrote:
>
>> Add support for VIR_DOMAIN_REBOOT_PARAVIRT and
>> VIR_DOMAIN_REBOOT_ACPI_POWER_BTN flags in
>> libxlDomainReboot().
>>
>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
>> ---
>> src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++----
>> 1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 28e8512..6c63251 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -938,7 +938,11 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
>> int ret = -1;
>> libxlDomainObjPrivatePtr priv;
>>
>> - virCheckFlags(0, -1);
>> + virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
>> + VIR_DOMAIN_REBOOT_PARAVIRT, -1);
>> + if (flags == 0)
>> + flags = VIR_DOMAIN_REBOOT_PARAVIRT |
>> + VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
>>
>> if (!(vm = libxlDomObjFromDomain(dom)))
>> goto cleanup;
>> @@ -953,13 +957,31 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
>> }
>>
>> priv = vm->privateData;
>> - if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) {
>> + if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
>> + ret = libxl_domain_reboot(priv->ctx, vm->def->id);
>> + if (ret == 0)
>> + goto cleanup;
>> +
>> + if (ret != ERROR_NOPARAVIRT) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Failed to reboot domain '%d' with
libxenlight"),
>> + vm->def->id);
>> + ret = -1;
>> + goto cleanup;
>> + }
>> + }
>> +
>> + if (flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) {
>> + ret = libxl_send_trigger(priv->ctx, vm->def->id,
>> + LIBXL_TRIGGER_RESET, 0);
>>
>
> What does this trigger in ACPI ? IIUC, it'll do a hard reset of the
> board,
Yes, I think you are right. Similar to pushing the reset button.
> which is not the same as a controlled reboot which this API
> wants. There isn't any ACPI button that I know of that guests will
> interpret todo a controlled reboot, so in the QEMU driver we actually
> just send a normal ACPI shutdown event. We have QEMU configured with
> the '-no-shutdown' flag so when it finishes doing an controlled APCI
> shutdown, we can then reset the board and start the CPUs again, which
> gives the illusion of a controlled reboot.
>
> Given that Xen has a decent paravirt reboot facility I'd probably
> just not bother with trying to fake the controlled reboot via ACPI.
>
Ok, that sounds reasonable to me. I'll drop this patch when pushing the
others, post 1.2.4. Should 1/3 retain the VIR_DOMAIN_REBOOT_PARAVIRT
addition tovirDomainRebootFlagValues?
I don't think you need to drop the patch/code. It is still useful, IMHO,
to have the explicit flag for VIR_DOMAIN_REBOOT_PARAVIRT. I'd just
suggest you remove the block of code for VIR_DOMAIN_REBOOT_ACPI_POWER_BTN
impl in the reboot method.
Regards,
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 :|
9:43 a.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
Daniel P. Berrange wrote:
On Fri, May 02, 2014 at 08:01:00AM -0600, Jim Fehlig wrote:
> Daniel P. Berrange wrote:
>
>> Given that Xen has a decent paravirt reboot facility I'd probably
>> just not bother with trying to fake the controlled reboot via ACPI.
>>
>>
> Ok, that sounds reasonable to me. I'll drop this patch when pushing the
> others, post 1.2.4. Should 1/3 retain the VIR_DOMAIN_REBOOT_PARAVIRT
> addition tovirDomainRebootFlagValues?
>
I don't think you need to drop the patch/code. It is still useful, IMHO,
to have the explicit flag for VIR_DOMAIN_REBOOT_PARAVIRT. I'd just
suggest you remove the block of code for VIR_DOMAIN_REBOOT_ACPI_POWER_BTN
impl in the reboot method.
Just to clarify, do you mean changing this patch to the attached one?
Regards,
Jim
9:57 a.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags
On Fri, May 02, 2014 at 08:43:57AM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
> On Fri, May 02, 2014 at 08:01:00AM -0600, Jim Fehlig wrote:
>
>> Daniel P. Berrange wrote:
>>
>>> Given that Xen has a decent paravirt reboot facility I'd probably
>>> just not bother with trying to fake the controlled reboot via ACPI.
>>>
>>>
>> Ok, that sounds reasonable to me. I'll drop this patch when pushing the
>> others, post 1.2.4. Should 1/3 retain the VIR_DOMAIN_REBOOT_PARAVIRT
>> addition tovirDomainRebootFlagValues?
>>
>
> I don't think you need to drop the patch/code. It is still useful, IMHO,
> to have the explicit flag for VIR_DOMAIN_REBOOT_PARAVIRT. I'd just
> suggest you remove the block of code for VIR_DOMAIN_REBOOT_ACPI_POWER_BTN
> impl in the reboot method.
>
Just to clarify, do you mean changing this patch to the attached one?
Yes, you got it.
From e76f891bd843dd4f7b895d3929c9f561162a69a9 Mon Sep 17 00:00:00
2001
From: Jim Fehlig <jfehlig(a)suse.com>
Date: Thu, 1 May 2014 15:00:47 -0600
Subject: [PATCH 3/3] libxl: support PARAVIRT reboot flag
Add support for the VIR_DOMAIN_REBOOT_PARAVIRT flag in
libxlDomainReboot().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 28e8512..edbfa57 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -938,7 +938,9 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
int ret = -1;
libxlDomainObjPrivatePtr priv;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_REBOOT_PARAVIRT, -1);
+ if (flags == 0)
+ flags = VIR_DOMAIN_REBOOT_PARAVIRT;
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
@@ -953,13 +955,16 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
}
priv = vm->privateData;
- if (libxl_domain_reboot(priv->ctx, vm->def->id) != 0) {
+ if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
+ ret = libxl_domain_reboot(priv->ctx, vm->def->id);
+ if (ret == 0)
+ goto cleanup;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to reboot domain '%d' with
libxenlight"),
vm->def->id);
- goto cleanup;
+ ret = -1;
}
- ret = 0;
cleanup:
if (vm)
ACK
Regards,
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 :|
11:56 a.m.
Jim Fehlig wrote:
This small series is a V2 of
https://www.redhat.com/archives/libvir-list/2014-April/msg00837.html
Patch 1 adds VIR_DOMAIN_SHUTDOWN_PARAVIRT and
VIR_REBOOT_SHUTDOWN_PARAVIRT flags to the API as requested by danpb.
Patch 2 makes use of the shutdown flags in libxlDomainShutdownFlags.
Patch 3 does the same for libxlDomainReboot.
Jim Fehlig (3):
Introduce a new flag for controlling shutdown/reboot
libxl: support PARAVIRT and ACPI shutdown flags
libxl: support PARAVIRT and ACPI reboot flags
Now that 1.2.4 is out, I've pushed this series after squashing in the
suggested changes to 2 and 3.
Regards,
Jim
include/libvirt/libvirt.h.in | 2 ++
src/libxl/libxl_driver.c | 64 +++++++++++++++++++++++++++++++++++---------
tools/virsh-domain.c | 14 +++++++---
tools/virsh.pod | 8 +++---
4 files changed, 68 insertions(+), 20 deletions(-)
3884
days inactive
3888
days old
13 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Ian Campbell -
Jim Fehlig