[libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour

The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index f1608dc..03b342f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1474,9 +1474,12 @@ virDomainScreenshot(virDomainPtr domain, * 'on_poweroff' XML setting resulting in a domain that reboots instead of * shutting down. For guests that react to a shutdown request, the differences * from virDomainDestroy() are that the guests disk storage will be in a - * stable state rather than having the (virtual) power cord pulled, and - * this command returns as soon as the shutdown request is issued rather - * than blocking until the guest is no longer running. + * stable state rather than having the (virtual) power cord pulled. It's up to + * hypervisor's driver implementation what methods of + * virDomainShutdownFlagValues are tried and in which order. As described in + * virDomainShutdownFlags, this call may return immediately after the shutdown + * request is send, or it may block indefinitely long, until the domain is + * actually shut off. * * If the domain is transient and has any snapshot metadata (see * virDomainSnapshotNum()), then that metadata will automatically @@ -1540,7 +1543,9 @@ virDomainShutdown(virDomainPtr domain) * and a hypervisor is not required to support all methods. * * To use guest agent (VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) the domain XML - * must have <channel> configured. + * must have <channel> configured. Moreover, depending on underlying + * hypervisor used, passing this flag may block the API until the domain + * is shut off (which is not guaranteed to happen anyway). * * Returns 0 in case of success and -1 in case of failure. */ -- 2.0.5

On 03/30/2015 06:07 AM, Michal Privoznik wrote:
The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is
s/of/off/
equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
I don't think this is the right approach. It is okay to block for a small finite amount of time, but if the guest agent really is something that can hang, we should fix that code to return without waiting for the agent response. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 30.03.2015 22:38, Eric Blake wrote:
On 03/30/2015 06:07 AM, Michal Privoznik wrote:
The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is
s/of/off/
equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
I don't think this is the right approach. It is okay to block for a small finite amount of time, but if the guest agent really is something that can hang, we should fix that code to return without waiting for the agent response.
The problem is, we can tell if the shutdown qemu-ga command succeeded only when we see the SHUTDOWN event on the domain monitor. The other option is to treat it like we're treating ACPI mode of the API: just send the request and report if the sending succeeded. Don't wait until domain actually shuts down. I can provide a patch. Michal

On 03/31/2015 01:37 AM, Michal Privoznik wrote:
I don't think this is the right approach. It is okay to block for a small finite amount of time, but if the guest agent really is something that can hang, we should fix that code to return without waiting for the agent response.
The problem is, we can tell if the shutdown qemu-ga command succeeded only when we see the SHUTDOWN event on the domain monitor.
But we don't NEED to know if it succeeded - per our docs, we only issue the request (or fail outright because the request cannot be delivered because the agent is non-responsive).
The other option is to treat it like we're treating ACPI mode of the API: just send the request and report if the sending succeeded. Don't wait until domain actually shuts down. I can provide a patch.
Yes, that would be more in line with the docs. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Michal Privoznik