From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; This code was based on a per-helper instance and peer-to-peer connections. The code that landed in qemu master for v5.0 is relying on a single instance and DBus bus. Instead of trying to adapt the existing dbus-vmstate code, let's remove it and resubmit. That should make reviewing easier. Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/Makefile.inc.am | 2 - src/qemu/qemu_alias.c | 16 ----- src/qemu/qemu_alias.h | 3 - src/qemu/qemu_command.c | 83 ------------------------- src/qemu/qemu_command.h | 3 - src/qemu/qemu_dbus.c | 94 ---------------------------- src/qemu/qemu_dbus.h | 42 ------------- src/qemu/qemu_domain.c | 13 ---- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_extdevice.c | 4 +- src/qemu/qemu_hotplug.c | 83 +------------------------ src/qemu/qemu_hotplug.h | 11 ---- src/qemu/qemu_migration.c | 8 --- src/qemu/qemu_slirp.c | 125 +------------------------------------- src/qemu/qemu_slirp.h | 4 +- 15 files changed, 7 insertions(+), 485 deletions(-) delete mode 100644 src/qemu/qemu_dbus.c delete mode 100644 src/qemu/qemu_dbus.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 967f6e75a2..028ab9043c 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -13,8 +13,6 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_capabilities.h \ qemu/qemu_command.c \ qemu/qemu_command.h \ - qemu/qemu_dbus.c \ - qemu/qemu_dbus.h \ qemu/qemu_domain.c \ qemu/qemu_domain.h \ qemu/qemu_domain_address.c \ diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 93bdcb7548..61f8ce05c9 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -840,19 +840,3 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias) return ret; } - -char * -qemuAliasDBusVMStateFromId(const char *id) -{ - char *ret; - size_t i; - - ret = g_strdup_printf("dbus-vms-%s", id); - - for (i = 0; ret[i]; i++) { - if (ret[i] == ':') - ret[i] = '_'; - } - - return ret; -} diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index ae2fce16bc..aaac09a1d1 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -95,6 +95,3 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias) const char *qemuDomainGetManagedPRAlias(void); char *qemuDomainGetUnmanagedPRAlias(const char *parentalias); - -char *qemuAliasDBusVMStateFromId(const char *id) - ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 904d2beab5..7429a0b7f5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -27,7 +27,6 @@ #include "qemu_interface.h" #include "qemu_alias.h" #include "qemu_security.h" -#include "qemu_dbus.h" #include "qemu_slirp.h" #include "qemu_block.h" #include "cpu/cpu.h" @@ -9459,85 +9458,6 @@ qemuBuildPflashBlockdevCommandLine(virCommandPtr cmd, } -static virJSONValuePtr -qemuBuildDBusVMStateInfoPropsInternal(const char *alias, - const char *addr) -{ - virJSONValuePtr ret = NULL; - - if (qemuMonitorCreateObjectProps(&ret, - "dbus-vmstate", alias, - "s:addr", addr, NULL) < 0) - return NULL; - - return ret; -} - - -virJSONValuePtr -qemuBuildDBusVMStateInfoProps(const char *id, - const char *addr) -{ - g_autofree char *alias = qemuAliasDBusVMStateFromId(id); - - if (!alias) - return NULL; - - return qemuBuildDBusVMStateInfoPropsInternal(alias, addr); -} - - -typedef struct qemuBuildDBusVMStateCommandLineData { - virCommandPtr cmd; -} qemuBuildDBusVMStateCommandLineData; - - -static int -qemuBuildDBusVMStateCommandLineEach(void *payload, - const void *id, - void *user_data) -{ - qemuBuildDBusVMStateCommandLineData *data = user_data; - qemuDBusVMStatePtr vms = payload; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - g_autoptr(virJSONValue) props = NULL; - - if (!(props = qemuBuildDBusVMStateInfoProps(id, vms->addr))) - return -1; - - if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) - return -1; - - virCommandAddArg(data->cmd, "-object"); - virCommandAddArgBuffer(data->cmd, &buf); - - return 0; -} - -static int -qemuBuildDBusVMStateCommandLine(virCommandPtr cmd, - qemuDomainObjPrivatePtr priv) -{ - qemuBuildDBusVMStateCommandLineData data = { - .cmd = cmd, - }; - - if (virHashSize(priv->dbusVMStates) == 0) - return 0; - - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("dbus-vmstate object is not supported by this QEMU binary")); - return 0; - } - - if (virHashForEach(priv->dbusVMStates, qemuBuildDBusVMStateCommandLineEach, &data) < 0) - return -1; - - return 0; -} - - /** * qemuBuildCommandLineValidate: * @@ -9769,9 +9689,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) return NULL; - if (qemuBuildDBusVMStateCommandLine(cmd, priv) < 0) - return NULL; - if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) return NULL; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 786991fd3d..cb8c394fb6 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -63,9 +63,6 @@ virJSONValuePtr qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr priv) int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, virJSONValuePtr *propsret); -virJSONValuePtr qemuBuildDBusVMStateInfoProps(const char *id, - const char *addr); - /* Generate the object properties for a tls-creds-x509 */ int qemuBuildTLSx509BackendProps(const char *tlspath, bool isListen, diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c deleted file mode 100644 index faee122021..0000000000 --- a/src/qemu/qemu_dbus.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * qemu_dbus.c: QEMU DBus-related helpers - * - * 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 "qemu_extdevice.h" -#include "qemu_dbus.h" -#include "qemu_hotplug.h" -#include "qemu_security.h" - -#include "viralloc.h" -#include "virlog.h" -#include "virstring.h" -#include "virtime.h" -#include "virpidfile.h" - -#define VIR_FROM_THIS VIR_FROM_QEMU - -VIR_LOG_INIT("qemu.dbus"); - - -qemuDBusVMStatePtr -qemuDBusVMStateNew(const char *id, const char *addr) -{ - g_autoptr(qemuDBusVMState) self = NULL; - - if (VIR_ALLOC(self) < 0) - return NULL; - - self->id = g_strdup(id); - self->addr = g_strdup(addr); - - return g_steal_pointer(&self); -} - - -void -qemuDBusVMStateFree(qemuDBusVMStatePtr self) -{ - if (!self) - return; - - VIR_FREE(self->id); - VIR_FREE(self->addr); - VIR_FREE(self); -} - - -int -qemuDBusVMStateAdd(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *id, const char *addr, bool hot) -{ - qemuDBusVMStatePtr d = qemuDBusVMStateNew(id, addr); - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (virHashAddEntry(priv->dbusVMStates, id, d) < 0) { - qemuDBusVMStateFree(d); - return -1; - } - - if (hot && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE) && - qemuDomainAttachDBusVMState(driver, vm, id, addr, QEMU_ASYNC_JOB_NONE) < 0) - return -1; - - return 0; -} - - -void -qemuDBusVMStateRemove(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *id, bool hot) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (virHashRemoveEntry(priv->dbusVMStates, id) < 0 || - (hot && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE) && - qemuDomainDetachDBusVMState(driver, vm, id, QEMU_ASYNC_JOB_NONE) < 0)) - VIR_ERROR(_("Failed to remove vmstate id '%s'"), vm->def->name); -} diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h deleted file mode 100644 index ccbeb83709..0000000000 --- a/src/qemu/qemu_dbus.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * qemu_dbus.h: QEMU DBus-related helpers - * - * 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/>;. - */ - -#pragma once - -#include "qemu_conf.h" -#include "qemu_domain.h" - -typedef struct _qemuDBusVMState qemuDBusVMState; -typedef qemuDBusVMState *qemuDBusVMStatePtr; -struct _qemuDBusVMState { - char *id; - char *addr; -}; - - -qemuDBusVMStatePtr qemuDBusVMStateNew(const char *id, const char *addr); - -void qemuDBusVMStateFree(qemuDBusVMStatePtr self); - -int qemuDBusVMStateAdd(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *id, const char *addr, bool hot); - -void qemuDBusVMStateRemove(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *id, bool hot); - -G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDBusVMState, qemuDBusVMStateFree); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a6dde15bad..ecd087a5cb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -26,7 +26,6 @@ #include "qemu_block.h" #include "qemu_cgroup.h" #include "qemu_command.h" -#include "qemu_dbus.h" #include "qemu_process.h" #include "qemu_capabilities.h" #include "qemu_hostdev.h" @@ -2164,13 +2163,6 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, } -static void -dbusVMStateHashFree(void *opaque) -{ - qemuDBusVMStateFree(opaque); -} - - static void * qemuDomainObjPrivateAlloc(void *opaque) { @@ -2191,9 +2183,6 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData))) goto error; - if (!(priv->dbusVMStates = virHashCreate(5, dbusVMStateHashFree))) - goto error; - /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; @@ -2264,7 +2253,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->migrationCaps = NULL; virHashRemoveAll(priv->blockjobs); - virHashRemoveAll(priv->dbusVMStates); virObjectUnref(priv->pflash0); priv->pflash0 = NULL; @@ -2308,7 +2296,6 @@ qemuDomainObjPrivateFree(void *data) qemuDomainMasterKeyFree(priv); virHashFree(priv->blockjobs); - virHashFree(priv->dbusVMStates); VIR_FREE(priv); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c6afc484f6..b2041ccea7 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -407,7 +407,6 @@ struct _qemuDomainObjPrivate { /* running block jobs */ virHashTablePtr blockjobs; - virHashTablePtr dbusVMStates; bool disableSlirp; /* Until we add full support for backing chains for pflash drives, these diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 463f76c21a..beb7adfe0c 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -180,7 +180,7 @@ qemuExtDevicesStart(virQEMUDriverPtr driver, qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; if (slirp && - qemuSlirpStart(slirp, vm, driver, net, false, incomingMigration) < 0) + qemuSlirpStart(slirp, vm, driver, net, incomingMigration) < 0) return -1; } @@ -213,7 +213,7 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; if (slirp) - qemuSlirpStop(slirp, vm, driver, net, false); + qemuSlirpStop(slirp, vm, driver, net); } } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 31d455505b..c5a4db3e79 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -403,83 +403,6 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, } -/** - * qemuDomainAttachDBusVMState: - * @driver: QEMU driver object - * @vm: domain object - * @id - * @addr - * @asyncJob: asynchronous job identifier - * - * Add dbus-vmstate object. - * - * Returns: 0 on success, -1 on error. - */ -int -qemuDomainAttachDBusVMState(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *id, - const char *addr, - qemuDomainAsyncJob asyncJob) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - g_autoptr(virJSONValue) props = NULL; - int ret; - - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("dbus-vmstate object is not supported by this QEMU binary")); - return -1; - } - - if (!(props = qemuBuildDBusVMStateInfoProps(id, addr))) - return -1; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - - ret = qemuMonitorAddObject(priv->mon, &props, NULL); - - if (qemuDomainObjExitMonitor(driver, vm) < 0) - return -1; - - return ret; -} - - -/** - * qemuDomainDetachDBusVMState: - * @driver: QEMU driver object - * @vm: domain object - * @asyncJob: asynchronous job identifier - * - * Remove dbus-vmstate object from @vm. - * - * Returns: 0 on success, -1 on error. - */ -int -qemuDomainDetachDBusVMState(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *id, - qemuDomainAsyncJob asyncJob) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - g_autofree char *alias = qemuAliasDBusVMStateFromId(id); - int ret; - - if (!alias || - qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - - ret = qemuMonitorDelObject(priv->mon, alias); - - if (qemuDomainObjExitMonitor(driver, vm) < 0) - return -1; - - return ret; -} - - /** * qemuDomainChangeMediaBlockdev: * @driver: qemu driver structure @@ -1301,7 +1224,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; if (qemuSlirpOpen(slirp, driver, vm->def) < 0 || - qemuSlirpStart(slirp, vm, driver, net, true, NULL) < 0) { + qemuSlirpStart(slirp, vm, driver, net, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to start slirp")); goto cleanup; @@ -1508,7 +1431,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virErrorPreserveLast(&originalError); netdev_name = g_strdup_printf("host%s", net->info.alias); if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) - qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true); + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net); qemuDomainObjEnterMonitor(driver, vm); if (charDevPlugged && qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) @@ -4575,7 +4498,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, return -1; if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) - qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true); + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net); virDomainAuditNet(vm, net, NULL, "detach", true); diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 1dfc601110..6605a6a3e0 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -151,15 +151,4 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, virBitmapPtr vcpus, bool state); -int qemuDomainAttachDBusVMState(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *id, - const char *addr, - qemuDomainAsyncJob asyncJob); - -int qemuDomainDetachDBusVMState(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *id, - qemuDomainAsyncJob asyncJob); - unsigned long long qemuDomainGetUnplugTimeout(virDomainObjPtr vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 29d228a8d9..71d0bb0879 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1125,7 +1125,6 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, bool remote, unsigned int flags) { - qemuDomainObjPrivatePtr priv = vm->privateData; int nsnapshots; int pauseReason; size_t i; @@ -1154,13 +1153,6 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, } } - if (virHashSize(priv->dbusVMStates) > 0 && - !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain requires dbus-vmstate support")); - return false; - } - for (i = 0; i < vm->def->nnets; i++) { virDomainNetDefPtr net = vm->def->nets[i]; qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index 5266b36eaa..8e001f0d10 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -18,7 +18,6 @@ #include <config.h> -#include "qemu_dbus.h" #include "qemu_extdevice.h" #include "qemu_security.h" #include "qemu_slirp.h" @@ -203,48 +202,14 @@ qemuSlirpGetFD(qemuSlirpPtr slirp) } -static char * -qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net) -{ - char macstr[VIR_MAC_STRING_BUFLEN] = ""; - char *id = NULL; - - /* can't use alias, because it's not stable across restarts */ - id = g_strdup_printf("slirp-%s", virMacAddrFormat(&net->mac, macstr)); - - return id; -} - - -static char * -qemuSlirpGetDBusPath(virQEMUDriverConfigPtr cfg, - const virDomainDef *def, - const char *alias) -{ - g_autofree char *shortName = NULL; - char *path = NULL; - - if (!(shortName = virDomainDefGetShortName(def))) - return NULL; - - path = g_strdup_printf("%s/%s-%s-slirp", - cfg->slirpStateDir, shortName, alias); - - return path; -} - - void qemuSlirpStop(qemuSlirpPtr slirp, virDomainObjPtr vm, virQEMUDriverPtr driver, - virDomainNetDefPtr net, - bool hot) + virDomainNetDefPtr net) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *pidfile = NULL; - g_autofree char *dbus_path = NULL; - g_autofree char *id = qemuSlirpGetDBusVMStateId(net); virErrorPtr orig_err; pid_t pid; int rc; @@ -254,12 +219,6 @@ qemuSlirpStop(qemuSlirpPtr slirp, return; } - if (id) { - qemuDBusVMStateRemove(driver, vm, id, hot); - } else { - VIR_WARN("Unable to construct vmstate id"); - } - virErrorPreserveLast(&orig_err); rc = virPidFileReadPathIfAlive(pidfile, &pid, cfg->slirpHelperName); if (rc >= 0 && pid != (pid_t) -1) @@ -273,18 +232,6 @@ qemuSlirpStop(qemuSlirpPtr slirp, } slirp->pid = 0; - dbus_path = qemuSlirpGetDBusPath(cfg, vm->def, net->info.alias); - if (dbus_path) { - if (unlink(dbus_path) < 0 && - errno != ENOENT) { - virReportSystemError(errno, - _("Unable to remove stale dbus socket %s"), - dbus_path); - } - } else { - VIR_WARN("Unable to construct dbus socket path"); - } - virErrorRestore(&orig_err); } @@ -294,17 +241,12 @@ qemuSlirpStart(qemuSlirpPtr slirp, virDomainObjPtr vm, virQEMUDriverPtr driver, virDomainNetDefPtr net, - bool hotplug, bool incoming) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virCommand) cmd = NULL; g_autofree char *pidfile = NULL; - g_autofree char *dbus_path = NULL; - g_autofree char *dbus_addr = NULL; - g_autofree char *id = NULL; size_t i; - const unsigned long long timeout = 5 * 1000; /* ms */ pid_t pid = (pid_t) -1; int rc; int exitstatus = 0; @@ -368,29 +310,6 @@ qemuSlirpStart(qemuSlirpPtr slirp, } } - if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_P2P)) { - if (!(id = qemuSlirpGetDBusVMStateId(net))) - return -1; - - if (!(dbus_path = qemuSlirpGetDBusPath(cfg, vm->def, net->info.alias))) - return -1; - - if (unlink(dbus_path) < 0 && errno != ENOENT) { - virReportSystemError(errno, _("Unable to unlink %s"), dbus_path); - return -1; - } - - dbus_addr = g_strdup_printf("unix:path=%s", dbus_path); - - virCommandAddArgFormat(cmd, "--dbus-id=%s", id); - - virCommandAddArgFormat(cmd, "--dbus-p2p=%s", dbus_addr); - - if (incoming && - qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) - virCommandAddArg(cmd, "--dbus-incoming"); - } - if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT)) virCommandAddArg(cmd, "--exit-with-parent"); @@ -414,46 +333,6 @@ qemuSlirpStart(qemuSlirpPtr slirp, goto error; } - if (dbus_path) { - virTimeBackOffVar timebackoff; - - if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0) - goto error; - - while (virTimeBackOffWait(&timebackoff)) { - char errbuf[1024] = { 0 }; - - if (virFileExists(dbus_path)) - break; - - if (virProcessKill(pid, 0) == 0) - continue; - - if (saferead(errfd, errbuf, sizeof(errbuf) - 1) < 0) { - virReportSystemError(errno, - _("slirp helper %s died unexpectedly"), - cfg->prHelperName); - } else { - virReportError(VIR_ERR_OPERATION_FAILED, - _("slirp helper died and reported: %s"), errbuf); - } - goto error; - } - - if (!virFileExists(dbus_path)) { - virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", - _("slirp dbus socket did not show up")); - goto error; - } - } - - if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE) && - qemuDBusVMStateAdd(driver, vm, id, dbus_addr, hotplug) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to register slirp migration")); - goto error; - } - slirp->pid = pid; return 0; @@ -462,7 +341,5 @@ qemuSlirpStart(qemuSlirpPtr slirp, virProcessKillPainfully(pid, true); if (pidfile) unlink(pidfile); - if (dbus_path) - unlink(dbus_path); return -1; } diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h index 5e80e79b15..5bf9596053 100644 --- a/src/qemu/qemu_slirp.h +++ b/src/qemu/qemu_slirp.h @@ -66,14 +66,12 @@ int qemuSlirpStart(qemuSlirpPtr slirp, virDomainObjPtr vm, virQEMUDriverPtr driver, virDomainNetDefPtr net, - bool hot, bool incoming); void qemuSlirpStop(qemuSlirpPtr slirp, virDomainObjPtr vm, virQEMUDriverPtr driver, - virDomainNetDefPtr net, - bool hot); + virDomainNetDefPtr net); int qemuSlirpGetFD(qemuSlirpPtr slirp); -- 2.25.0.rc2.1.g09a9a1a997

