Problem: libvirt has a 5 second timeout (generally) for hotplug/unplug operations which can time out due to heavy load in guest. vcpu hotunplug occurs one vcpu at a time. But, if we perform hotplug-unplug repeatedly, there is a situation where qemu has multiple timedout vcpu unplug operations pending. libvirt waits for an async event notification from qemu regarding successful vcpu delete. qemu deletes the vcpu-vcpuinfo and sends this notification to libvirt. libvirt handles vcpu delete notification, and refreshes vcpuinfo by querying vcpuinfo from qemu in qemuDomainRefreshVcpuInfo(). qemu's vcpuinfo will not contain vcpu that was deleted and sent. qemu's vcpuinfo will overwrite libvirt's vcpuinfo of the other pending timedout vcpu(s) which qemu has deleted and notified libvirt. The overwrite resets other timedout vcpu's alias to NULL and tid to 0. The error is then seen when validating tid of vcpus. Example error log: "internal error: qemu didn't report thread id for vcpu 'XX'" Solution: Create a per vcpu hp_timeout_alias which will be set as vcpu alias when a timeout occurs else it is set as NULL. This data does not get reset when vcpuinfo refresh occurs. And we can remove a vcpu if it's alias was NULL by checking this alias. During vcpu validation, we can skip to nextvcpu if this hotplug-timeout-alias is set indicating that this vcpu is actually valid. Co-authored-by: Partha Satapathy <partha.satapathy(a)oracle.com&gt; Signed-off-by: Shaleen Bathla <shaleen.bathla(a)oracle.com&gt; --- src/qemu/qemu_domain.c | 6 ++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c3afc6c9d3ba..0f6e4dc78b93 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9568,6 +9568,8 @@ qemuDomainValidateVcpuInfo(virDomainObj *vm) vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); if (vcpu->online && vcpupriv->tid == 0) { + if (vcpupriv->hp_timeout_alias) + continue; virReportError(VIR_ERR_INTERNAL_ERROR, _("qemu didn't report thread id for vcpu '%zu'"), i); return -1; @@ -9696,6 +9698,10 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, if (validTIDs) vcpupriv->tid = info[i].tid; + if (vcpupriv->hp_timeout_alias && !info[i].alias) + VIR_DEBUG("no alias for timed out vcpu%zu domain %s", i, + vm->def->name); + vcpupriv->socket_id = info[i].socket_id; vcpupriv->core_id = info[i].core_id; vcpupriv->thread_id = info[i].thread_id; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2bbd492d62da..68f6c645ef79 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -324,6 +324,7 @@ struct _qemuDomainVcpuPrivate { int thread_id; int node_id; int vcpus; + char *hp_timeout_alias; /* alias after hotplug timeout */ char *qomPath; }; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index da92ced2f444..5d54e9c67c3f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6138,9 +6138,21 @@ qemuDomainRemoveVcpuAlias(virDomainObj *vm, if (STREQ_NULLABLE(alias, vcpupriv->alias)) { qemuDomainRemoveVcpu(vm, i); + if (vcpupriv->hp_timeout_alias) + g_free(vcpupriv->hp_timeout_alias); + return; + } + if (STREQ_NULLABLE(alias, vcpupriv->hp_timeout_alias)) { + qemuDomainRemoveVcpu(vm, i); + VIR_DEBUG("%s found in timed out list of domain %s", + alias, vm->def->name); + g_free(vcpupriv->hp_timeout_alias); return; } } + + VIR_DEBUG("%s not found in vcpulist of domain %s ", + alias, vm->def->name); } @@ -6172,11 +6184,19 @@ qemuDomainHotplugDelVcpu(virQEMUDriver *driver, } if ((rc = qemuDomainWaitForDeviceRemoval(vm)) <= 0) { - if (rc == 0) + if (rc == 0) { virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("vcpu unplug request timed out. Unplug result " "must be manually inspected in the domain")); + if (vcpupriv) { + VIR_INFO("unplug timeout set for vcpu '%u' of domain %s", + vcpu, vm->def->name); + + vcpupriv->hp_timeout_alias = g_strdup(vcpupriv->alias); + } + } + goto cleanup; } @@ -6362,6 +6382,8 @@ qemuDomainSetVcpusLive(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; virCgroupEmulatorAllNodesData *emulatorCgroup = NULL; + qemuDomainVcpuPrivate *vcpupriv = NULL; + virDomainVcpuDef *vcpuinfo = NULL; ssize_t nextvcpu = -1; int ret = -1; @@ -6370,6 +6392,14 @@ qemuDomainSetVcpusLive(virQEMUDriver *driver, if (enable) { while ((nextvcpu = virBitmapNextSetBit(vcpumap, nextvcpu)) != -1) { + vcpuinfo = virDomainDefGetVcpu(vm->def, nextvcpu); + vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpuinfo); + + if (vcpupriv && vcpupriv->hp_timeout_alias) { + VIR_INFO("reject hotplug of timed out vcpu '%zd' from domain %s", + nextvcpu, vm->def->name); + goto cleanup; + } if (qemuDomainHotplugAddVcpu(driver, cfg, vm, nextvcpu) < 0) goto cleanup; } @@ -6378,6 +6408,15 @@ qemuDomainSetVcpusLive(virQEMUDriver *driver, if (!virBitmapIsBitSet(vcpumap, nextvcpu)) continue; + vcpuinfo = virDomainDefGetVcpu(vm->def, nextvcpu); + vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpuinfo); + + if (vcpupriv && vcpupriv->hp_timeout_alias) { + VIR_INFO("already serving unplug for vcpu '%zd' in domain %s", + nextvcpu, vm->def->name); + goto cleanup; + } + if (qemuDomainHotplugDelVcpu(driver, cfg, vm, nextvcpu) < 0) goto cleanup; } -- 2.31.1