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