[libvirt] [PATCH v2] Add API to change qemu agent response timeout
Daniel P. Berrangé
berrange at redhat.com
Fri Nov 8 10:52:12 UTC 2019
On Wed, Nov 06, 2019 at 03:13:21PM -0600, Jonathon Jongsma wrote:
> Some layered products such as oVirt have requested a way to avoid being
> blocked by guest agent commands when querying a loaded vm. For example,
> many guest agent commands are polled periodically to monitor changes,
> and rather than blocking the calling process, they'd prefer to simply
> time out when an agent query is taking too long.
>
> This patch adds a way for the user to specify a custom agent timeout
> that is applied to all agent commands.
>
> One special case to note here is the 'guest-sync' command. 'guest-sync'
> is issued internally prior to calling any other command. (For example,
> when libvirt wants to call 'guest-get-fsinfo', we first call
> 'guest-sync' and then call 'guest-get-fsinfo').
>
> Previously, the 'guest-sync' command used a 5-second timeout
> (VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT), whereas the actual command that
> followed always blocked indefinitely
> (VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK). As part of this patch, if a
> custom timeout is specified that is shorter than
> 5 seconds, this new timeout is also used for 'guest-sync'. If there is
> no custom timeout or if the custom timeout is longer than 5 seconds, we
> will continue to use the 5-second timeout.
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=1705426 for additional details.
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
> I've addressed most of the previous review comments from Peter, Daniel, and
> Michal but I'm unsure yet whether there's concensus on this feature. Here's a
> v2 for your consideration.
>
> I admit that the proposed API is not perfect. It would probably be more elegant
> if each agent command instead had its own 'timeout' argument so that the caller
> could specify the timeout for each invocation. But that is not the case, and
> introducing duplicate API for each agent command (with an additional timeout
> argument) would clutter the public API. In addition, we still have the issue
> Michal mentioned elsewhere in the thread where some commands like shutdown may
> or may not use the agent, so a timeout argument could be confusing.
>
> So: is this a viable approach, or should I rethink it?
>
> Changes in v2:
> - Make this an official public API rather than putting it in libvirt-qemu.
> - Don't use the qemuDomainObjEnterAgent()/ExitAgent() API, which expects you
> to acquire an agent job. Instead, introduce
> qemuDomainObjSetAgentResponseTimeout() which simply locks the agent while
> setting the variable.
> - rename the function slightly for better descriptiveness:
> virDomainQemuAgentSetTimeout() -> virDomainAgentSetResponseTimeout()
> - added 'flags' argument for future-proofing.
>
> include/libvirt/libvirt-domain.h | 9 ++++
> include/libvirt/libvirt-qemu.h | 8 +--
> src/driver-hypervisor.h | 6 +++
> src/libvirt-domain.c | 45 +++++++++++++++++
> src/libvirt_public.syms | 1 +
> src/qemu/qemu_agent.c | 84 ++++++++++++++++++++------------
> src/qemu/qemu_agent.h | 4 ++
> src/qemu/qemu_domain.c | 15 ++++++
> src/qemu/qemu_domain.h | 5 ++
> src/qemu/qemu_driver.c | 22 +++++++++
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 18 ++++++-
> src/remote_protocol-structs | 9 ++++
> 13 files changed, 190 insertions(+), 37 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 22277b0a84..83f6c1b835 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4916,4 +4916,13 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> int *nparams,
> unsigned int flags);
>
> +typedef enum {
> + VIR_DOMAIN_AGENT_COMMAND_BLOCK = -2,
> + VIR_DOMAIN_AGENT_COMMAND_DEFAULT = -1,
> + VIR_DOMAIN_AGENT_COMMAND_NOWAIT = 0,
> +} virDomainAgentCommandTimeoutValues;
These constants should all have the word "TIMEOUT_" in their name
eg VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_NOWAIT
> +int virDomainAgentSetResponseTimeout(virDomainPtr domain,
> + int timeout,
> + unsigned int flags);
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list