Re: [PATCH 2/4] libxl: queue domain event earlier in shutdown handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michal Privoznik wrote:
> On 21.02.2014 00:02, Jim Fehlig wrote:
>> The shutdown handler may restart a domain when handling a reboot
>> event or when <on_*> is set to 'restart'.  Restarting consists of
>> calling libxlVmCleanup followed by libxlVmStart.  libxlVmStart will
>> emit a VIR_DOMAIN_EVENT_STARTED event, but the SHUTDOWN event is
>> not emitted until exiting the shutdown handler, after the STARTED
>> event.  Queue the event immediately after creation to avoid emitting
>> it after the start event.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
>> ---
>>   src/libxl/libxl_driver.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 721577d..e600de7 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -376,6 +376,8 @@ libxlDomainShutdownThread(void *opaque)
>>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>>                                              VIR_DOMAIN_EVENT_STOPPED,
>>                                             
>> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
>> +        if (dom_event)
>> +            libxlDomainEventQueue(driver, dom_event);
>>           switch (vm->def->onPoweroff) {
>>           case VIR_DOMAIN_LIFECYCLE_DESTROY:
>>               reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
>> @@ -390,6 +392,8 @@ libxlDomainShutdownThread(void *opaque)
>>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>>                                              VIR_DOMAIN_EVENT_STOPPED,
>>                                             
>> VIR_DOMAIN_EVENT_STOPPED_CRASHED);
>> +        if (dom_event)
>> +            libxlDomainEventQueue(driver, dom_event);
>>           switch (vm->def->onCrash) {
>>           case VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY:
>>               reason = VIR_DOMAIN_SHUTOFF_CRASHED;
>> @@ -404,6 +408,8 @@ libxlDomainShutdownThread(void *opaque)
>>           dom_event = virDomainEventLifecycleNewFromObj(vm,
>>                                              VIR_DOMAIN_EVENT_STOPPED,
>>                                             
>> VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
>> +        if (dom_event)
>> +            libxlDomainEventQueue(driver, dom_event);
>>           switch (vm->def->onReboot) {
>>           case VIR_DOMAIN_LIFECYCLE_DESTROY:
>>               reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
>> @@ -437,8 +443,6 @@ restart:
>>   cleanup:
>>       if (vm)
>>           virObjectUnlock(vm);
>> -    if (dom_event)
>> -        libxlDomainEventQueue(driver, dom_event);
>>       libxl_event_free(ctx, ev);
>>       VIR_FREE(shutdown_info);
>>   }
>>
>
> Wouldn't it be better to enqueue events at the beginning of 'destroy'
> and 'restart' labels? I'm thinking about something among these lines:

Yes, agreed.  As discussed on IRC, better to enqueue the event at the
labels since it is one less call to libxlDomainEventQueue, and improves
the code to handle any future <on_foo> action.

Regards,
Jim

>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 0b9bf7d..c009407 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -425,6 +425,10 @@ libxlDomainShutdownThread(void *opaque)
>      }
>
>  destroy:
> +    if (dom_event) {
> +        libxlDomainEventQueue(driver, dom_event);
> +        dom_event = NULL;
> +    }
>      libxl_domain_destroy(ctx, vm->def->id, NULL);
>      if (libxlVmCleanupJob(driver, vm, reason)) {
>          if (!vm->persistent) {
> @@ -435,6 +439,10 @@ destroy:
>      goto cleanup;
>
>  restart:
> +    if (dom_event) {
> +        libxlDomainEventQueue(driver, dom_event);
> +        dom_event = NULL;
> +    }
>      libxl_domain_destroy(ctx, vm->def->id, NULL);
>      libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
>      libxlVmStart(driver, vm, 0, -1);
>
>
> Michal
>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]