Several paths repeat the same guest-agent teardown sequence; extract it into one helper so it stays in sync. Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com> --- src/qemu/qemu_driver.c | 5 +---- src/qemu/qemu_process.c | 15 +++++++++++---- src/qemu/qemu_process.h | 2 ++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ec9e1f9c4..0edc568bd7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3929,10 +3929,7 @@ processSerialChangedEvent(virQEMUDriver *driver, if (qemuConnectAgent(driver, vm) < 0) goto endjob; } else { - if (priv->agent) { - g_clear_pointer(&priv->agent, qemuAgentClose); - } - priv->agentError = false; + qemuProcessResetAgent(vm); } agentEvent = virDomainEventAgentLifecycleNewFromObj(vm, newstate, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4c94ff3c91..77d9880d14 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -140,6 +140,16 @@ qemuProcessRemoveDomainStatus(virQEMUDriver *driver, } +void +qemuProcessResetAgent(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + g_clear_pointer(&priv->agent, qemuAgentClose); + priv->agentError = false; +} + + /* * This is a callback registered with a qemuAgent *instance, * and to be invoked when the agent console hits an end of file @@ -9401,10 +9411,7 @@ qemuProcessStop(virDomainObj *vm, virObjectLock(vm); } - if (priv->agent) { - g_clear_pointer(&priv->agent, qemuAgentClose); - } - priv->agentError = false; + qemuProcessResetAgent(vm); if (priv->mon) { g_clear_pointer(&priv->mon, qemuMonitorClose); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 5874214596..3fc9f844d3 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -210,6 +210,8 @@ virDomainDiskDef *qemuProcessFindDomainDiskByAliasOrQOM(virDomainObj *vm, int qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm); +void qemuProcessResetAgent(virDomainObj *vm); + int qemuProcessSetupVcpu(virDomainObj *vm, unsigned int vcpuid, -- 2.54.0

Add vsock as a second QGA transport alongside the virtio-serial channel. It's attempted when there's no CONNECTED virtio-serial channel. Failed vsock connects retry on a capped exponential backoff (2s..60s, giving up after ~2 min), reset by any lifecycle event (channel-ready, replug, restart). priv->agentIsVsock specifies the type of the currently live transport, so the EOF, channel-state and unplug paths know which transport is dying. Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com> --- src/hypervisor/qemu_agent.c | 73 +++++---- src/hypervisor/qemu_agent.h | 9 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 19 ++- src/qemu/qemu_domain.h | 6 + src/qemu/qemu_driver.c | 13 +- src/qemu/qemu_hotplug.c | 6 + src/qemu/qemu_process.c | 290 ++++++++++++++++++++++++++++++++---- 8 files changed, 357 insertions(+), 60 deletions(-) diff --git a/src/hypervisor/qemu_agent.c b/src/hypervisor/qemu_agent.c index e549947fbf..00bd44b52b 100644 --- a/src/hypervisor/qemu_agent.c +++ b/src/hypervisor/qemu_agent.c @@ -592,52 +592,37 @@ qemuAgentIO(GSocket *socket G_GNUC_UNUSED, qemuAgent * -qemuAgentOpen(virDomainObj *vm, - const virDomainChrSourceDef *config, - GMainContext *context, - qemuAgentCallbacks *cb, - int timeout) +qemuAgentOpenFd(virDomainObj *vm, + int fd, + GMainContext *context, + qemuAgentCallbacks *cb, + int timeout) { - qemuAgent *agent; + qemuAgent *agent = NULL; g_autoptr(GError) gerr = NULL; if (!cb || !cb->eofNotify) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("EOF notify callback must be supplied")); - return NULL; + goto error; } if (qemuAgentInitialize() < 0) - return NULL; + goto error; if (!(agent = virObjectLockableNew(qemuAgentClass))) - return NULL; + goto error; agent->timeout = timeout; - agent->fd = -1; + agent->fd = fd; if (virCondInit(&agent->notify) < 0) { virReportSystemError(errno, "%s", _("cannot initialize agent condition")); - virObjectUnref(agent); - return NULL; + goto error; } agent->vm = virObjectRef(vm); agent->cb = cb; - if (config->type != VIR_DOMAIN_CHR_TYPE_UNIX) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to handle agent type: %1$s"), - virDomainChrTypeToString(config->type)); - goto cleanup; - } - - virObjectUnlock(vm); - agent->fd = qemuAgentOpenUnix(config->data.nix.path); - virObjectLock(vm); - - if (agent->fd == -1) - goto cleanup; - agent->context = g_main_context_ref(context); agent->socket = g_socket_new_from_fd(agent->fd, &gerr); @@ -645,7 +630,8 @@ qemuAgentOpen(virDomainObj *vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create socket object: %1$s"), gerr->message); - goto cleanup; + qemuAgentClose(agent); + return NULL; } qemuAgentRegister(agent); @@ -655,12 +641,41 @@ qemuAgentOpen(virDomainObj *vm, return agent; - cleanup: - qemuAgentClose(agent); + error: + /* qemuAgentDispose() doesn't touch agent->fd, so close it here */ + VIR_FORCE_CLOSE(fd); + virObjectUnref(agent); return NULL; } +qemuAgent * +qemuAgentOpen(virDomainObj *vm, + const virDomainChrSourceDef *config, + GMainContext *context, + qemuAgentCallbacks *cb, + int timeout) +{ + int fd; + + if (config->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to handle agent type: %1$s"), + virDomainChrTypeToString(config->type)); + return NULL; + } + + virObjectUnlock(vm); + fd = qemuAgentOpenUnix(config->data.nix.path); + virObjectLock(vm); + + if (fd == -1) + return NULL; + + return qemuAgentOpenFd(vm, fd, context, cb, timeout); +} + + static void qemuAgentNotifyCloseLocked(qemuAgent *agent) { diff --git a/src/hypervisor/qemu_agent.h b/src/hypervisor/qemu_agent.h index 3dbc3baec1..ce75ad2829 100644 --- a/src/hypervisor/qemu_agent.h +++ b/src/hypervisor/qemu_agent.h @@ -24,6 +24,9 @@ #include "internal.h" #include "domain_conf.h" +/* Fixed vsock port the guest agent listens on by convention */ +#define QEMU_AGENT_VSOCK_PORT 1024 + typedef struct _qemuAgent qemuAgent; typedef struct _qemuAgentCallbacks qemuAgentCallbacks; @@ -41,6 +44,12 @@ qemuAgent *qemuAgentOpen(virDomainObj *vm, qemuAgentCallbacks *cb, int timeout); +qemuAgent *qemuAgentOpenFd(virDomainObj *vm, + int fd, + GMainContext *context, + qemuAgentCallbacks *cb, + int timeout); + void qemuAgentClose(qemuAgent *mon); void qemuAgentNotifyClose(qemuAgent *mon); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6a9cd34134..95db6a885d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1735,6 +1735,7 @@ qemuAgentGetVCPUs; qemuAgentNotifyClose; qemuAgentNotifyEvent; qemuAgentOpen; +qemuAgentOpenFd; qemuAgentSetResponseTimeout; qemuAgentSetTime; qemuAgentSetUserPassword; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8539bb72fa..9560ae78ad 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1980,6 +1980,22 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivate *priv) priv->iommufdState = false; g_clear_pointer(&priv->memoryBackingDir, g_free); + + qemuDomainCancelAgentVsockReconnect(priv); + priv->agentIsVsock = false; +} + + +void +qemuDomainCancelAgentVsockReconnect(qemuDomainObjPrivate *priv) +{ + priv->agentVsockReconnectAttempts = 0; + + if (!priv->agentVsockReconnectTimer) + return; + + g_source_destroy(priv->agentVsockReconnectTimer); + g_clear_pointer(&priv->agentVsockReconnectTimer, g_source_unref); } @@ -7417,7 +7433,8 @@ qemuDomainAgentAvailable(virDomainObj *vm, return false; } if (!priv->agent) { - if (qemuFindAgentConfig(vm->def)) { + if (qemuFindAgentConfig(vm->def) || + qemuDomainVsockHasGuestCid(vm->def)) { if (reportError) { virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("QEMU guest agent is not connected")); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e994776996..a3d8fa7a86 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -120,6 +120,10 @@ struct _qemuDomainObjPrivate { qemuAgent *agent; bool agentError; + bool agentIsVsock; + + GSource *agentVsockReconnectTimer; + unsigned int agentVsockReconnectAttempts; bool beingDestroyed; char *pidfile; @@ -807,6 +811,8 @@ bool qemuDomainCheckABIStability(virQEMUDriver *driver, bool qemuDomainVsockHasGuestCid(const virDomainDef *def); +void qemuDomainCancelAgentVsockReconnect(qemuDomainObjPrivate *priv); + bool qemuDomainAgentAvailable(virDomainObj *vm, bool reportError); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0edc568bd7..a4a30627d4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3926,15 +3926,20 @@ processSerialChangedEvent(virQEMUDriver *driver, if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")) { virObjectEvent *agentEvent = NULL; if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { + qemuDomainCancelAgentVsockReconnect(priv); if (qemuConnectAgent(driver, vm) < 0) goto endjob; } else { - qemuProcessResetAgent(vm); + if (!priv->agentIsVsock) + qemuProcessResetAgent(vm); } - agentEvent = virDomainEventAgentLifecycleNewFromObj(vm, newstate, - VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL); - virObjectEventStateQueue(driver->domainEventState, agentEvent); + if ((newstate == VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED && !priv->agent) || + (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED && priv->agent)) { + agentEvent = virDomainEventAgentLifecycleNewFromObj(vm, newstate, + VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, agentEvent); + } } /* we deliberately allow for goto endjob to skip generic event emission diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5be567b510..8018b5d3c5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5467,9 +5467,15 @@ static int qemuDomainRemoveVsockDevice(virDomainObj *vm, virDomainVsockDef *dev) { + qemuDomainObjPrivate *priv = vm->privateData; + VIR_DEBUG("Removing vsock device %s from domain %p %s", dev->info.alias, vm, vm->def->name); + qemuDomainCancelAgentVsockReconnect(priv); + if (priv->agentIsVsock && priv->agent) + qemuProcessResetAgent(vm); + qemuDomainReleaseDeviceAddress(vm, &dev->info); g_clear_pointer(&vm->def->vsock, virDomainVsockDefFree); return 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 77d9880d14..1d94dc4673 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -140,16 +140,123 @@ qemuProcessRemoveDomainStatus(virQEMUDriver *driver, } +#define QEMU_AGENT_VSOCK_RECONNECT_MIN_MS 2000 /* initial backoff */ +#define QEMU_AGENT_VSOCK_RECONNECT_MAX_MS 60000 /* interval cap */ +#define QEMU_AGENT_VSOCK_RECONNECT_MAX_ATTEMPTS 6 /* give up (~2 min) */ + + +static int +qemuChannelOpenVsock(virQEMUDriver *driver, + virDomainObj *vm, + unsigned int cid) +{ + int fd; + + if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0) { + VIR_ERROR(_("Failed to set security context for vsock agent for %1$s"), + vm->def->name); + return -1; + } + + virObjectUnlock(vm); + fd = virVsockConnectQuiet(cid, QEMU_AGENT_VSOCK_PORT); + virObjectLock(vm); + + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) + VIR_ERROR(_("Failed to clear security context for vsock agent for %1$s"), + vm->def->name); + + return fd; +} + + +static gboolean qemuProcessAgentVsockReconnectTimer(gpointer user_data); +static int qemuConnectAgentVsock(virQEMUDriver *driver, virDomainObj *vm); + + +static void +qemuProcessScheduleAgentVsockReconnect(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + unsigned int interval; + + if (priv->agentVsockReconnectTimer || !priv->eventThread) + return; + + if (priv->agentVsockReconnectAttempts >= QEMU_AGENT_VSOCK_RECONNECT_MAX_ATTEMPTS) { + VIR_DEBUG("Giving up vsock QGA reconnect for %s after %u attempts", + vm->def->name, priv->agentVsockReconnectAttempts); + return; + } + + interval = QEMU_AGENT_VSOCK_RECONNECT_MIN_MS << priv->agentVsockReconnectAttempts; + if (interval > QEMU_AGENT_VSOCK_RECONNECT_MAX_MS) + interval = QEMU_AGENT_VSOCK_RECONNECT_MAX_MS; + priv->agentVsockReconnectAttempts++; + + VIR_DEBUG("Scheduling vsock QGA reconnect for %s in %u ms (attempt %u)", + vm->def->name, interval, priv->agentVsockReconnectAttempts); + + priv->agentVsockReconnectTimer = + g_timeout_source_new(interval); + g_source_set_callback(priv->agentVsockReconnectTimer, + qemuProcessAgentVsockReconnectTimer, + virObjectRef(vm), + (GDestroyNotify)virObjectUnref); + g_source_attach(priv->agentVsockReconnectTimer, + virEventThreadGetContext(priv->eventThread)); +} + + +static gboolean +qemuProcessAgentVsockReconnectTimer(gpointer user_data) +{ + virDomainObj *vm = user_data; + qemuDomainObjPrivate *priv; + + virObjectLock(vm); + priv = vm->privateData; + + /* A concurrent cancel may have already destroyed and cleared the source + * while we blocked on the lock; drop our ref only if it's still ours. */ + if (priv->agentVsockReconnectTimer) + g_clear_pointer(&priv->agentVsockReconnectTimer, g_source_unref); + + if (!priv->beingDestroyed && virDomainObjIsActive(vm) && + !priv->agent && qemuDomainVsockHasGuestCid(vm->def)) + ignore_value(qemuConnectAgentVsock(priv->driver, vm)); + + virObjectUnlock(vm); + return G_SOURCE_REMOVE; +} + + void qemuProcessResetAgent(virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; g_clear_pointer(&priv->agent, qemuAgentClose); + priv->agentIsVsock = false; priv->agentError = false; } +/* The virtio-serial agent lifecycle is reported from the channel event; + * vsock has no such event, so emit it from the connect/EOF paths instead. */ +static void +qemuProcessEmitVsockAgentLifecycle(virQEMUDriver *driver, + virDomainObj *vm, + virDomainChrDeviceState newstate) +{ + virObjectEvent *event; + + event = virDomainEventAgentLifecycleNewFromObj(vm, newstate, + VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); +} + + /* * This is a callback registered with a qemuAgent *instance, * and to be invoked when the agent console hits an end of file @@ -161,6 +268,7 @@ qemuProcessHandleAgentEOF(qemuAgent *agent, virDomainObj *vm) { qemuDomainObjPrivate *priv; + bool wasVsock; virObjectLock(vm); VIR_DEBUG("Received EOF from agent on %p '%s'", vm, vm->def->name); @@ -172,14 +280,31 @@ qemuProcessHandleAgentEOF(qemuAgent *agent, goto unlock; } + /* Stale EOF from an instance we already replaced. */ + if (priv->agent != agent) { + VIR_DEBUG("Agent EOF from stale instance %p (current %p)", + agent, priv->agent); + qemuAgentClose(agent); + goto unlock; + } + if (priv->beingDestroyed) { VIR_DEBUG("Domain is being destroyed, agent EOF is expected"); goto unlock; } - qemuAgentClose(agent); - priv->agent = NULL; - priv->agentError = false; + wasVsock = priv->agentIsVsock; + + qemuProcessResetAgent(vm); + + if (wasVsock) { + qemuProcessEmitVsockAgentLifecycle(priv->driver, vm, + VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED); + + /* The timer is never set while we hold an agent, so no cancel needed. */ + if (qemuDomainVsockHasGuestCid(vm->def)) + qemuProcessScheduleAgentVsockReconnect(vm); + } virObjectUnlock(vm); return; @@ -219,35 +344,31 @@ static qemuAgentCallbacks agentCallbacks = { }; -int -qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm) +/* + * Connect agent over the UNIX virtio-serial channel. + * + * Writes vm->privateData->agentError on failure. + * + * Returns: -1 if the domain is dead, 0 otherwise. + */ +static int +qemuConnectAgentUnix(virQEMUDriver *driver, virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; qemuAgent *agent = NULL; virDomainChrDef *config = qemuFindAgentConfig(vm->def); - if (!config) - return 0; - - if (priv->agent) - return 0; - - if (config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { - VIR_DEBUG("Deferring connecting to guest agent"); - return 0; - } - if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0) { VIR_ERROR(_("Failed to set security context for agent for %1$s"), vm->def->name); - goto cleanup; + goto error; } agent = qemuAgentOpen(vm, config->source, virEventThreadGetContext(priv->eventThread), &agentCallbacks, - QEMU_DOMAIN_PRIVATE(vm)->agentTimeout); + priv->agentTimeout); if (!virDomainObjIsActive(vm)) { qemuAgentClose(agent); @@ -260,20 +381,120 @@ qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm) VIR_ERROR(_("Failed to clear security context for agent for %1$s"), vm->def->name); qemuAgentClose(agent); - goto cleanup; + goto error; + } + + if (!agent) + goto error; + + /* Another caller may have connected while the domain was unlocked. */ + if (priv->agent) { + qemuAgentClose(agent); + return 0; } priv->agent = agent; - if (!priv->agent) - VIR_INFO("Failed to connect agent for %s", vm->def->name); + priv->agentIsVsock = false; + priv->agentError = false; + return 0; - cleanup: - if (!priv->agent) { - VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name); - priv->agentError = true; - virResetLastError(); + error: + VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name); + priv->agentError = true; + virResetLastError(); + return 0; +} + + +/* + * Connect agent over vsock. + * + * Sets the reconnect timer on failure. + * + * Returns: -1 if the domain is dead, 0 otherwise. + */ +static int +qemuConnectAgentVsock(virQEMUDriver *driver, virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuAgent *agent = NULL; + unsigned int cid = vm->def->vsock->guest_cid; + int fd; + + fd = qemuChannelOpenVsock(driver, vm, cid); + + if (!virDomainObjIsActive(vm)) { + VIR_FORCE_CLOSE(fd); + return -1; } + /* Another caller may have connected while the domain was unlocked. */ + if (priv->agent) { + VIR_FORCE_CLOSE(fd); + return 0; + } + + /* The vsock device may have been unplugged while unlocked. */ + if (!vm->def->vsock) { + VIR_FORCE_CLOSE(fd); + return 0; + } + + if (fd < 0) { + VIR_DEBUG("vsock QGA connect failed for %s", vm->def->name); + goto retry; + } + + agent = qemuAgentOpenFd(vm, fd, + virEventThreadGetContext(priv->eventThread), + &agentCallbacks, + priv->agentTimeout); + if (agent == NULL) { + VIR_WARN("Cannot open vsock QGA for %s", vm->def->name); + goto retry; + } + + priv->agent = agent; + priv->agentIsVsock = true; + priv->agentError = false; + qemuDomainCancelAgentVsockReconnect(priv); + qemuProcessEmitVsockAgentLifecycle(driver, vm, + VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED); + VIR_DEBUG("Connected to vsock QGA cid=%u for %s", cid, vm->def->name); + return 0; + + retry: + virResetLastError(); + qemuProcessScheduleAgentVsockReconnect(vm); + return 0; +} + + +/* + * Connect agent. + * + * Use the virtio-serial channel if it's CONNECTED. + * Otherwise, if a usable CID is configured, try vsock with exponential backoff. + * + * Returns: -1 if the domain is dead, 0 otherwise. + */ +int +qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virDomainChrDef *config = qemuFindAgentConfig(vm->def); + bool unix_ready = config && + config->state == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED; + + if (priv->agent) + return 0; + + if (unix_ready) + return qemuConnectAgentUnix(driver, vm); + + if (qemuDomainVsockHasGuestCid(vm->def)) + return qemuConnectAgentVsock(driver, vm); + return 0; } @@ -448,6 +669,14 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED, if (priv->agent) qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_RESET); + /* A rebooting guest brings its agent back up; restart vsock QGA reconnect + * from scratch in case we'd already given up. + * Skip if we're tearing down: the timer would outlive the event thread. */ + if (!priv->beingDestroyed && !priv->agent && qemuDomainVsockHasGuestCid(vm->def)) { + qemuDomainCancelAgentVsockReconnect(priv); + qemuProcessScheduleAgentVsockReconnect(vm); + } + qemuDomainSetFakeReboot(vm, false); qemuDomainSaveStatus(vm); @@ -9375,6 +9604,10 @@ qemuProcessStop(virDomainObj *vm, virDomainAsyncJobTypeToString(asyncJob)); } + /* Drop the timer before the active-check: Stop can goto endjob early, and a + * live timer pins a vm ref and may fire after the event thread dies. */ + qemuDomainCancelAgentVsockReconnect(priv); + if (!virDomainObjIsActive(vm)) { VIR_DEBUG("VM '%s' not active", vm->def->name); goto endjob; @@ -9409,6 +9642,11 @@ qemuProcessStop(virDomainObj *vm, virObjectUnlock(vm); virEventThreadStop(priv->eventThread); virObjectLock(vm); + + /* The event thread is joined now, so no reconnect callback is running. + * Drop any timer one re-armed in the unlock window above; otherwise it + * would pin a vm ref forever. */ + qemuDomainCancelAgentVsockReconnect(priv); } qemuProcessResetAgent(vm); -- 2.54.0