On 1/14/20 2:46 PM, marcandre.lureau(a)redhat.com wrote: Oops, s/slirpHelperName/dbusDaemonName/ :-D Michal

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; Location of DBus daemon state configuration, socket, pid... Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e1fea390c7..ade12dac6c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -144,6 +144,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", LOCALSTATEDIR); + cfg->dbusStateDir = g_strdup_printf("%s/run/libvirt/qemu/dbus", LOCALSTATEDIR); + cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu", LOCALSTATEDIR); cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir); cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir); @@ -174,6 +176,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir); cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir); + cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir); cfg->configBaseDir = virGetUserConfigDirectory(); @@ -274,6 +277,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->stateDir); VIR_FREE(cfg->swtpmStateDir); VIR_FREE(cfg->slirpStateDir); + VIR_FREE(cfg->dbusStateDir); VIR_FREE(cfg->libDir); VIR_FREE(cfg->cacheDir); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 6354925e0b..de6430cf97 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -97,6 +97,7 @@ struct _virQEMUDriverConfig { char *stateDir; char *swtpmStateDir; char *slirpStateDir; + char *dbusStateDir; /* These two directories are ones QEMU processes use (so must match * the QEMU user/group */ char *libDir; -- 2.25.0.rc2.1.g09a9a1a997

Hi On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: Makes sense. I think I based this on slirpStateDir code. Any reason it's not using vm->priv->libdir too?

Hi On Tue, Feb 25, 2020 at 12:24 PM Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: Actually, $cfg->libDir is virQEMUDriverConfigNew () cfg->configBaseDir/qemu/lib, so it ends up under ~/.config for users, and /etc for priviledged/root. A bad place to store transient runtime data/sockets. You may want to reconsider the paths for pr-helper. I don't intend to change that, so you could take a look at "[libvirt PATCH v2 0/9]".

On 2/25/20 6:24 AM, Michal Privoznik wrote: It would need to be done carefully, but it's not out of the question: There was previously a case where we had been storing the network state files in an incorrect location (/var/lib/libvirt) (starting in commit 23a090ab92 - libvirt-0.6.0 in 2009), and moved them to a more proper location (/var/run/libvirt) (commit b9e95491d1 - libvirt 1.2.4 in 2014). At the time the move was made, we also added a code to networkStateInitialize() that looked for files in the old location and moved them to the new location. This code remained in place, at first doing its job whenever someone upgraded, and then silently doing nothing for a very long time, until 5 years later when it was removed in commit 43be65a4817f - libvirt 5.1.0. (probably could have been removed much earlier than that, but it had just been forgotten.)

On 1/14/20 2:46 PM, marcandre.lureau(a)redhat.com wrote: These can be added where they were just a moment ago ;-) Instead of having each caller pass cfg->dbusStateDir, we can accept cfg here and deref ourselves. I'd introduce qemuDBusCreateConfPath() so that nobody else has to call qemuDBusCreateFilename() again. After the daemon startup is reworked, this function is no longer needed. 'make syntax-check' complains about these leading spaces. Pff. No need to specifically kill the process. virPidFileForceCleanupPath() does that. Just like it unlinks the pidfile. We can do this only in success path. s/int/VIR_AUTOCLOSE/ to make sure the FD is closed when returning from the function. This can fail and this the retval needs to be checked for. There is no need to read the pid in every iteration. The virCommandRun() (wrapped in qemuSecurityCommandRun()) will write the pidfile right before exec(). So I think this loop should look a bit different: while (virTimeBackOffWait(&timebackoff)) { if (virFileExists(sockpath)) break; if (pid still exists) continue; read the error; goto cleanup; } Oh, and we will need cleanup label, so that in case of failure happening after we've forked off a child, we make sure to kill it. This looks like a perfect place to put the children process into CGroup. You define a function for that below but never call it ;-) Interesting, qemuDBusConnect() is declared but never defined and my gcc doesn't complain (!). This shouldn't be necessary. If the qemuDBusStop() is called I don't think this is needed - you are not introducing anything DBus related in qemu_domain.h. If this is dropped then the Makefile.am change done below can be dropped too. Michal

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; This avoids trying to start a dbus-daemon when its already running. Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7722a53c62..dda3cb781f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } + if (priv->dbusDaemonRunning) + virBufferAddLit(buf, "<dbusDaemon/>\n"); + if (priv->namespaces) { ssize_t ns = -1; @@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; } + priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0; + if ((node = virXPathNode("./namespaces", ctxt))) { xmlNodePtr next; -- 2.25.0.rc2.1.g09a9a1a997

Hi On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: Well, as we introduce DBus bus for the VM, it would be a foundation for IPC/multi-process communication, not specific to slirp. So I'd leave it near the top.

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_process.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4195042194..845a7caa55 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -58,6 +58,7 @@ #include "qemu_extdevice.h" #include "qemu_firmware.h" #include "qemu_backup.h" +#include "qemu_dbus.h" #include "cpu/cpu.h" #include "cpu/cpu_x86.h" @@ -6457,6 +6458,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + if (qemuDBusPrepareHost(driver) < 0) + return -1; + if (qemuPrepareNVRAM(cfg, vm) < 0) return -1; @@ -7378,6 +7382,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuExtDevicesStop(driver, vm); + qemuDBusStop(driver, vm); + vm->def->id = -1; /* Stop autodestroy in case guest is restarted */ -- 2.25.0.rc2.1.g09a9a1a997

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; Helper processes may have their state migrated with QEMU data stream thanks to the QEMU "dbus-vmstate". libvirt maintains the list of helpers to be migrated. The "dbus-vmstate" is added when required, and given the list of helper Ids that must be migrated, on save & load sides. See also: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_alias.c | 7 +++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 62 +++++++++++++++++++++++++++ src/qemu/qemu_command.h | 3 ++ src/qemu/qemu_dbus.c | 14 ++++++ src/qemu/qemu_dbus.h | 4 ++ src/qemu/qemu_domain.c | 10 +++++ src/qemu/qemu_domain.h | 5 +++ src/qemu/qemu_hotplug.c | 82 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 8 ++++ src/qemu/qemu_migration.c | 51 ++++++++++++++++++++++ src/qemu/qemu_monitor.c | 21 +++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 15 +++++++ src/qemu/qemu_monitor_json.h | 5 +++ 15 files changed, 292 insertions(+) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 61f8ce05c9..d2e1ce155e 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -840,3 +840,10 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias) return ret; } + + +const char * +qemuDomainGetDBusVMStateAlias(void) +{ + return "dbus-vmstate0"; +} diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index aaac09a1d1..e3492116c5 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -95,3 +95,5 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias) const char *qemuDomainGetManagedPRAlias(void); char *qemuDomainGetUnmanagedPRAlias(const char *parentalias); + +const char *qemuDomainGetDBusVMStateAlias(void); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7429a0b7f5..53051a8726 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -24,6 +24,7 @@ #include "qemu_command.h" #include "qemu_hostdev.h" #include "qemu_capabilities.h" +#include "qemu_dbus.h" #include "qemu_interface.h" #include "qemu_alias.h" #include "qemu_security.h" @@ -9458,6 +9459,64 @@ qemuBuildPflashBlockdevCommandLine(virCommandPtr cmd, } +static virJSONValuePtr +qemuBuildDBusVMStateInfoPropsInternal(const char *alias, + const char *addr) +{ + virJSONValuePtr ret = NULL; + + if (qemuMonitorCreateObjectProps(&ret, + "dbus-vmstate", alias, + "s:addr", addr, NULL) < 0) + return NULL; + + return ret; +} + + +virJSONValuePtr +qemuBuildDBusVMStateInfoProps(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_autofree char *addr = qemuDBusGetAddress(driver, vm); + + return qemuBuildDBusVMStateInfoPropsInternal(qemuDomainGetDBusVMStateAlias(), + addr); +} + + +static int +qemuBuildDBusVMStateCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autoptr(virJSONValue) props = NULL; + qemuDomainObjPrivatePtr priv = QEMU_DOMAIN_PRIVATE(vm); + + if (virStringListLength((const char **)priv->dbusVMStateIds) == 0) + return 0; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { + VIR_INFO("dbus-vmstate object is not supported by this QEMU binary"); + return 0; + } + + if (!(props = qemuBuildDBusVMStateInfoProps(driver, vm))) + return -1; + + if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) + return -1; + + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); + + priv->dbusVMState = true; + + return 0; +} + + /** * qemuBuildCommandLineValidate: * @@ -9689,6 +9748,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) return NULL; + if (qemuBuildDBusVMStateCommandLine(cmd, driver, vm) < 0) + return NULL; + if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) return NULL; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index cb8c394fb6..239d3daede 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -59,6 +59,9 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, virJSONValuePtr qemuBuildPRManagerInfoProps(virStorageSourcePtr src); virJSONValuePtr qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr priv); +virJSONValuePtr qemuBuildDBusVMStateInfoProps(virQEMUDriverPtr driver, + virDomainObjPtr vm); + /* Generate the object properties for a secret */ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, virJSONValuePtr *propsret); diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c index 9c8a03c947..911713944b 100644 --- a/src/qemu/qemu_dbus.c +++ b/src/qemu/qemu_dbus.c @@ -297,3 +297,17 @@ qemuDBusSetupCgroup(virQEMUDriverPtr driver, return 0; } + + +int +qemuDBusVMStateAdd(virDomainObjPtr vm, const char *id) +{ + return virStringListAdd(&QEMU_DOMAIN_PRIVATE(vm)->dbusVMStateIds, id); +} + + +void +qemuDBusVMStateRemove(virDomainObjPtr vm, const char *id) +{ + virStringListRemove(&QEMU_DOMAIN_PRIVATE(vm)->dbusVMStateIds, id); +} diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h index 8728824bd7..e86134b2a1 100644 --- a/src/qemu/qemu_dbus.h +++ b/src/qemu/qemu_dbus.h @@ -38,3 +38,7 @@ void qemuDBusStop(virQEMUDriverPtr driver, int qemuDBusSetupCgroup(virQEMUDriverPtr driver, virDomainDefPtr def, virCgroupPtr cgroup); + +int qemuDBusVMStateAdd(virDomainObjPtr vm, const char *id); + +void qemuDBusVMStateRemove(virDomainObjPtr vm, const char *id); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dda3cb781f..81dcedcc04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2266,6 +2266,11 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) qemuDomainStorageIdReset(priv); priv->dbusDaemonRunning = false; + + virStringListFree(priv->dbusVMStateIds); + priv->dbusVMStateIds = NULL; + + priv->dbusVMState = false; } @@ -2919,6 +2924,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, if (priv->dbusDaemonRunning) virBufferAddLit(buf, "<dbusDaemon/>\n"); + if (priv->dbusVMState) + virBufferAddLit(buf, "<dbusVMState/>\n"); + if (priv->namespaces) { ssize_t ns = -1; @@ -3702,6 +3710,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0; + priv->dbusVMState = virXPathBoolean("boolean(./dbusVMState)", ctxt) > 0; + if ((node = virXPathNode("./namespaces", ctxt))) { xmlNodePtr next; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 02c792ea2a..8de436fc9a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -420,6 +420,11 @@ struct _qemuDomainObjPrivate { virDomainBackupDefPtr backup; bool dbusDaemonRunning; + + /* list of Ids to migrate */ + char **dbusVMStateIds; + /* true if -object dbus-vmstate was added */ + bool dbusVMState; }; #define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c5a4db3e79..4563e3c77a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -311,6 +311,88 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver, } +/** + * qemuHotplugAttachDBusVMState: + * @driver: QEMU driver object + * @vm: domain object + * @asyncJob: asynchronous job identifier + * + * Add -object dbus-vmstate if necessary. + * + * Returns: 0 on success, -1 on error. + */ +int +qemuHotplugAttachDBusVMState(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + g_autoptr(virJSONValue) props = NULL; + int ret; + + if (priv->dbusVMState) + return 0; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { + VIR_INFO("dbus-vmstate object is not supported by this QEMU binary"); + return 0; + } + + if (!(props = qemuBuildDBusVMStateInfoProps(driver, vm))) + return -1; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorAddObject(priv->mon, &props, NULL); + + if (ret == 0) + priv->dbusVMState = true; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return ret; +} + + +/** + * qemuHotplugRemoveDBusVMState: + * @driver: QEMU driver object + * @vm: domain object + * @asyncJob: asynchronous job identifier + * + * Remove -object dbus-vmstate from @vm if the configuration does not require + * it any more. + * + * Returns: 0 on success, -1 on error. + */ +int +qemuHotplugRemoveDBusVMState(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (!priv->dbusVMState) + return 0; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorDelObject(priv->mon, qemuDomainGetDBusVMStateAlias()); + + if (ret == 0) + priv->dbusVMState = false; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return ret; +} + + /** * qemuHotplugAttachManagedPR: * @driver: QEMU driver object diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 6605a6a3e0..4a49e04a15 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -152,3 +152,11 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, bool state); unsigned long long qemuDomainGetUnplugTimeout(virDomainObjPtr vm); + +int qemuHotplugAttachDBusVMState(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); + +int qemuHotplugRemoveDBusVMState(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 71d0bb0879..8c281f3a70 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1125,10 +1125,18 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, bool remote, unsigned int flags) { + qemuDomainObjPrivatePtr priv = vm->privateData; int nsnapshots; int pauseReason; size_t i; + if (virStringListLength((const char **)priv->dbusVMStateIds) && + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot migrate this domain without dbus-vmstate support")); + return false; + } + /* perform these checks only when migrating to remote hosts */ if (remote) { nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0); @@ -1893,8 +1901,14 @@ qemuMigrationDstRun(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; + rv = qemuMonitorSetDBusVMStateIdList(priv->mon, + (const char **)priv->dbusVMStateIds); + if (rv < 0) + goto exit_monitor; + rv = qemuMonitorMigrateIncoming(priv->mon, uri); + exit_monitor: if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) return -1; @@ -3369,6 +3383,37 @@ qemuMigrationSrcContinue(virQEMUDriverPtr driver, } +static int +qemuMigrationSetDBusVMState(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (virStringListLength((const char **)priv->dbusVMStateIds) > 0) { + int rv; + + if (qemuHotplugAttachDBusVMState(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + rv = qemuMonitorSetDBusVMStateIdList(priv->mon, + (const char **)priv->dbusVMStateIds); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + rv = -1; + + return rv; + } else { + if (qemuHotplugRemoveDBusVMState(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + } + + return 0; +} + + static int qemuMigrationSrcRun(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3521,6 +3566,9 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, } } + if (qemuMigrationSetDBusVMState(driver, vm) < 0) + goto exit_monitor; + /* Before EnterMonitor, since already qemuProcessStopCPUs does that */ if (!(flags & VIR_MIGRATE_LIVE) && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -5204,6 +5252,9 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, char *errbuf = NULL; virErrorPtr orig_err = NULL; + if (qemuMigrationSetDBusVMState(driver, vm) < 0) + return -1; + /* Increase migration bandwidth to unlimited since target is a file. * Failure to change migration speed is not fatal. */ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ccd20b3740..191dbe4d02 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -27,6 +27,7 @@ #include <unistd.h> #include <fcntl.h> +#include "qemu_alias.h" #include "qemu_monitor.h" #include "qemu_monitor_text.h" #include "qemu_monitor_json.h" @@ -2407,6 +2408,26 @@ qemuMonitorSavePhysicalMemory(qemuMonitorPtr mon, } +int +qemuMonitorSetDBusVMStateIdList(qemuMonitorPtr mon, + const char **list) +{ + g_autofree char *path = NULL; + + VIR_DEBUG("list=%p", list); + + if (virStringListLength(list) == 0) + return 0; + + path = g_strdup_printf("/objects/%s", + qemuDomainGetDBusVMStateAlias()); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSetDBusVMStateIdList(mon, path, list); +} + + int qemuMonitorSetMigrationSpeed(qemuMonitorPtr mon, unsigned long bandwidth) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 3f3b81cddd..9d82d263c3 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -740,6 +740,9 @@ int qemuMonitorSavePhysicalMemory(qemuMonitorPtr mon, unsigned long long length, const char *path); +int qemuMonitorSetDBusVMStateIdList(qemuMonitorPtr mon, + const char **list); + int qemuMonitorSetMigrationSpeed(qemuMonitorPtr mon, unsigned long bandwidth); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e5164d218a..42795b4227 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2342,6 +2342,21 @@ qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, } +int +qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon, + const char *vmstatepath, + const char **list) +{ + g_autofree char *str = virStringListJoin(list, ","); + qemuMonitorJSONObjectProperty prop = { + .type = QEMU_MONITOR_OBJECT_PROPERTY_STRING, + .val.str = str, + }; + + return qemuMonitorJSONSetObjectProperty(mon, vmstatepath, "id-list", &prop); +} + + /* qemuMonitorJSONQueryBlock: * @mon: Monitor pointer * diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 61f5b0061d..e7f02d1131 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -679,3 +679,8 @@ qemuMonitorJSONTransactionBackup(virJSONValuePtr actions, const char *target, const char *bitmap, qemuMonitorTransactionBackupSyncMode syncmode); + +int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon, + const char *vmstatepath, + const char **list) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); -- 2.25.0.rc2.1.g09a9a1a997

Hi On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: Right, thanks

On 1/14/20 2:46 PM, marcandre.lureau(a)redhat.com wrote: 'make syntax-check' complains here. Michal