[libvirt PATCH v2 0/9] Second take on slirp-helper & dbus-vmstate
From: Marc-André Lureau <marcandre.lureau@redhat.com> Hi, The series "[libvirt] [PATCH v2 00/23] Use a slirp helper process" has been merged and partially reverted. Meanwhile, qemu dbus-vmstate design has been changed and merged upstream. This new series fixes the slirp-helper support. The significant change is that dbus-vmstate now requires a bus (instead of the earlier peer-to-peer connection). The current series doesn't attempt to enforce strict policies on the bus. As long as you can connect to the bus, you can send/receive from/to anyone. A follow-up series should implement the recommendations from https://qemu.readthedocs.io/en/latest/interop/dbus.html#security. The libslirp-rs slirp-helper hasn't yet received an official release. For testing, you may: $ cargo install --features=all --git https://gitlab.freedesktop.org/slirp/libslirp-rs The resulting binary should be ~/.cargo/bin/slirp-helper, so qemu.conf slirp_helper location should be adjusted. With that in place, a VM with user networking (slirp) should now start with the helper process. thanks v2: - merge most suggestions/changes from Michal Privoznik review of v1. - added "WIP: qemu_slirp: update to follow current spec" Marc-André Lureau (9): qemu: remove dbus-vmstate code qemu-conf: add configurable dbus-daemon location qemu-conf: add dbusStateDir qemu: add a DBus daemon helper unit domain: save/restore the state of dbus-daemon running qemu: prepare and stop the dbus daemon qemu: add dbus-vmstate helper migration support qemu-slirp: register helper for migration WIP: qemu-slirp: update to follow current spec m4/virt-driver-qemu.m4 | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 + src/qemu/qemu_alias.c | 17 +- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_command.c | 81 +++------ src/qemu/qemu_command.h | 6 +- src/qemu/qemu_conf.c | 7 + src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 264 +++++++++++++++++++++++++---- src/qemu/qemu_dbus.h | 25 ++- src/qemu/qemu_domain.c | 30 ++-- src/qemu/qemu_domain.h | 8 +- src/qemu/qemu_extdevice.c | 4 +- src/qemu/qemu_hotplug.c | 165 +++++++++--------- src/qemu/qemu_hotplug.h | 17 +- src/qemu/qemu_migration.c | 57 ++++++- 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 + src/qemu/qemu_process.c | 6 + src/qemu/qemu_slirp.c | 157 +++-------------- src/qemu/qemu_slirp.h | 4 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + 25 files changed, 544 insertions(+), 364 deletions(-) -- 2.25.0.rc2.1.g09a9a1a997
From: Marc-André Lureau <marcandre.lureau@redhat.com> 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@redhat.com> --- 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@/src/qemu/qemu_checkpoint.c @SRCDIR@/src/qemu/qemu_command.c @SRCDIR@/src/qemu/qemu_conf.c -@SRCDIR@/src/qemu/qemu_dbus.c @SRCDIR@/src/qemu/qemu_domain.c @SRCDIR@/src/qemu/qemu_domain_address.c @SRCDIR@/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@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- m4/virt-driver-qemu.m4 | 6 ++++++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 +++ src/qemu/qemu_conf.c | 5 +++++ src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 17 insertions(+) diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4 index a1d2c66bba..886261fce5 100644 --- a/m4/virt-driver-qemu.m4 +++ b/m4/virt-driver-qemu.m4 @@ -110,6 +110,12 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [ [/usr/bin:/usr/libexec]) AC_DEFINE_UNQUOTED([QEMU_SLIRP_HELPER], ["$QEMU_SLIRP_HELPER"], [QEMU slirp helper]) + + AC_PATH_PROG([QEMU_DBUS_DAEMON], [dbus-daemon], + [/usr/bin/dbus-daemon], + [/usr/bin:/usr/libexec]) + AC_DEFINE_UNQUOTED([QEMU_DBUS_DAEMON], ["$QEMU_DBUS_DAEMON"], + [QEMU dbus daemon]) ]) AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [ diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 557b6f38f8..58a2b6416f 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -89,6 +89,7 @@ module Libvirtd_qemu = | str_entry "bridge_helper" | str_entry "pr_helper" | str_entry "slirp_helper" + | str_entry "dbus_daemon" | bool_entry "set_process_name" | int_entry "max_processes" | int_entry "max_files" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index b6805ffc41..72ad4d4a10 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -828,6 +828,9 @@ # Path to the SLIRP networking helper. #slirp_helper = "/usr/bin/slirp-helper" +# Path to the dbus-daemon +#dbus_daemon = "/usr/bin/dbus-daemon" + # User for the swtpm TPM Emulator # # Default is 'tss'; this is the same user that tcsd (TrouSerS) installs diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 0357501dc6..c0be78261f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -261,6 +261,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged, cfg->bridgeHelperName = g_strdup(QEMU_BRIDGE_HELPER); cfg->prHelperName = g_strdup(QEMU_PR_HELPER); cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER); + cfg->dbusDaemonName = g_strdup(QEMU_DBUS_DAEMON); cfg->securityDefaultConfined = true; cfg->securityRequireConfined = false; @@ -347,6 +348,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->bridgeHelperName); VIR_FREE(cfg->prHelperName); VIR_FREE(cfg->slirpHelperName); + VIR_FREE(cfg->dbusDaemonName); VIR_FREE(cfg->saveImageFormat); VIR_FREE(cfg->dumpImageFormat); @@ -638,6 +640,9 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueString(conf, "slirp_helper", &cfg->slirpHelperName) < 0) return -1; + if (virConfGetValueString(conf, "dbus_daemon", &cfg->dbusDaemonName) < 0) + return -1; + if (virConfGetValueBool(conf, "set_process_name", &cfg->setProcessName) < 0) return -1; if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses) < 0) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index cedf232223..07c1944998 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -155,6 +155,7 @@ struct _virQEMUDriverConfig { char *bridgeHelperName; char *prHelperName; char *slirpHelperName; + char *dbusDaemonName; bool macFilter; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index dd90edf687..95540a4b5e 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -104,6 +104,7 @@ module Test_libvirtd_qemu = { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" } { "pr_helper" = "/usr/bin/qemu-pr-helper" } { "slirp_helper" = "/usr/bin/slirp-helper" } +{ "dbus_daemon" = "/usr/bin/dbus-daemon" } { "swtpm_user" = "tss" } { "swtpm_group" = "tss" } { "capability_filters" -- 2.25.0.rc2.1.g09a9a1a997
From: Marc-André Lureau <marcandre.lureau@redhat.com> Location of DBus daemon state configuration, socket, pid... Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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
From: Marc-André Lureau <marcandre.lureau@redhat.com> Add a unit to start & stop a private dbus-daemon. The daemon is meant to be started on demand, and associated with a QEMU process. It should be stopped when the QEMU process is stopped. The current policy is permissive like a session bus. Stricter policies can be added later, following recommendations from: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- po/POTFILES.in | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_dbus.c | 282 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_dbus.h | 33 +++++ src/qemu/qemu_domain.h | 2 + 5 files changed, 320 insertions(+) create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 2d54623dc7..fe361204bb 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -150,6 +150,7 @@ @SRCDIR@/src/qemu/qemu_checkpoint.c @SRCDIR@/src/qemu/qemu_command.c @SRCDIR@/src/qemu/qemu_conf.c +@SRCDIR@/src/qemu/qemu_dbus.c @SRCDIR@/src/qemu/qemu_domain.c @SRCDIR@/src/qemu/qemu_domain_address.c @SRCDIR@/src/qemu/qemu_driver.c diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 2b8517ecff..94a333f855 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -13,6 +13,8 @@ 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_dbus.c b/src/qemu/qemu_dbus.c new file mode 100644 index 0000000000..383efa0209 --- /dev/null +++ b/src/qemu/qemu_dbus.c @@ -0,0 +1,282 @@ +/* + * qemu_dbus.c: QEMU dbus daemon + * + * 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_security.h" + +#include "viralloc.h" +#include "virlog.h" +#include "virstring.h" +#include "virtime.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.dbus"); + + +int +qemuDBusPrepareHost(virQEMUDriverPtr driver) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + + return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group, + VIR_DIR_CREATE_ALLOW_EXIST); +} + + +static char * +qemuDBusCreatePidFilename(virQEMUDriverConfigPtr cfg, + const char *shortName) +{ + g_autofree char *name = g_strdup_printf("%s-dbus", shortName); + + return virPidFileBuildPath(cfg->dbusStateDir, name); +} + + +static char * +qemuDBusCreateFilename(const char *stateDir, + const char *shortName, + const char *ext) +{ + g_autofree char *name = g_strdup_printf("%s-dbus", shortName); + + return virFileBuildPath(stateDir, name, ext); +} + + +static char * +qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg, + const char *shortName) +{ + return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock"); +} + + +static char * +qemuDBusCreateConfPath(virQEMUDriverConfigPtr cfg, + const char *shortName) +{ + return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".conf"); +} + + +char * +qemuDBusGetAddress(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *shortName = virDomainDefGetShortName(vm->def); + g_autofree char *path = NULL; + + if (!shortName) + return NULL; + + path = qemuDBusCreateSocketPath(cfg, shortName); + + return g_strdup_printf("unix:path=%s", path); +} + + +static int +qemuDBusWriteConfig(const char *filename, const char *path) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + g_autofree char *config = NULL; + + virBufferAddLit(&buf, "<!DOCTYPE busconfig PUBLIC \"-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN\"\n"); + virBufferAddLit(&buf, " \"http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd\">\n"); + virBufferAddLit(&buf, "<busconfig>\n"); + virBufferAdjustIndent(&buf, 2); + + virBufferAddLit(&buf, "<type>org.libvirt.qemu</type>\n"); + virBufferAsprintf(&buf, "<listen>unix:path=%s</listen>\n", path); + virBufferAddLit(&buf, "<auth>EXTERNAL</auth>\n"); + + virBufferAddLit(&buf, "<policy context='default'>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAddLit(&buf, "<!-- Allow everything to be sent -->\n"); + virBufferAddLit(&buf, "<allow send_destination='*' eavesdrop='true'/>\n"); + virBufferAddLit(&buf, "<!-- Allow everything to be received -->\n"); + virBufferAddLit(&buf, "<allow eavesdrop='true'/>\n"); + virBufferAddLit(&buf, "<!-- Allow anyone to own anything -->\n"); + virBufferAddLit(&buf, "<allow own='*'/>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</policy>\n"); + + virBufferAddLit(&buf, "<include if_selinux_enabled='yes' selinux_root_relative='yes'>contexts/dbus_contexts</include>\n"); + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</busconfig>\n"); + + config = virBufferContentAndReset(&buf); + + return virFileWriteStr(filename, config, 0600); +} + + +void +qemuDBusStop(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + g_autofree char *shortName = NULL; + g_autofree char *pidfile = NULL; + g_autofree char *configfile = NULL; + + if (!(shortName = virDomainDefGetShortName(vm->def))) + return; + + pidfile = qemuDBusCreatePidFilename(cfg, shortName); + configfile = qemuDBusCreateConfPath(cfg, shortName); + + if (virPidFileForceCleanupPath(pidfile) < 0) { + VIR_WARN("Unable to kill dbus-daemon process"); + } else { + if (unlink(configfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove stale configfile %s"), + pidfile); + + } + priv->dbusDaemonRunning = false; + } +} + + +int +qemuDBusStart(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *shortName = NULL; + g_autofree char *pidfile = NULL; + g_autofree char *configfile = NULL; + g_autofree char *sockpath = NULL; + virTimeBackOffVar timebackoff; + const unsigned long long timeout = 500 * 1000; /* ms */ + VIR_AUTOCLOSE errfd = -1; + int cmdret = 0; + int exitstatus = 0; + pid_t cpid = -1; + int ret = -1; + + if (!virFileIsExecutable(cfg->dbusDaemonName)) { + virReportSystemError(errno, + _("'%s' is not a suitable dbus-daemon"), + cfg->dbusDaemonName); + return -1; + } + + if (!(shortName = virDomainDefGetShortName(vm->def))) + return -1; + + pidfile = qemuDBusCreatePidFilename(cfg, shortName); + configfile = qemuDBusCreateConfPath(cfg, shortName); + sockpath = qemuDBusCreateSocketPath(cfg, shortName); + + if (qemuDBusWriteConfig(configfile, sockpath) < 0) { + virReportSystemError(errno, _("Failed to write '%s'"), configfile); + return -1; + } + + if (qemuSecurityDomainSetPathLabel(driver, vm, configfile, false) < 0) + goto cleanup; + + cmd = virCommandNew(cfg->dbusDaemonName); + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandSetErrorFD(cmd, &errfd); + virCommandDaemonize(cmd); + virCommandAddArgFormat(cmd, "--config-file=%s", configfile); + + if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, + &exitstatus, &cmdret) < 0) + goto cleanup; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start dbus-daemon. exitstatus: %d"), exitstatus); + goto cleanup; + } + + if (virPidFileReadPath(pidfile, &cpid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("dbus-daemon %s didn't show up"), + cfg->dbusDaemonName); + goto cleanup; + } + + if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0) + goto cleanup; + while (virTimeBackOffWait(&timebackoff)) { + char errbuf[1024] = { 0 }; + + if (virFileExists(sockpath)) + break; + + if (virProcessKill(cpid, 0) == 0) + continue; + + if (saferead(errfd, errbuf, sizeof(errbuf) - 1) < 0) { + virReportSystemError(errno, + _("dbus-daemon %s died unexpectedly"), + cfg->dbusDaemonName); + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("dbus-daemon died and reported: %s"), errbuf); + } + + goto cleanup; + } + + if (!virFileExists(sockpath)) { + virReportError(VIR_ERR_OPERATION_TIMEOUT, + _("DBus daemon %s didn't show up"), + cfg->dbusDaemonName); + goto cleanup; + } + + if (priv->cgroup && + virCgroupAddProcess(priv->cgroup, cpid) < 0) + goto cleanup; + + if (qemuSecurityDomainSetPathLabel(driver, vm, sockpath, false) < 0) + goto cleanup; + + priv->dbusDaemonRunning = true; + ret = 0; + cleanup: + if (ret < 0) { + virCommandAbort(cmd); + if (cpid >= 0) + virProcessKillPainfully(cpid, true); + unlink(pidfile); + unlink(configfile); + unlink(sockpath); + } + return ret; +} diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h new file mode 100644 index 0000000000..d6cb1bc84a --- /dev/null +++ b/src/qemu/qemu_dbus.h @@ -0,0 +1,33 @@ +/* + * qemu_dbus.h: QEMU dbus daemon + * + * 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" + +int qemuDBusPrepareHost(virQEMUDriverPtr driver); + +char *qemuDBusGetAddress(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +int qemuDBusStart(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +void qemuDBusStop(virQEMUDriverPtr driver, + virDomainObjPtr vm); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c4fd7ac302..97e52b7a81 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -417,6 +417,8 @@ struct _qemuDomainObjPrivate { /* running backup job */ virDomainBackupDefPtr backup; + + bool dbusDaemonRunning; }; #define QEMU_DOMAIN_PRIVATE(vm) \ -- 2.25.0.rc2.1.g09a9a1a997
On 2/25/20 10:55 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a unit to start & stop a private dbus-daemon.
The daemon is meant to be started on demand, and associated with a QEMU process. It should be stopped when the QEMU process is stopped.
The current policy is permissive like a session bus. Stricter policies can be added later, following recommendations from: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- po/POTFILES.in | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_dbus.c | 282 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_dbus.h | 33 +++++ src/qemu/qemu_domain.h | 2 + 5 files changed, 320 insertions(+) create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c new file mode 100644 index 0000000000..383efa0209 --- /dev/null +++ b/src/qemu/qemu_dbus.c @@ -0,0 +1,282 @@
+void +qemuDBusStop(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + g_autofree char *shortName = NULL; + g_autofree char *pidfile = NULL; + g_autofree char *configfile = NULL; + + if (!(shortName = virDomainDefGetShortName(vm->def))) + return; + + pidfile = qemuDBusCreatePidFilename(cfg, shortName); + configfile = qemuDBusCreateConfPath(cfg, shortName); + + if (virPidFileForceCleanupPath(pidfile) < 0) { + VIR_WARN("Unable to kill dbus-daemon process"); + } else { + if (unlink(configfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove stale configfile %s"), + pidfile); + + }
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.
+ priv->dbusDaemonRunning = false; + } +} +
Michal
From: Marc-André Lureau <marcandre.lureau@redhat.com> This avoids trying to start a dbus-daemon when its already running. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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 4c6814a676..663938c9ff 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2882,6 +2882,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } + if (priv->dbusDaemonRunning) + virBufferAddLit(buf, "<dbusDaemon/>\n"); + if (priv->namespaces) { ssize_t ns = -1; @@ -3634,6 +3637,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
From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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
On Tue, Feb 25, 2020 at 10:55:10AM +0100, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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;
This launches dbus unconditionally for every VM regardless of whether its actually going to be used. There is certainly a nice conceptual simplicity in doing this, but I'm pretty concerned that there are going to be mgmt apps which do not like this extra overhead being added for every VM. I'm thinking KubeVirt and Kata containers in particular. This is also relevant for the libvirt embedded driver which is trying to eliminate all libvirt added overhead on managing QEMU, so that our fastest QEMU startup time can match that achieved by running QEMU directly. Unconditionally starting dbus will make that much more challenging. This is a long winded way of saying I think we need to do this only when it is actually required. This will certainly add complexity as we'll need to cope with dynamically launching DBus when we hotplug certain types of device which require it. I think we can ignore the hot-unplug case at least, as once you've taken the overhead for DBus for the VM, I don't think there's much to complain about, even if the device using it is unplugged. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Hi On Tue, Feb 25, 2020 at 12:49 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Tue, Feb 25, 2020 at 10:55:10AM +0100, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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;
This launches dbus unconditionally for every VM regardless of whether its actually going to be used.
No, it is qemuDBusStart()
There is certainly a nice conceptual simplicity in doing this, but I'm pretty concerned that there are going to be mgmt apps which do not like this extra overhead being added for every VM. I'm thinking KubeVirt and Kata containers in particular. This is also relevant for the libvirt embedded driver which is trying to eliminate all libvirt added overhead on managing QEMU, so that our fastest QEMU startup time can match that achieved by running QEMU directly. Unconditionally starting dbus will make that much more challenging.
This is a long winded way of saying I think we need to do this only when it is actually required. This will certainly add complexity as we'll need to cope with dynamically launching DBus when we hotplug certain types of device which require it. I think we can ignore the hot-unplug case at least, as once you've taken the overhead for DBus for the VM, I don't think there's much to complain about, even if the device using it is unplugged.
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@redhat.com> 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@redhat.com> --- src/qemu/qemu_alias.c | 7 +++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 54 ++++++++++++++++++++++++ src/qemu/qemu_command.h | 3 ++ src/qemu/qemu_dbus.c | 14 ++++++ src/qemu/qemu_dbus.h | 4 ++ src/qemu/qemu_domain.c | 12 ++++++ 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, 286 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 7eec7b7577..4696fd715c 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" @@ -9516,6 +9517,56 @@ qemuBuildPflashBlockdevCommandLine(virCommandPtr cmd, } +virJSONValuePtr +qemuBuildDBusVMStateInfoProps(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virJSONValuePtr ret = NULL; + const char *alias = qemuDomainGetDBusVMStateAlias(); + g_autofree char *addr = qemuDBusGetAddress(driver, vm); + + if (!addr) + return NULL; + + qemuMonitorCreateObjectProps(&ret, + "dbus-vmstate", alias, + "s:addr", addr, NULL); + return ret; +} + + +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: * @@ -9747,6 +9798,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 d4927d2191..bc3ba44fb3 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 383efa0209..1bce6ffc69 100644 --- a/src/qemu/qemu_dbus.c +++ b/src/qemu/qemu_dbus.c @@ -280,3 +280,17 @@ qemuDBusStart(virQEMUDriverPtr driver, } return ret; } + + +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 d6cb1bc84a..a96f19ac0d 100644 --- a/src/qemu/qemu_dbus.h +++ b/src/qemu/qemu_dbus.h @@ -31,3 +31,7 @@ int qemuDBusStart(virQEMUDriverPtr driver, void qemuDBusStop(virQEMUDriverPtr driver, virDomainObjPtr vm); + +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 663938c9ff..07ba6ecf0e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2239,6 +2239,13 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) /* reset node name allocator */ qemuDomainStorageIdReset(priv); + + priv->dbusDaemonRunning = false; + + virStringListFree(priv->dbusVMStateIds); + priv->dbusVMStateIds = NULL; + + priv->dbusVMState = false; } @@ -2885,6 +2892,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; @@ -3639,6 +3649,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 97e52b7a81..7804af4e04 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -419,6 +419,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 5db03c4e9f..1a40a1dbcc 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_DEBUG("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 29aaad58c7..773d7c9799 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1168,6 +1168,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, bool remote, unsigned int flags) { + qemuDomainObjPrivatePtr priv = vm->privateData; int nsnapshots; int pauseReason; size_t i; @@ -1272,6 +1273,13 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, return false; } } + + if (virStringListLength((const char **)priv->dbusVMStateIds) > 0 && + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot migrate this domain without dbus-vmstate support")); + return false; + } } return true; @@ -1937,8 +1945,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; @@ -3407,6 +3421,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, @@ -3559,6 +3604,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) { @@ -5244,6 +5292,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 008d4a0e75..d91af76503 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -25,6 +25,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" @@ -2382,6 +2383,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 8cf9e11899..cd9a6afe80 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -747,6 +747,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 50d93c0c7e..6fc716e3a1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2352,6 +2352,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 fd2e09025e..b4c013d46c 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
From: Marc-André Lureau <marcandre.lureau@redhat.com> 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@redhat.com> --- 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
On 2/25/20 10:55 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
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@redhat.com> --- 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; +
So at this point we've started the dbus daemon.
+ 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;
And if this fails, we leave it behind. All these 'return -1' (even those ones not visible in this hunk need to become 'goto error'. And we also need qemuDBusStop() call under the error label.
+ } + if (incoming) + virCommandAddArg(cmd, "--dbus-incoming"); + } + } + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT)) virCommandAddArg(cmd, "--exit-with-parent");
Michal
From: Marc-André Lureau <marcandre.lureau@redhat.com> 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@redhat.com> --- 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
On Tue, Feb 25, 2020 at 10:55 AM <marcandre.lureau@redhat.com> wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
The series "[libvirt] [PATCH v2 00/23] Use a slirp helper process" has been merged and partially reverted. Meanwhile, qemu dbus-vmstate design has been changed and merged upstream.
This new series fixes the slirp-helper support. The significant change is that dbus-vmstate now requires a bus (instead of the earlier peer-to-peer connection). The current series doesn't attempt to enforce strict policies on the bus. As long as you can connect to the bus, you can send/receive from/to anyone. A follow-up series should implement the recommendations from https://qemu.readthedocs.io/en/latest/interop/dbus.html#security.
The libslirp-rs slirp-helper hasn't yet received an official release. For testing, you may: $ cargo install --features=all --git https://gitlab.freedesktop.org/slirp/libslirp-rs
The resulting binary should be ~/.cargo/bin/slirp-helper, so qemu.conf slirp_helper location should be adjusted. With that in place, a VM with user networking (slirp) should now start with the helper process.
thanks
v2: - merge most suggestions/changes from Michal Privoznik review of v1. - added "WIP: qemu_slirp: update to follow current spec"
Marc-André Lureau (9): qemu: remove dbus-vmstate code qemu-conf: add configurable dbus-daemon location qemu-conf: add dbusStateDir qemu: add a DBus daemon helper unit domain: save/restore the state of dbus-daemon running qemu: prepare and stop the dbus daemon qemu: add dbus-vmstate helper migration support qemu-slirp: register helper for migration WIP: qemu-slirp: update to follow current spec
ping ? thanks
m4/virt-driver-qemu.m4 | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 + src/qemu/qemu_alias.c | 17 +- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_command.c | 81 +++------ src/qemu/qemu_command.h | 6 +- src/qemu/qemu_conf.c | 7 + src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 264 +++++++++++++++++++++++++---- src/qemu/qemu_dbus.h | 25 ++- src/qemu/qemu_domain.c | 30 ++-- src/qemu/qemu_domain.h | 8 +- src/qemu/qemu_extdevice.c | 4 +- src/qemu/qemu_hotplug.c | 165 +++++++++--------- src/qemu/qemu_hotplug.h | 17 +- src/qemu/qemu_migration.c | 57 ++++++- 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 + src/qemu/qemu_process.c | 6 + src/qemu/qemu_slirp.c | 157 +++-------------- src/qemu/qemu_slirp.h | 4 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + 25 files changed, 544 insertions(+), 364 deletions(-)
-- 2.25.0.rc2.1.g09a9a1a997
-- Marc-André Lureau
On 2/25/20 10:55 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
The series "[libvirt] [PATCH v2 00/23] Use a slirp helper process" has been merged and partially reverted. Meanwhile, qemu dbus-vmstate design has been changed and merged upstream.
This new series fixes the slirp-helper support. The significant change is that dbus-vmstate now requires a bus (instead of the earlier peer-to-peer connection). The current series doesn't attempt to enforce strict policies on the bus. As long as you can connect to the bus, you can send/receive from/to anyone. A follow-up series should implement the recommendations from https://qemu.readthedocs.io/en/latest/interop/dbus.html#security.
The libslirp-rs slirp-helper hasn't yet received an official release. For testing, you may: $ cargo install --features=all --git https://gitlab.freedesktop.org/slirp/libslirp-rs
The resulting binary should be ~/.cargo/bin/slirp-helper, so qemu.conf slirp_helper location should be adjusted. With that in place, a VM with user networking (slirp) should now start with the helper process.
thanks
v2: - merge most suggestions/changes from Michal Privoznik review of v1. - added "WIP: qemu_slirp: update to follow current spec"
Marc-André Lureau (9): qemu: remove dbus-vmstate code qemu-conf: add configurable dbus-daemon location qemu-conf: add dbusStateDir qemu: add a DBus daemon helper unit domain: save/restore the state of dbus-daemon running qemu: prepare and stop the dbus daemon qemu: add dbus-vmstate helper migration support qemu-slirp: register helper for migration WIP: qemu-slirp: update to follow current spec
m4/virt-driver-qemu.m4 | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 + src/qemu/qemu_alias.c | 17 +- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_command.c | 81 +++------ src/qemu/qemu_command.h | 6 +- src/qemu/qemu_conf.c | 7 + src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 264 +++++++++++++++++++++++++---- src/qemu/qemu_dbus.h | 25 ++- src/qemu/qemu_domain.c | 30 ++-- src/qemu/qemu_domain.h | 8 +- src/qemu/qemu_extdevice.c | 4 +- src/qemu/qemu_hotplug.c | 165 +++++++++--------- src/qemu/qemu_hotplug.h | 17 +- src/qemu/qemu_migration.c | 57 ++++++- 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 + src/qemu/qemu_process.c | 6 + src/qemu/qemu_slirp.c | 157 +++-------------- src/qemu/qemu_slirp.h | 4 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + 25 files changed, 544 insertions(+), 364 deletions(-)
I've fixed all the small bits I've raised locally. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> However, as I mention in 4/9 I need to merge some other patches first. I will merge these after that. Michal
participants (5)
-
Daniel P. Berrangé -
Marc-André Lureau -
Marc-André Lureau -
marcandre.lureau＠redhat.com
-
Michal Privoznik