Re: [Xen-devel] [libvirt] [PATCH V2 3/3] libxl: support PARAVIRT and ACPI reboot flags

[Jim Fehlig <jfehlig@xxxxxxxx>]

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?

> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel