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; --- po/POTFILES.in | 1 - 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 +- 16 files changed, 7 insertions(+), 486 deletions(-) delete mode 100644 src/qemu/qemu_dbus.c delete mode 100644 src/qemu/qemu_dbus.h diff --git a/po/POTFILES.in b/po/POTFILES.in index fe361204bb..2d54623dc7 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -150,7 +150,6 @@ @SRCDIR(a)/src/qemu/qemu_checkpoint.c @SRCDIR(a)/src/qemu/qemu_command.c @SRCDIR(a)/src/qemu/qemu_conf.c -@SRCDIR(a)/src/qemu/qemu_dbus.c @SRCDIR(a)/src/qemu/qemu_domain.c @SRCDIR(a)/src/qemu/qemu_domain_address.c @SRCDIR(a)/src/qemu/qemu_driver.c diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 94a333f855..2b8517ecff 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 6d5b53d30a..7eec7b7577 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" @@ -9517,85 +9516,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: * @@ -9827,9 +9747,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 d7fdba9942..d4927d2191 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 af6817cc05..4c6814a676 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" @@ -2139,13 +2138,6 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, } -static void -dbusVMStateHashFree(void *opaque) -{ - qemuDBusVMStateFree(opaque); -} - - static void * qemuDomainObjPrivateAlloc(void *opaque) { @@ -2166,9 +2158,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; @@ -2239,7 +2228,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->migrationCaps = NULL; virHashRemoveAll(priv->blockjobs); - virHashRemoveAll(priv->dbusVMStates); virObjectUnref(priv->pflash0); priv->pflash0 = NULL; @@ -2283,7 +2271,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 f8fb48f2ff..c4fd7ac302 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 9c0c0fd573..c1f7e7fc84 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -178,7 +178,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; } @@ -211,7 +211,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 9800491755..5db03c4e9f 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 @@ -1308,7 +1231,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; @@ -1515,7 +1438,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) @@ -4615,7 +4538,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 a307c5ebe2..29aaad58c7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1168,7 +1168,6 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, bool remote, unsigned int flags) { - qemuDomainObjPrivatePtr priv = vm->privateData; int nsnapshots; int pauseReason; size_t i; @@ -1263,13 +1262,6 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, return false; } - 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

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 | 2 ++ src/qemu/qemu_conf.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c0be78261f..2309086576 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -225,6 +225,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged, cfg->configDir = g_strdup_printf("%s/qemu", cfg->configBaseDir); cfg->autostartDir = g_strdup_printf("%s/qemu/autostart", cfg->configBaseDir); cfg->slirpStateDir = g_strdup_printf("%s/slirp", cfg->stateDir); + cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir); /* Set the default directory to find TLS X.509 certificates. * This will then be used as a fallback if the service specific @@ -308,6 +309,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 07c1944998..e67b4cc342 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

On 2/25/20 10:55 AM, marcandre.lureau(a)redhat.com wrote: This unlink is needless as it's done by virPidFileForceCleanupPath(). Unfortunatelly, I might have directed you in a not so good way in my review of v1. I thought that virCommandSetPidFile() will cause the daemon to open and hold the pidfile opened. Which is not the case. Therefore, virPidFileForceCleanupPath() will see unlocked pidfile and assumes that the daemon had died meanwhle and left the pidfile behind. I've posted patches for that: https://www.redhat.com/archives/libvir-list/2020-March/msg00499.html Sorry. But once I merge them, this will start to work as expected. Michal

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 8c1ed76677..3a6cb4b2b0 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" @@ -6480,6 +6481,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; @@ -7408,6 +7412,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

Hi On Tue, Feb 25, 2020 at 12:49 PM Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: No, it is qemuDBusStart() It is done on-demand, by qemuSlirpStart() at the moment, and actually only when the backend has dbus support.

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; When the helper supports DBus, connect it to the bus and set its ID. If the helper supports migration, register its ID to the list of dbus-vmstate ID to migrate, and specify --dbus-incoming when restoring the VM. Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_slirp.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index 8e001f0d10..e9b23f72a5 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -18,6 +18,7 @@ #include <config.h> +#include "qemu_dbus.h" #include "qemu_extdevice.h" #include "qemu_security.h" #include "qemu_slirp.h" @@ -202,6 +203,16 @@ qemuSlirpGetFD(qemuSlirpPtr slirp) } +static char * +qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net) +{ + char macstr[VIR_MAC_STRING_BUFLEN] = ""; + + /* can't use alias, because it's not stable across restarts */ + return g_strdup_printf("slirp-%s", virMacAddrFormat(&net->mac, macstr)); +} + + void qemuSlirpStop(qemuSlirpPtr slirp, virDomainObjPtr vm, @@ -209,11 +220,14 @@ qemuSlirpStop(qemuSlirpPtr slirp, virDomainNetDefPtr net) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *id = qemuSlirpGetDBusVMStateId(net); g_autofree char *pidfile = NULL; virErrorPtr orig_err; pid_t pid; int rc; + qemuDBusVMStateRemove(vm, id); + if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, net->info.alias))) { VIR_WARN("Unable to construct slirp pidfile path"); return; @@ -310,6 +324,28 @@ qemuSlirpStart(qemuSlirpPtr slirp, } } + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_ADDRESS)) { + g_autofree char *id = qemuSlirpGetDBusVMStateId(net); + g_autofree char *dbus_addr = qemuDBusGetAddress(driver, vm); + + if (qemuDBusStart(driver, vm) < 0) + return -1; + + virCommandAddArgFormat(cmd, "--dbus-id=%s", id); + + virCommandAddArgFormat(cmd, "--dbus-address=%s", dbus_addr); + + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + if (qemuDBusVMStateAdd(vm, id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to register slirp migration")); + return -1; + } + if (incoming) + virCommandAddArg(cmd, "--dbus-incoming"); + } + } + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT)) virCommandAddArg(cmd, "--exit-with-parent"); -- 2.25.0.rc2.1.g09a9a1a997

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; The WIP specification is hosted on slirp wiki at this point: https://gitlab.freedesktop.org/slirp/libslirp/-/wikis/Slirp-Helper We would need more feedback from various parties (including libvirt, podman, and other developpers) before declaring a frozen version. So for now, follow it, and feedback welcome! Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; --- src/qemu/qemu_slirp.c | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index e9b23f72a5..5e1fc1cf47 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -293,35 +293,21 @@ qemuSlirpStart(qemuSlirpPtr slirp, const virNetDevIPAddr *ip = net->guestIP.ips[i]; g_autofree char *addr = NULL; const char *opt = ""; + unsigned prefix = ip->prefix; if (!(addr = virSocketAddrFormat(&ip->address))) return -1; - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { opt = "--net"; - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) - opt = "--prefix-ipv6"; - - virCommandAddArgFormat(cmd, "%s=%s", opt, addr); - - if (ip->prefix) { - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { - virSocketAddr netmask; - g_autofree char *netmaskStr = NULL; - - if (virSocketAddrPrefixToNetmask(ip->prefix, &netmask, AF_INET) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to translate prefix %d to netmask"), - ip->prefix); - return -1; - } - if (!(netmaskStr = virSocketAddrFormat(&netmask))) - return -1; - virCommandAddArgFormat(cmd, "--mask=%s", netmaskStr); - } - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) - virCommandAddArgFormat(cmd, "--prefix-length-ipv6=%u", ip->prefix); + prefix = prefix ?: 24; + } + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) { + opt = "--net6"; + prefix = prefix ?: 64; } + + virCommandAddArgFormat(cmd, "%s=%s/%u", opt, addr, prefix); } if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_ADDRESS)) { -- 2.25.0.rc2.1.g09a9a1a997