On 10/3/19 11:52 PM, 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 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@xxxxxxxxxx>
---
include/libvirt/libvirt-qemu.h | 2 +
src/driver-hypervisor.h | 5 +++
src/libvirt-qemu.c | 40 ++++++++++++++++++++
src/libvirt_qemu.syms | 4 ++
src/qemu/qemu_agent.c | 69 +++++++++++++++++++---------------
src/qemu/qemu_agent.h | 3 ++
src/qemu/qemu_driver.c | 24 ++++++++++++
src/qemu_protocol-structs | 8 ++++
src/remote/qemu_protocol.x | 18 ++++++++-
src/remote/remote_driver.c | 1 +
10 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 891617443f..8d3cc776e9 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -53,6 +53,8 @@ typedef enum {
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
int timeout, unsigned int flags);
+int virDomainQemuAgentSetTimeout(virDomainPtr domain, int timeout);
+
/**
* virConnectDomainQemuMonitorEventCallback:
* @conn: the connection pointer
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 015b2cd01c..2f17bff844 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1372,6 +1372,10 @@ typedef int
int *nparams,
unsigned int flags);
+typedef int
+(*virDrvDomainQemuAgentSetTimeout)(virDomainPtr domain,
+ int timeout);
+
typedef struct _virHypervisorDriver virHypervisorDriver;
typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1632,4 +1636,5 @@ struct _virHypervisorDriver {
virDrvDomainCheckpointGetParent domainCheckpointGetParent;
virDrvDomainCheckpointDelete domainCheckpointDelete;
virDrvDomainGetGuestInfo domainGetGuestInfo;
+ virDrvDomainQemuAgentSetTimeout domainQemuAgentSetTimeout;
};
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index 1afb5fe529..73f119cb23 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -216,6 +216,46 @@ virDomainQemuAgentCommand(virDomainPtr domain,
return NULL;
}
+/**
+ * virDomainQemuAgentSetTimeout:
+ * @domain: a domain object
+ * @timeout: timeout in seconds
+ *
+ * Set how long to wait for a response from qemu agent commands. By default,
+ * agent commands block forever waiting for a response.
+ *
+ * @timeout must be -2, -1, 0 or positive.
+ * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK(-2): meaning to block forever waiting for
+ * a result.
+ * VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT(-1): use default timeout value.
+ * VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT(0): does not wait.
+ * positive value: wait for @timeout seconds
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virDomainQemuAgentSetTimeout(virDomainPtr domain,
+ int timeout)
This definitely deserves some @flags argument. Even though we don't have
any flags to set now, it has proven to save us in the future couple of
times. Look at "extra flags; not used yet ...." occurring in docs for
our public APIs.
+{
+ virConnectPtr conn;
+
+ virResetLastError();
+
+ virCheckDomainReturn(domain, -1);
+ conn = domain->conn;
+
+ if (conn->driver->domainQemuAgentSetTimeout) {
+ if (conn->driver->domainQemuAgentSetTimeout(domain, timeout) < 0)
+ goto error;
+ return 0;
+ }
+
+ virReportUnsupportedError();
+
+ error:
+ virDispatchError(conn);
+ return -1;
+}
/**
* virConnectDomainQemuMonitorEventRegister:
diff --git a/src/libvirt_qemu.syms b/src/libvirt_qemu.syms
index 3a297e3a2b..348caea72e 100644
--- a/src/libvirt_qemu.syms
+++ b/src/libvirt_qemu.syms
@@ -30,3 +30,7 @@ LIBVIRT_QEMU_1.2.3 {
virConnectDomainQemuMonitorEventDeregister;
virConnectDomainQemuMonitorEventRegister;
} LIBVIRT_QEMU_0.10.0;
+LIBVIRT_QEMU_5.8.0 {
+ global:
+ virDomainQemuAgentSetTimeout;
+} LIBVIRT_QEMU_1.2.3;
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 34e1a85d64..86352aaec5 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -128,6 +128,7 @@ struct _qemuAgent {
* but fire up an event on qemu monitor instead.
* Take that as indication of successful completion */
qemuAgentEvent await_event;
+ int timeout;
};
static virClassPtr qemuAgentClass;
@@ -696,6 +697,8 @@ qemuAgentOpen(virDomainObjPtr vm,
if (!(mon = virObjectLockableNew(qemuAgentClass)))
return NULL;
+ /* agent commands block by default, user can choose different behavior */
+ mon->timeout = VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK;
mon->fd = -1;
if (virCondInit(&mon->notify) < 0) {
virReportSystemError(errno, "%s",
@@ -851,6 +854,11 @@ static int qemuAgentSend(qemuAgentPtr mon,
return -1;
if (seconds == VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT)
seconds = QEMU_AGENT_WAIT_TIME;
+
+ /* if user specified a custom agent timeout that is lower than the
+ * default timeout, use the shorter timeout instead */
+ if ((mon->timeout > 0) && (mon->timeout < seconds))
+ seconds = mon->timeout;
then = now + seconds * 1000ull;
}
@@ -1305,8 +1313,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints,
if (!cmd)
goto cleanup;
- if (qemuAgentCommand(mon, cmd, &reply, true,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+ if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0)
This looks redundant - you're already passing @mon, qemuAgentCommand can
grab ->timeout from it instead of having it as an argument.
goto cleanup;
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