qemuagenttest hands an fd straight to qemuAgentOpen(). Transport selection logic was added to qemuConnectAgent(), so to test the vsock path and related logic, we need to test it instead. Mock virVsockConnectQuiet() to the scripted unix QGA server, proving the CID-based vsock branch is taken and a command passes through. Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com> --- src/util/virvsock.h | 5 +- tests/meson.build | 1 + tests/qemuagenttest.c | 43 ++++++++++++- tests/qemumonitortestutils.c | 119 ++++++++++++++++++++++++++++++++--- tests/qemumonitortestutils.h | 4 ++ tests/qemuvsockagentmock.c | 65 +++++++++++++++++++ 6 files changed, 228 insertions(+), 9 deletions(-) create mode 100644 tests/qemuvsockagentmock.c diff --git a/src/util/virvsock.h b/src/util/virvsock.h index 8bf08f9228..215bde2d3e 100644 --- a/src/util/virvsock.h +++ b/src/util/virvsock.h @@ -18,6 +18,8 @@ #pragma once +#include "internal.h" + #define VIR_VSOCK_GUEST_CID_MIN 3 #define VIR_VSOCK_CONNECT_TIMEOUT_MS 200 @@ -31,4 +33,5 @@ virVsockAcquireGuestCid(int fd, int virVsockConnectQuiet(unsigned int cid, - unsigned int port); + unsigned int port) + ATTRIBUTE_MOCKABLE; diff --git a/tests/meson.build b/tests/meson.build index 63e0474a29..cd041dd086 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -178,6 +178,7 @@ if conf.has('WITH_QEMU') { 'name': 'qemucapsprobemock', 'link_with': [ test_qemu_driver_lib ] }, { 'name': 'qemucpumock' }, { 'name': 'qemuhotplugmock', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_lib, test_utils_lib ] }, + { 'name': 'qemuvsockagentmock' }, { 'name': 'qemuxml2argvmock', 'link_with': [ test_utils_lib ] }, { 'name': 'virhostidmock' }, ] diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 74cd317e74..c8e11cfd3c 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -23,6 +23,7 @@ #include "testutilsqemu.h" #include "qemumonitortestutils.h" #include "qemu/qemu_conf.h" +#include "qemu/qemu_domain.h" #include "hypervisor/qemu_agent.h" #include "virerror.h" @@ -1394,6 +1395,45 @@ testQemuAgentGetLoadAvg(const void *data) } +static int +testQemuAgentVsock(const void *data) +{ + virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgentVsock(&driver, xmlopt); + virDomainObj *vm; + int rc; + + if (!test) + return -1; + + /* The point of this test: qemuConnectAgent() chose vsock and flagged it. */ + vm = qemuMonitorTestGetDomainObj(test); + if (!QEMU_DOMAIN_PRIVATE(vm)->agentIsVsock) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "agent transport is not vsock"); + return -1; + } + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + return -1; + + if (qemuMonitorTestAddItem(test, "guest-fsfreeze-thaw", + "{ \"return\" : 5 }") < 0) + return -1; + + if ((rc = qemuAgentFSThaw(qemuMonitorTestGetAgent(test))) < 0) + return -1; + + if (rc != 5) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected 5 thawed filesystems, got %d", rc); + return -1; + } + + return 0; +} + + static int mymain(void) { @@ -1431,6 +1471,7 @@ mymain(void) DO_TEST(SSHKeys); DO_TEST(GetDisks); DO_TEST(GetLoadAvg); + DO_TEST(Vsock); DO_TEST(Timeout); /* Timeout should always be called last */ @@ -1439,4 +1480,4 @@ mymain(void) return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("qemuvsockagent")) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index e83dd1d9c4..ffdf55a5f9 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -30,12 +30,14 @@ #include "hypervisor/qemu_agent.h" #include "qemu/qemu_domain.h" #include "qemu/qemu_processpriv.h" +#include "qemu/qemu_process.h" #include "qemu/qemu_monitor.h" #include "rpc/virnetsocket.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" #include "vireventthread.h" +#include "virvsock.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1187,16 +1189,48 @@ qemuMonitorTestNewFromFileFull(const char *fileName, } +/* Build the bare agent test: the common test object plus the event + * thread the agent runs on. @src is filled in by the caller's transport. */ +static qemuMonitorTest * +qemuMonitorTestNewAgentInit(virDomainXMLOption *xmlopt, + const char *threadName, + virDomainChrSourceDef *src) +{ + g_autoptr(qemuMonitorTest) test = NULL; + + if (!(test = qemuMonitorCommonTestNew(xmlopt, NULL, src))) + return NULL; + + if (!(test->eventThread = virEventThreadNew(threadName))) + return NULL; + + return g_steal_pointer(&test); +} + + +/* Finish an agent test once test->agent is connected: lock it, run the monitor + * init and release @src. On failure the caller's error path frees @test. */ +static int +qemuMonitorTestAgentStart(qemuMonitorTest *test, + virDomainChrSourceDef *src) +{ + virObjectLock(test->agent); + + if (qemuMonitorCommonTestInit(test) < 0) + return -1; + + virDomainChrSourceDefClear(src); + return 0; +} + + qemuMonitorTest * qemuMonitorTestNewAgent(virDomainXMLOption *xmlopt) { g_autoptr(qemuMonitorTest) test = NULL; virDomainChrSourceDef src = { 0 }; - if (!(test = qemuMonitorCommonTestNew(xmlopt, NULL, &src))) - goto error; - - if (!(test->eventThread = virEventThreadNew("agent-test"))) + if (!(test = qemuMonitorTestNewAgentInit(xmlopt, "agent-test", &src))) goto error; if (!(test->agent = qemuAgentOpen(test->vm, @@ -1206,16 +1240,87 @@ qemuMonitorTestNewAgent(virDomainXMLOption *xmlopt) QEMU_DOMAIN_PRIVATE(test->vm)->agentTimeout))) goto error; - virObjectLock(test->agent); - - if (qemuMonitorCommonTestInit(test) < 0) + if (qemuMonitorTestAgentStart(test, &src) < 0) goto error; + return g_steal_pointer(&test); + + error: virDomainChrSourceDefClear(&src); + return NULL; +} + + +/* + * Like qemuMonitorTestNewAgent(), but drives the real public qemuConnectAgent() + * so the transport-selection and vsock connect path actually run. The + * qemuvsockagentmock overrides virVsockConnectQuiet() to feed back an fd wired + * to @test's scripted unix mock-QGA server, so no real AF_VSOCK is involved. + * + * @driver must be the same driver @xmlopt was built for, so the domain's + * private data auto-wires priv->driver. + */ +qemuMonitorTest * +qemuMonitorTestNewAgentVsock(virQEMUDriver *driver, + virDomainXMLOption *xmlopt) +{ + g_autoptr(qemuMonitorTest) test = NULL; + virDomainChrSourceDef src = { 0 }; + qemuDomainObjPrivate *priv; + + if (!(test = qemuMonitorTestNewAgentInit(xmlopt, "agent-vsock-test", &src))) + goto error; + + /* Make the domain look active and give it a usable guest CID so + * qemuConnectAgent() picks the vsock transport. */ + test->vm->def->id = 1; + test->vm->def->name = g_strdup("agent-vsock-test"); + + if (!(test->vm->def->vsock = virDomainVsockDefNew(xmlopt))) + goto error; + test->vm->def->vsock->guest_cid = VIR_VSOCK_GUEST_CID_MIN; + + priv = QEMU_DOMAIN_PRIVATE(test->vm); + priv->agentTimeout = 5; + /* Alias the event thread without a ref: production only reads it during the + * connect (the agent takes its own context ref). Severed below before the + * private-data teardown could unref the thread qemuMonitorTestFree owns. */ + priv->eventThread = test->eventThread; + + g_setenv("LIBVIRT_VSOCK_MOCK_PATH", src.data.nix.path, true); + + /* test->vm is already locked (virDomainObjNew() returns it locked), which + * is what the connect path needs: it unlocks/relocks the vm internally. */ + + /* Drive the real connect. Do NOT ref the vm: qemuAgentOpenFd() takes its + * own reference for the agent. */ + if (qemuConnectAgent(driver, test->vm) < 0 || + !priv->agent || !priv->agentIsVsock) { + /* A graceful failure may have armed a reconnect timer holding a vm + * ref; drop it (and the thread alias) before tearing down. */ + qemuDomainCancelAgentVsockReconnect(priv); + priv->eventThread = NULL; + virObjectUnlock(test->vm); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "guest agent did not connect over vsock"); + goto error; + } + + /* Hand the agent to the test object (matches the unix path's ref accounting) + * and sever the thread alias. */ + test->agent = g_steal_pointer(&priv->agent); + priv->eventThread = NULL; + + virObjectUnlock(test->vm); + + if (qemuMonitorTestAgentStart(test, &src) < 0) + goto error; + g_unsetenv("LIBVIRT_VSOCK_MOCK_PATH"); return g_steal_pointer(&test); error: + g_unsetenv("LIBVIRT_VSOCK_MOCK_PATH"); virDomainChrSourceDefClear(&src); return NULL; } diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index c87cdf0e6a..1908e91fc0 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -99,6 +99,10 @@ qemuMonitorTestNewFromFileFull(const char *fileName, qemuMonitorTest * qemuMonitorTestNewAgent(virDomainXMLOption *xmlopt); +qemuMonitorTest * +qemuMonitorTestNewAgentVsock(virQEMUDriver *driver, + virDomainXMLOption *xmlopt); + void qemuMonitorTestFree(qemuMonitorTest *test); diff --git a/tests/qemuvsockagentmock.c b/tests/qemuvsockagentmock.c new file mode 100644 index 0000000000..d1c2760302 --- /dev/null +++ b/tests/qemuvsockagentmock.c @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2026 Virtuozzo International GmbH + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "internal.h" +#include "virvsock.h" +#include "virfile.h" +#include "virstring.h" + +#include <sys/socket.h> +#include <sys/un.h> +#include <errno.h> +#include <stdlib.h> + +/* + * Override the only genuinely vsock-specific syscall wrapper so the real + * qemuConnectAgent()/qemuChannelOpenVsock() dispatch runs under test. The + * cid/port are irrelevant here: hand back an fd wired to the scripted mock-QGA + * unix server the test set up (mirrors qemuMonitorTestOpenChannel()). + */ +int +virVsockConnectQuiet(unsigned int cid G_GNUC_UNUSED, + unsigned int port G_GNUC_UNUSED) +{ + const char *path = getenv("LIBVIRT_VSOCK_MOCK_PATH"); + struct sockaddr_un addr = { .sun_family = AF_UNIX }; + int fd; + + if (!path) { + errno = ENOENT; + return -1; + } + + if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) + return -1; + + if (virStrcpyStatic(addr.sun_path, path) < 0) { + VIR_FORCE_CLOSE(fd); + errno = ENAMETOOLONG; + return -1; + } + + if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + VIR_FORCE_CLOSE(fd); + return -1; + } + + return fd; +} -- 2.54.0