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@xxxxxxxx>
> >> ---
> >> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list