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

Re: [libvirt] [PATCH] qemu: Rework handling of shutdown event

On 12/02/2011 08:02 AM, Jiri Denemark wrote:
> When QEMU guest finishes its shutdown sequence, qemu stops virtual CPUs
> and when started with -no-shutdown waits for us to kill it using
> SGITERM. Since QEMU is flushing its internal buffers, some time may pass
> before QEMU actually dies. We mistakenly used "paused" state (and
> events) for this which is quite confusing since users may see a domain
> going to pause while they expect it to shutdown. Since we already have
> "shutdown" state with "the domain is being shut down" semantics, we
> should use it for this state.
> However, the state didn't have a corresponding event so I created one
> and called its detail as VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED (guest OS
> finished its shutdown sequence) with the intent to add
> VIR_DOMAIN_EVENT_SHUTDOWN_STARTED in the future if we have a
> sufficiently capable guest agent that can notify us when guest OS starts
> to shutdown.

Not for this patch, but should we use VIR_DOMAIN_EVENT_SHUTDOWN_STARTED
any time we use the virDomainShutdown API to request a graceful
shutdown?  That is, by actually changing the state of the domain after a
call to virDomainShutdown, it will be more apparent whether a user has
previously requested shutdown, and if the guest is still running, it
means the guest either didn't react to the ACPI interrupt or else is
taking a long time to shut down.

> +++ b/include/libvirt/libvirt.h.in
> @@ -2288,6 +2288,7 @@ typedef enum {
>  } virDomainEventType;
>  /**
> @@ -2363,6 +2364,15 @@ typedef enum {
>  /**
> + * virDomainEventShutdownDetailType:
> + *
> + * Details about the 'shutdown' lifecycle event
> + */
> +typedef enum {
> +    VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0, /* Guest finished shutdown sequence */
> +} virDomainEventShutdownDetailType;
> +

Makes sense.  But you're missing a change to
examples/domain-events/events-c/event-test.c to reflect the new event.

> +
> +        VIR_DEBUG("Domain %s %s while its monitor was disconnected;"
> +                  " changing state to %s (%s)",
> +                  vm->def->name,
> +                  msg,

I was about to complain that building up an English sentence in pieces
like this doesn't translate well; then I realized this is VIR_DEBUG and
it isn't translated in the first place :)

ACK.  Up to you whether to fold the examples/ change in now or as a
followon patch, as long as you remember to do it.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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