[libvirt] [PATCH v2 00/23] Use a slirp helper process

From: Marc-André Lureau <marcandre.lureau@redhat.com> Hi, SLIRP networking can be running in a separate process. This allows for stricter security policies for QEMU & SLIRP, as SLIRP is notoriously not very safe (discussed on ML, various CVEs, and even the code says so explicitly in the comments), yet people rely on it for various reasons. With this series, for a network interface "user", libvirt will: - check the slirp-helper presence and capabilites (see [1]) - setup a socket pair between qemu and the helper - use -net socket - setup migration thanks to dbus-vmstate There are no changes required to domain configuration to benefit it. "guestfwd" isn't supported at this point, but it is known to be in a broken state with libvirt+qemu anyway. The dbus-vmstate is being proposed to QEMU. The libslirp-rs slirp-helper hasn't yet received a release. The current DBus p2p mode works ok, but is a hack. This is due to poor DBus support in Rust, and also relatively poor DBus p2p mode support in libdbus. fwiw, I have been working on an alternative rust-only implementation of a slirp-helper that will also follow [1], but I am now wondering if netstack or vpnkit could do the job. [1] https://gitlab.freedesktop.org/slirp/libslirp-rs/blob/master/src/bin/README.... Marc-André Lureau (23): Add .editorconfig tests: fix xml2xml tpm-emulator.xml test dbus: correctly build reply message qemu: replace logCtxt with qemuDomainLogAppendMessage() qemu: add socket datagram capability qemu: add dbus-vmstate capability qemu: reset VM id after external devices stop qemu-security: add qemuSecurityCommandRun() qemu: add dbus-vmstate domain-conf: add network def private data qemu: add qemuDomainNetworkPrivate qemu-conf: add configurable slirp-helper location qemu-conf: add slirp state dir qemu: add slirp helper unit qemu-domain: save and restore slirp state qemu: add a flag to the cookie to prevent slirp-helper setup qemu-migration: prevent migration if dbus-vmstate is required qemu-migration: prevent migration if slirp cannot be migrated qemu-extdevice: prepare, start and stop slirp-helper qemu-command: use -net socket,fd= with slirp-helper qemu-process: prepare slirp-helper qemu-hotplug: handle hotplugging of slirp-helper tests: add slirp-helper qemuxml2argv test .editorconfig | 21 + m4/virt-driver-qemu.m4 | 5 + src/conf/domain_conf.c | 21 +- src/conf/domain_conf.h | 6 + src/qemu/Makefile.inc.am | 4 + 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_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 118 ++++- src/qemu/qemu_command.h | 6 +- src/qemu/qemu_conf.c | 11 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 94 ++++ src/qemu/qemu_dbus.h | 42 ++ src/qemu/qemu_domain.c | 220 ++++++++- src/qemu/qemu_domain.h | 20 + src/qemu/qemu_driver.c | 8 + src/qemu/qemu_extdevice.c | 82 ++-- src/qemu/qemu_extdevice.h | 10 +- src/qemu/qemu_hotplug.c | 112 ++++- src/qemu/qemu_hotplug.h | 11 + src/qemu/qemu_interface.c | 27 ++ src/qemu/qemu_interface.h | 4 + src/qemu/qemu_migration.c | 19 + src/qemu/qemu_monitor.c | 13 +- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_process.c | 24 +- src/qemu/qemu_security.c | 22 + src/qemu/qemu_security.h | 6 + src/qemu/qemu_slirp.c | 448 ++++++++++++++++++ src/qemu/qemu_slirp.h | 81 ++++ src/qemu/qemu_tpm.c | 4 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/virdbus.c | 18 +- src/util/virdbus.h | 6 +- .../caps_4.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml | 1 + .../caps_4.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../net-user.x86_64-4.0.0.args | 34 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 2 +- tests/qemuxml2argvtest.c | 16 + tests/testutilsqemu.h | 1 + tests/virfirewalltest.c | 9 +- tests/virpolkittest.c | 3 +- 51 files changed, 1498 insertions(+), 77 deletions(-) create mode 100644 .editorconfig create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h create mode 100644 src/qemu/qemu_slirp.c create mode 100644 src/qemu/qemu_slirp.h create mode 100644 tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args -- 2.23.0.rc1

From: Marc-André Lureau <marcandre.lureau@redhat.com> Consistent code style across editors. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .editorconfig | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000000..e766441a50 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,21 @@ +# EditorConfig is a file format and collection of text editor plugins +# for maintaining consistent coding styles between different editors +# and IDEs. Most popular editors support this either natively or via +# plugin. +# +# Check https://editorconfig.org for details. + +root = true + +[*] +end_of_line = lf +insert_final_newline = true +charset = utf-8 + +[*.c] +indent_style = space +indent_size = 4 + +[*.{rng,xml}] +indent_style = space +indent_size = 2 -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Consistent code style across editors.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .editorconfig | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .editorconfig
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> It is failing, because it ends up being parsed with version='default' and expects '1.2' instead. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/qemuxml2argvdata/tpm-emulator.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuxml2argvdata/tpm-emulator.xml b/tests/qemuxml2argvdata/tpm-emulator.xml index 7f1e5756cb..defc3789ad 100644 --- a/tests/qemuxml2argvdata/tpm-emulator.xml +++ b/tests/qemuxml2argvdata/tpm-emulator.xml @@ -23,7 +23,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> - <backend type='emulator'/> + <backend type='emulator' version='1.2'/> </tpm> <memballoon model='virtio'/> </devices> -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
It is failing, because it ends up being parsed with version='default' and expects '1.2' instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/qemuxml2argvdata/tpm-emulator.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> dbus_message_new() does not construct correct replies by itself, it is recommended to use dbus_message_new_method_return() instead. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virdbus.c | 18 ++++++++++++------ src/util/virdbus.h | 6 ++++-- tests/virfirewalltest.c | 9 ++++++--- tests/virpolkittest.c | 3 ++- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index b0ac8d7055..64513eef14 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1456,6 +1456,7 @@ int virDBusCreateMethod(DBusMessage **call, /** * virDBusCreateReplyV: + * @msg: the message to reply to * @reply: pointer to be filled with a method reply message * @types: type signature for following method arguments * @args: method arguments @@ -1468,13 +1469,14 @@ int virDBusCreateMethod(DBusMessage **call, * as variadic args. See virDBusCreateMethodV for a * description of this parameter. */ -int virDBusCreateReplyV(DBusMessage **reply, +int virDBusCreateReplyV(DBusMessage *msg, + DBusMessage **reply, const char *types, va_list args) { int ret = -1; - if (!(*reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN))) { + if (!(*reply = dbus_message_new_method_return(msg))) { virReportOOMError(); goto cleanup; } @@ -1493,6 +1495,7 @@ int virDBusCreateReplyV(DBusMessage **reply, /** * virDBusCreateReply: + * @msg: the message to reply to * @reply: pointer to be filled with a method reply message * @types: type signature for following method arguments * @...: method arguments @@ -1500,14 +1503,15 @@ int virDBusCreateReplyV(DBusMessage **reply, * See virDBusCreateReplyV for a description of the * behaviour of this method. */ -int virDBusCreateReply(DBusMessage **reply, +int virDBusCreateReply(DBusMessage *msg, + DBusMessage **reply, const char *types, ...) { va_list args; int ret; va_start(args, types); - ret = virDBusCreateReplyV(reply, types, args); + ret = virDBusCreateReplyV(msg, reply, types, args); va_end(args); return ret; @@ -1811,7 +1815,8 @@ int virDBusCreateMethodV(DBusMessage **call ATTRIBUTE_UNUSED, return -1; } -int virDBusCreateReplyV(DBusMessage **reply ATTRIBUTE_UNUSED, +int virDBusCreateReplyV(DBusMessage *msg ATTRIBUTE_UNUSED, + DBusMessage **reply ATTRIBUTE_UNUSED, const char *types ATTRIBUTE_UNUSED, va_list args ATTRIBUTE_UNUSED) { @@ -1820,7 +1825,8 @@ int virDBusCreateReplyV(DBusMessage **reply ATTRIBUTE_UNUSED, return -1; } -int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED, +int virDBusCreateReply(DBusMessage *msg ATTRIBUTE_UNUSED, + DBusMessage **reply ATTRIBUTE_UNUSED, const char *types ATTRIBUTE_UNUSED, ...) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 083c074d59..0303e91045 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -52,9 +52,11 @@ int virDBusCreateMethodV(DBusMessage **call, const char *member, const char *types, va_list args); -int virDBusCreateReply(DBusMessage **reply, +int virDBusCreateReply(DBusMessage *msg, + DBusMessage **reply, const char *types, ...); -int virDBusCreateReplyV(DBusMessage **reply, +int virDBusCreateReplyV(DBusMessage *msg, + DBusMessage **reply, const char *types, va_list args); diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 7c586877d3..d2c85a27cc 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -150,7 +150,8 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, if (nargs == 1 && STREQ(type, "ipv4") && STREQ(args[0], "-L")) { - if (virDBusCreateReply(&reply, + if (virDBusCreateReply(message, + &reply, "s", TEST_FILTER_TABLE_LIST) < 0) goto error; } else if (nargs == 3 && @@ -158,11 +159,13 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, STREQ(args[0], "-t") && STREQ(args[1], "nat") && STREQ(args[2], "-L")) { - if (virDBusCreateReply(&reply, + if (virDBusCreateReply(message, + &reply, "s", TEST_NAT_TABLE_LIST) < 0) goto error; } else { - if (virDBusCreateReply(&reply, + if (virDBusCreateReply(message, + &reply, "s", "success") < 0) goto error; } diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index 94a6daae0c..598eca8803 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -123,7 +123,8 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, VIR_FREE(cancellationId); virStringListFreeCount(details, detailslen); - if (virDBusCreateReply(&reply, + if (virDBusCreateReply(message, + &reply, "(bba&{ss})", is_authorized, is_challenge, -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
dbus_message_new() does not construct correct replies by itself, it is recommended to use dbus_message_new_method_return() instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virdbus.c | 18 ++++++++++++------ src/util/virdbus.h | 6 ++++-- tests/virfirewalltest.c | 9 ++++++--- tests/virpolkittest.c | 3 ++- 4 files changed, 24 insertions(+), 12 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Once QEMU is started, the qemuDomainLogContext is owned by it, and can no longer be used from libvirt. Instead, use qemuDomainLogAppendMessage() which will redirect the log. This is not strictly necessary for swtpm, but the following patches are going to reuse qemuExtDeviceLogCommand(). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 35 ++++++++++------------------------- src/qemu/qemu_extdevice.h | 5 +++-- src/qemu/qemu_tpm.c | 4 ++-- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index af52466421..0aa050cb0a 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -36,39 +36,24 @@ VIR_LOG_INIT("qemu.qemu_extdevice"); int -qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, +qemuExtDeviceLogCommand(virQEMUDriverPtr driver, + virDomainObjPtr vm, virCommandPtr cmd, const char *info) { - int ret = -1; - char *timestamp = NULL; - char *logline = NULL; - int logFD; + VIR_AUTOFREE(char *) timestamp = virTimeStringNow(); + VIR_AUTOFREE(char *) cmds = virCommandToString(cmd, false); - logFD = qemuDomainLogContextGetWriteFD(logCtxt); - - if ((timestamp = virTimeStringNow()) == NULL) - goto cleanup; - - if (virAsprintf(&logline, "%s: Starting external device: %s\n", - timestamp, info) < 0) - goto cleanup; - - if (safewrite(logFD, logline, strlen(logline)) < 0) - goto cleanup; - - virCommandWriteArgLog(cmd, logFD); - - ret = 0; - - cleanup: - VIR_FREE(timestamp); - VIR_FREE(logline); + if (!timestamp || !cmds) + return -1; - return ret; + return qemuDomainLogAppendMessage(driver, vm, + _("%s: Starting external device: %s\n%s\n"), + timestamp, info, cmds); } + /* * qemuExtDevicesInitPaths: * diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 5a53c79f38..cdd20c28ef 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -23,10 +23,11 @@ #include "qemu_conf.h" #include "qemu_domain.h" -int qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, +int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, + virDomainObjPtr vm, virCommandPtr cmd, const char *info) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 98fe8a38b4..7537091e90 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -834,7 +834,7 @@ qemuExtTPMCleanupHost(virDomainDefPtr def) static int qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuDomainLogContextPtr logCtxt, + qemuDomainLogContextPtr logCtxt ATTRIBUTE_UNUSED, bool incomingMigration) { int ret = -1; @@ -863,7 +863,7 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, incomingMigration))) goto cleanup; - if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) + if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0) goto cleanup; virCommandSetErrorBuffer(cmd, &errbuf); -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Once QEMU is started, the qemuDomainLogContext is owned by it, and can no longer be used from libvirt. Instead, use qemuDomainLogAppendMessage() which will redirect the log.
This is not strictly necessary for swtpm, but the following patches are going to reuse qemuExtDeviceLogCommand().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 35 ++++++++++------------------------- src/qemu/qemu_extdevice.h | 5 +++-- src/qemu/qemu_tpm.c | 4 ++-- 3 files changed, 15 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index af52466421..0aa050cb0a 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -36,39 +36,24 @@ VIR_LOG_INIT("qemu.qemu_extdevice");
int -qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, +qemuExtDeviceLogCommand(virQEMUDriverPtr driver, + virDomainObjPtr vm, virCommandPtr cmd, const char *info) { - int ret = -1; - char *timestamp = NULL; - char *logline = NULL; - int logFD; + VIR_AUTOFREE(char *) timestamp = virTimeStringNow(); + VIR_AUTOFREE(char *) cmds = virCommandToString(cmd, false);
- logFD = qemuDomainLogContextGetWriteFD(logCtxt); - - if ((timestamp = virTimeStringNow()) == NULL) - goto cleanup; - - if (virAsprintf(&logline, "%s: Starting external device: %s\n", - timestamp, info) < 0) - goto cleanup; - - if (safewrite(logFD, logline, strlen(logline)) < 0) - goto cleanup; - - virCommandWriteArgLog(cmd, logFD); - - ret = 0; - - cleanup: - VIR_FREE(timestamp); - VIR_FREE(logline); + if (!timestamp || !cmds) + return -1;
- return ret; + return qemuDomainLogAppendMessage(driver, vm, + _("%s: Starting external device: %s\n%s\n"), + timestamp, info, cmds); }
+ /* * qemuExtDevicesInitPaths: * diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 5a53c79f38..cdd20c28ef 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -23,10 +23,11 @@ #include "qemu_conf.h" #include "qemu_domain.h"
-int qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, +int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, + virDomainObjPtr vm, virCommandPtr cmd, const char *info) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 98fe8a38b4..7537091e90 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -834,7 +834,7 @@ qemuExtTPMCleanupHost(virDomainDefPtr def) static int qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuDomainLogContextPtr logCtxt, + qemuDomainLogContextPtr logCtxt ATTRIBUTE_UNUSED,
I'd go a step further and remove logCtxt from the rest of the functions. But that can be done in a separate commit. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Datagram socket is available since qemu 4.0, commit fdec16e3c2a614e2861f3086b05d444b5d8c3406 ("net/socket: learn to talk with a unix dgram socket"). Required for slirp-helper communication. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + 9 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 405bc3f288..4cb135cd93 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -536,6 +536,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 335 */ "bochs-display", + "net-socket-dgram", ); @@ -4389,6 +4390,11 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) ARCH_IS_PPC64(qemuCaps->arch)) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); } + + /* -net socket,fd= with dgram socket (for ex, with slirp helper) */ + if (qemuCaps->version >= 3001092) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM); + } } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d7c6df20c7..c8b9970eb3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -517,6 +517,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 335 */ QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */ + QEMU_CAPS_NET_SOCKET_DGRAM, /* -net socket,fd= with dgram socket */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml index 8fe369f518..f9eccf0596 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml @@ -164,6 +164,7 @@ <flag name='bitmap-merge'/> <flag name='nbd-bitmap'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml index 2df230c4f7..8b96b85d4e 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml @@ -169,6 +169,7 @@ <flag name='bitmap-merge'/> <flag name='nbd-bitmap'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index f4acda457a..704cf612d6 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -167,6 +167,7 @@ <flag name='bitmap-merge'/> <flag name='nbd-bitmap'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index e71d83ee06..946484db8c 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -167,6 +167,7 @@ <flag name='bitmap-merge'/> <flag name='nbd-bitmap'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index a1ac2587a0..9c50b1afdf 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -131,6 +131,7 @@ <flag name='query-current-machine'/> <flag name='bitmap-merge'/> <flag name='nbd-bitmap'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 1d44a5a1ba..53b1ff6e11 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -206,6 +206,7 @@ <flag name='nbd-bitmap'/> <flag name='x86-max-cpu'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index f336aeb48c..6ad8194eb5 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -208,6 +208,7 @@ <flag name='cpu-unavailable-features'/> <flag name='canonical-cpu-features'/> <flag name='bochs-display'/> + <flag name='net-socket-dgram'/> <version>4000050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100759</microcodeVersion> -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Datagram socket is available since qemu 4.0, commit fdec16e3c2a614e2861f3086b05d444b5d8c3406 ("net/socket: learn to talk with a unix dgram socket").
Required for slirp-helper communication.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + 9 files changed, 14 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 405bc3f288..4cb135cd93 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -536,6 +536,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */ "bochs-display", + "net-socket-dgram", );
@@ -4389,6 +4390,11 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) ARCH_IS_PPC64(qemuCaps->arch)) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); } + + /* -net socket,fd= with dgram socket (for ex, with slirp helper) */ + if (qemuCaps->version >= 3001092) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM); + }
For some weird reasons we don't allow curly braces around a single line body. Also, I know this is available since 3.1.92 because that's how qemu advertises its versions towards the end of release cycle, but I'd rather see 4.0.0 encoded - that's what is going to match version from a package anyway. Or even better, isn't there a way for libvirt to detect this using some monitor sorcery? Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> This object is being proposed to qemu upstream "Add dbus-vmstate object". It handles data migration of external processes. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4cb135cd93..f698034cb5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -537,6 +537,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 335 */ "bochs-display", "net-socket-dgram", + "dbus-vmstate", ); @@ -1127,6 +1128,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-serial-pci-non-transitional", QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL }, { "max-x86_64-cpu", QEMU_CAPS_X86_MAX_CPU }, { "bochs-display", QEMU_CAPS_DEVICE_BOCHS_DISPLAY }, + { "dbus-vmstate", QEMU_CAPS_DBUS_VMSTATE }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c8b9970eb3..670978b8ba 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -518,6 +518,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 335 */ QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */ QEMU_CAPS_NET_SOCKET_DGRAM, /* -net socket,fd= with dgram socket */ + QEMU_CAPS_DBUS_VMSTATE, /* -object dbus-vmstate */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This object is being proposed to qemu upstream "Add dbus-vmstate object". It handles data migration of external processes.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> pid filenames (from swtpm and other helpers from this series) are based on VM shortname, which is derived from VM id. If the id is reset to early, the state filenames will not be found. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1ed56457b1..f8d740979d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7400,8 +7400,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false); - vm->def->id = -1; - if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); @@ -7477,6 +7475,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuExtDevicesStop(driver, vm); + vm->def->id = -1; + /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
pid filenames (from swtpm and other helpers from this series) are based on VM shortname, which is derived from VM id. If the id is reset to early, the state filenames will not be found.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Add a generic way to run a command through the security management. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_security.c | 22 ++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 3cd6d9bd3d..f8b53e06b3 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -632,3 +632,25 @@ qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver, virSecurityManagerTransactionAbort(driver->securityManager); return ret; } + + +int +qemuSecurityCommandRun(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret) +{ + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + vm->def, cmd) < 0) + return -1; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + return -1; + + *cmdret = virCommandRun(cmd, exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + return 0; +} diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 68e377f418..8cf4ab0721 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -101,6 +101,12 @@ int qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *savefile); +int qemuSecurityCommandRun(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret); + /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add * new APIs here. If an API can touch a file add a proper wrapper instead. */ -- 2.23.0.rc1

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a generic way to run a command through the security management.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_security.c | 22 ++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++++ 2 files changed, 28 insertions(+)
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 3cd6d9bd3d..f8b53e06b3 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -632,3 +632,25 @@ qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver, virSecurityManagerTransactionAbort(driver->securityManager); return ret; } + + +int +qemuSecurityCommandRun(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret) +{ + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + vm->def, cmd) < 0) + return -1; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + return -1; + + *cmdret = virCommandRun(cmd, exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + return 0; +} diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 68e377f418..8cf4ab0721 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -101,6 +101,12 @@ int qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *savefile);
+int qemuSecurityCommandRun(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd, + int *exitstatus, + int *cmdret); + /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add * new APIs here. If an API can touch a file add a proper wrapper instead. */
Since this is copied from qemuSecurityStartTPMEmulator() I'd expect some lines to be removed there. And also document what this function does and describe arguments. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Add dbusVMStates to keep a list of dbus-vmstate objects needed for migration. They are populated on the command line during start or qemuDBusVMStateAdd/Remove() will hotplug them as needed. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_alias.c | 17 ++++++++ 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 | 14 ++++++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_hotplug.c | 75 ++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 11 +++++ 11 files changed, 346 insertions(+) create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 254ba07dc0..94dd0e56ff 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_alias.c b/src/qemu/qemu_alias.c index 585cc972ba..fc5246bc7f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -843,3 +843,20 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias) return ret; } + +char * +qemuAliasDBusVMStateFromId(const char *id) +{ + char *ret; + int i; + + if (virAsprintf(&ret, "dbus-vms-%s", id) < 0) + return NULL; + + 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 aaac09a1d1..ae2fce16bc 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -95,3 +95,6 @@ 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 71a36ff63a..4357aa2fe1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -27,6 +27,7 @@ #include "qemu_interface.h" #include "qemu_alias.h" #include "qemu_security.h" +#include "qemu_dbus.h" #include "qemu_block.h" #include "cpu/cpu.h" #include "dirname.h" @@ -10386,6 +10387,85 @@ qemuBuildManagedPRCommandLine(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) +{ + VIR_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; + VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; + VIR_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: * @@ -10630,6 +10710,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) goto error; + if (qemuBuildDBusVMStateCommandLine(cmd, priv) < 0) + goto error; + if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) goto error; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 7e2dc5a60a..3a957c52fc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -62,6 +62,9 @@ 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 new file mode 100644 index 0000000000..76cd3bd346 --- /dev/null +++ b/src/qemu/qemu_dbus.c @@ -0,0 +1,94 @@ +/* + * 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) +{ + VIR_AUTOPTR(qemuDBusVMState) self; + + if (VIR_ALLOC(self) < 0) + return NULL; + + if (VIR_STRDUP(self->id, id) < 0) + return NULL; + + if (VIR_STRDUP(self->addr, addr) < 0) + return NULL; + + VIR_RETURN_PTR(self); +} + + +void +qemuDBusVMStateFree(qemuDBusVMStatePtr self) +{ + 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(QEMU_DOMAIN_PRIVATE(vm)->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(QEMU_DOMAIN_PRIVATE(vm)->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"); +} diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h new file mode 100644 index 0000000000..68ef6d1abf --- /dev/null +++ b/src/qemu/qemu_dbus.h @@ -0,0 +1,42 @@ +/* + * 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/>. + */ + +#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); + +VIR_DEFINE_AUTOPTR_FUNC(qemuDBusVMState, qemuDBusVMStateFree); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0555caa6ab..806dbfd1f8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -26,6 +26,7 @@ #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_migration.h" @@ -1961,6 +1962,14 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, } +static void +dbusVMStateHashFree(void *opaque, + const void *name ATTRIBUTE_UNUSED) +{ + qemuDBusVMStateFree(opaque); +} + + static void * qemuDomainObjPrivateAlloc(void *opaque) { @@ -1981,6 +1990,9 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData))) goto error; + if (!(priv->dbusVMStates = virHashCreate(5, dbusVMStateHashFree))) + goto error; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; priv->driver = opaque; @@ -2052,6 +2064,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) qemuDomainObjResetAsyncJob(priv); virHashRemoveAll(priv->blockjobs); + virHashRemoveAll(priv->dbusVMStates); } @@ -2084,6 +2097,7 @@ 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 b76d3cace9..851fb98f42 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -392,6 +392,8 @@ struct _qemuDomainObjPrivate { /* running block jobs */ virHashTablePtr blockjobs; + + virHashTablePtr dbusVMStates; }; #define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d8be63b71c..028921fb47 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -417,6 +417,81 @@ 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; + VIR_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; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorDelObject(priv->mon, qemuAliasDBusVMStateFromId(id)); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return ret; +} + + /** * qemuDomainChangeMediaBlockdev: * @driver: qemu driver structure diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 896e6c7b98..6d2cd34dbc 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -150,3 +150,14 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, virDomainDefPtr persistentDef, 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); -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add dbusVMStates to keep a list of dbus-vmstate objects needed for migration. They are populated on the command line during start or qemuDBusVMStateAdd/Remove() will hotplug them as needed.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_alias.c | 17 ++++++++ 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 | 14 ++++++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_hotplug.c | 75 ++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 11 +++++ 11 files changed, 346 insertions(+) create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 254ba07dc0..94dd0e56ff 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_alias.c b/src/qemu/qemu_alias.c index 585cc972ba..fc5246bc7f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -843,3 +843,20 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias)
return ret; } + +char * +qemuAliasDBusVMStateFromId(const char *id) +{ + char *ret; + int i;
This needs to be size_t rather than int.
+ + if (virAsprintf(&ret, "dbus-vms-%s", id) < 0) + return NULL; + + 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 aaac09a1d1..ae2fce16bc 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -95,3 +95,6 @@ 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 71a36ff63a..4357aa2fe1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -27,6 +27,7 @@ #include "qemu_interface.h" #include "qemu_alias.h" #include "qemu_security.h" +#include "qemu_dbus.h" #include "qemu_block.h" #include "cpu/cpu.h" #include "dirname.h" @@ -10386,6 +10387,85 @@ qemuBuildManagedPRCommandLine(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) +{ + VIR_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; + VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; + VIR_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: * @@ -10630,6 +10710,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) goto error;
+ if (qemuBuildDBusVMStateCommandLine(cmd, priv) < 0) + goto error; + if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 7e2dc5a60a..3a957c52fc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -62,6 +62,9 @@ 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 new file mode 100644 index 0000000000..76cd3bd346 --- /dev/null +++ b/src/qemu/qemu_dbus.c @@ -0,0 +1,94 @@ +/* + * 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) +{ + VIR_AUTOPTR(qemuDBusVMState) self;
This needs to be initialized to NULL to avoid freeing some random memory.
+ + if (VIR_ALLOC(self) < 0) + return NULL; + + if (VIR_STRDUP(self->id, id) < 0) + return NULL; + + if (VIR_STRDUP(self->addr, addr) < 0) + return NULL; + + VIR_RETURN_PTR(self); +} + + +void +qemuDBusVMStateFree(qemuDBusVMStatePtr self) +{
Well, we want our free functions to accept NULL. Not only because it simplifies the code in some cases it also ensures we don't SIGSEGV. For instance, if qemuDBusVMStateNew() fails rigt at the VIR_ALLOC(self) line, this free function is called with @self == NULL.
+ 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(QEMU_DOMAIN_PRIVATE(vm)->dbusVMStates, id, d) < 0) {
No need for this macro, it returns @priv in fact.
+ 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(QEMU_DOMAIN_PRIVATE(vm)->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");
This needs to be in _("Failed ...") so that the message can be translated. Also, we might want to log the domain name at least to help sysadmins/users understand which domain the error relates to.
+} diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h new file mode 100644 index 0000000000..68ef6d1abf --- /dev/null +++ b/src/qemu/qemu_dbus.h @@ -0,0 +1,42 @@ +/* + * qemu_dbus.c: QEMU DBus-related helpers
qemu_dbus.h
+ * + * 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); + +VIR_DEFINE_AUTOPTR_FUNC(qemuDBusVMState, qemuDBusVMStateFree); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0555caa6ab..806dbfd1f8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -26,6 +26,7 @@ #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_migration.h" @@ -1961,6 +1962,14 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, }
+static void +dbusVMStateHashFree(void *opaque, + const void *name ATTRIBUTE_UNUSED) +{ + qemuDBusVMStateFree(opaque); +} + + static void * qemuDomainObjPrivateAlloc(void *opaque) { @@ -1981,6 +1990,9 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData))) goto error;
+ if (!(priv->dbusVMStates = virHashCreate(5, dbusVMStateHashFree))) + goto error; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; priv->driver = opaque;
@@ -2052,6 +2064,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) qemuDomainObjResetAsyncJob(priv);
virHashRemoveAll(priv->blockjobs); + virHashRemoveAll(priv->dbusVMStates); }
@@ -2084,6 +2097,7 @@ 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 b76d3cace9..851fb98f42 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -392,6 +392,8 @@ struct _qemuDomainObjPrivate {
/* running block jobs */ virHashTablePtr blockjobs; + + virHashTablePtr dbusVMStates; };
#define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d8be63b71c..028921fb47 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -417,6 +417,81 @@ 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; + VIR_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; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorDelObject(priv->mon, qemuAliasDBusVMStateFromId(id));
The retval of qemuAliasDBusVMStateFromId() is leaked here.
+ + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return ret; +} +
With all those small problems fixed: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++++++++- src/conf/domain_conf.h | 6 ++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0456369d55..fb0904177f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2454,6 +2454,7 @@ virDomainNetDefFree(virDomainNetDefPtr def) if (!def) return; virDomainNetDefClear(def); + virObjectUnref(def->privateData); VIR_FREE(def); } @@ -11441,7 +11442,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) trustGuestRxFilters = NULL; VIR_AUTOFREE(char *) vhost_path = NULL; - if (VIR_ALLOC(def) < 0) + if (!(def = virDomainNetDefNew(xmlopt))) return NULL; ctxt->node = node; @@ -14337,6 +14338,24 @@ virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt) } +virDomainNetDefPtr +virDomainNetDefNew(virDomainXMLOptionPtr xmlopt) +{ + virDomainNetDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + if (xmlopt && xmlopt->privateData.networkNew && + !(def->privateData = xmlopt->privateData.networkNew())) { + VIR_FREE(def); + def = NULL; + } + + return def; +} + + /* Parse the XML definition for a graphics device */ static virDomainGraphicsDefPtr virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 57ca2a8ad1..9bd196b53c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1018,6 +1018,7 @@ struct _virDomainNetDef { unsigned int mtu; virNetDevCoalescePtr coalesce; virDomainVirtioOptionsPtr virtio; + virObjectPtr privateData; }; typedef enum { @@ -2711,6 +2712,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc chrSourceNew; virDomainXMLPrivateDataNewFunc vsockNew; virDomainXMLPrivateDataNewFunc graphicsNew; + virDomainXMLPrivateDataNewFunc networkNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; /* following function shall return a pointer which will be used as the @@ -2894,6 +2896,10 @@ virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt); virDomainGraphicsDefPtr virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt); + +virDomainNetDefPtr +virDomainNetDefNew(virDomainXMLOptionPtr xmlopt); + virDomainDefPtr virDomainDefNew(void); void virDomainObjAssignDef(virDomainObjPtr domain, -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++++++++- src/conf/domain_conf.h | 6 ++++++ 2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0456369d55..fb0904177f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2454,6 +2454,7 @@ virDomainNetDefFree(virDomainNetDefPtr def) if (!def) return; virDomainNetDefClear(def); + virObjectUnref(def->privateData); VIR_FREE(def); }
@@ -11441,7 +11442,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) trustGuestRxFilters = NULL; VIR_AUTOFREE(char *) vhost_path = NULL;
- if (VIR_ALLOC(def) < 0) + if (!(def = virDomainNetDefNew(xmlopt))) return NULL;
ctxt->node = node; @@ -14337,6 +14338,24 @@ virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt) }
+virDomainNetDefPtr +virDomainNetDefNew(virDomainXMLOptionPtr xmlopt) +{ + virDomainNetDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + if (xmlopt && xmlopt->privateData.networkNew && + !(def->privateData = xmlopt->privateData.networkNew())) { + VIR_FREE(def); + def = NULL;
This call to 'def = NULL' is not needed. VIR_FREE() does that for us. However, I actually prefer using virDomainNetDefFree() as that is more fool proof if somebody ever allocs something else in @def after VIR_ALLOC() and before these lines.
+ } + + return def; +} + + /* Parse the XML definition for a graphics device */ static virDomainGraphicsDefPtr virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 57ca2a8ad1..9bd196b53c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1018,6 +1018,7 @@ struct _virDomainNetDef { unsigned int mtu; virNetDevCoalescePtr coalesce; virDomainVirtioOptionsPtr virtio; + virObjectPtr privateData; };
typedef enum { @@ -2711,6 +2712,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc chrSourceNew; virDomainXMLPrivateDataNewFunc vsockNew; virDomainXMLPrivateDataNewFunc graphicsNew; + virDomainXMLPrivateDataNewFunc networkNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; /* following function shall return a pointer which will be used as the @@ -2894,6 +2896,10 @@ virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
virDomainGraphicsDefPtr virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt); + +virDomainNetDefPtr +virDomainNetDefNew(virDomainXMLOptionPtr xmlopt);
This function must be exposed in libvirt_private.syms too. I've identified other places where we VIR_ALLOC() a virDomainNetDef structure: bhyveParsePCINet(), xenParseVif(), lxcCreateNetDef() and vboxDumpNetwork() In some of them we already have xmlopt available, in others we might pass NULL safely: diff --git c/src/bhyve/bhyve_parse_command.c w/src/bhyve/bhyve_parse_command.c index 490381688c..7d460e9824 100644 --- c/src/bhyve/bhyve_parse_command.c +++ w/src/bhyve/bhyve_parse_command.c @@ -501,7 +501,7 @@ bhyveParsePCINet(virDomainDefPtr def, const char *separator = NULL; const char *mac = NULL; - if (VIR_ALLOC(net) < 0) + if (!(net = virDomainNetDefNew(xmlopt))) goto cleanup; /* As we only support interface type='bridge' and cannot diff --git c/src/libvirt_private.syms w/src/libvirt_private.syms index a34d92f5ef..f1fe7259f9 100644 --- c/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -478,6 +478,7 @@ virDomainNetDefActualToNetworkPort; virDomainNetDefClear; virDomainNetDefFormat; virDomainNetDefFree; +virDomainNetDefNew; virDomainNetDefToNetworkPort; virDomainNetFind; virDomainNetFindByName; diff --git c/src/libxl/xen_common.c w/src/libxl/xen_common.c index 7eb52c8c84..d327f03d73 100644 --- c/src/libxl/xen_common.c +++ w/src/libxl/xen_common.c @@ -1234,7 +1234,7 @@ xenParseVif(char *entry, const char *vif_typename) key = nextkey; } - if (VIR_ALLOC(net) < 0) + if (!(net = virDomainNetDefNew(NULL))) goto cleanup; if (mac[0]) { diff --git c/src/lxc/lxc_native.c w/src/lxc/lxc_native.c index b4c6e790d8..018eec6977 100644 --- c/src/lxc/lxc_native.c +++ w/src/lxc/lxc_native.c @@ -359,7 +359,7 @@ lxcCreateNetDef(const char *type, virDomainNetDefPtr net = NULL; virMacAddr macAddr; - if (VIR_ALLOC(net) < 0) + if (!(net = virDomainNetDefNew(NULL))) goto error; if (STREQ_NULLABLE(flag, "up")) diff --git c/src/vbox/vbox_common.c w/src/vbox/vbox_common.c index 49e657cdb7..ddabcb80ca 100644 --- c/src/vbox/vbox_common.c +++ w/src/vbox/vbox_common.c @@ -3692,7 +3692,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) char *utf8 = NULL; virDomainNetDefPtr net = NULL; - if (VIR_ALLOC(net) < 0) + if (!(net = virDomainNetDefNew(data->xmlopt))) return NULL; gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); With this squashed in: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 806dbfd1f8..7315fe103e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1269,6 +1269,44 @@ qemuDomainGraphicsPrivateDispose(void *obj) } +static virClassPtr qemuDomainNetworkPrivateClass; +static void qemuDomainNetworkPrivateDispose(void *obj); + + +static int +qemuDomainNetworkPrivateOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuDomainNetworkPrivate, virClassForObject())) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(qemuDomainNetworkPrivate); + + +static virObjectPtr +qemuDomainNetworkPrivateNew(void) +{ + qemuDomainNetworkPrivatePtr priv; + + if (qemuDomainNetworkPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainNetworkPrivateClass))) + return NULL; + + return (virObjectPtr) priv; +} + + +static void +qemuDomainNetworkPrivateDispose(void *obj ATTRIBUTE_UNUSED) +{ +} + + /* qemuDomainSecretPlainSetup: * @secinfo: Pointer to secret info * @usageType: The virSecretUsageType @@ -3428,6 +3466,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .chrSourceNew = qemuDomainChrSourcePrivateNew, .vsockNew = qemuDomainVsockPrivateNew, .graphicsNew = qemuDomainGraphicsPrivateNew, + .networkNew = qemuDomainNetworkPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 851fb98f42..560b01d80a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,6 +509,18 @@ struct _qemuDomainGraphicsPrivate { }; +#define QEMU_DOMAIN_NETWORK_PRIVATE(dev) \ + ((qemuDomainNetworkPrivatePtr) (dev)->privateData) + +typedef struct _qemuDomainNetworkPrivate qemuDomainNetworkPrivate; +typedef qemuDomainNetworkPrivate *qemuDomainNetworkPrivatePtr; +struct _qemuDomainNetworkPrivate { + virObject parent; + + bool tmp_to_be_larger_than_parent; +}; + + typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, QEMU_PROCESS_EVENT_GUESTPANIC, -- 2.23.0.rc1

On 8/8/19 4:55 PM, 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_domain.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 806dbfd1f8..7315fe103e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1269,6 +1269,44 @@ qemuDomainGraphicsPrivateDispose(void *obj) }
+static virClassPtr qemuDomainNetworkPrivateClass; +static void qemuDomainNetworkPrivateDispose(void *obj); + + +static int +qemuDomainNetworkPrivateOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuDomainNetworkPrivate, virClassForObject())) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(qemuDomainNetworkPrivate); + + +static virObjectPtr +qemuDomainNetworkPrivateNew(void) +{ + qemuDomainNetworkPrivatePtr priv; + + if (qemuDomainNetworkPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainNetworkPrivateClass))) + return NULL; + + return (virObjectPtr) priv; +} + + +static void +qemuDomainNetworkPrivateDispose(void *obj ATTRIBUTE_UNUSED) +{ +} + + /* qemuDomainSecretPlainSetup: * @secinfo: Pointer to secret info * @usageType: The virSecretUsageType @@ -3428,6 +3466,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .chrSourceNew = qemuDomainChrSourcePrivateNew, .vsockNew = qemuDomainVsockPrivateNew, .graphicsNew = qemuDomainGraphicsPrivateNew, + .networkNew = qemuDomainNetworkPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 851fb98f42..560b01d80a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,6 +509,18 @@ struct _qemuDomainGraphicsPrivate { };
+#define QEMU_DOMAIN_NETWORK_PRIVATE(dev) \ + ((qemuDomainNetworkPrivatePtr) (dev)->privateData)
We don't try to align backslashes anymore. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> A slirp helper is a process that provides user-mode networking through a unix domain socket. It is expected to follow the following specification: https://gitlab.freedesktop.org/slirp/libslirp-rs/blob/master/src/bin/README.... Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- m4/virt-driver-qemu.m4 | 5 +++++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 +++ src/qemu/qemu_conf.c | 7 ++++++- src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 17 insertions(+), 1 deletion(-) diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4 index a1d05bbd7f..a1d2c66bba 100644 --- a/m4/virt-driver-qemu.m4 +++ b/m4/virt-driver-qemu.m4 @@ -105,6 +105,11 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [ [/usr/bin:/usr/libexec]) AC_DEFINE_UNQUOTED([QEMU_PR_HELPER], ["$QEMU_PR_HELPER"], [QEMU PR helper]) + AC_PATH_PROG([QEMU_SLIRP_HELPER], [slirp-helper], + [/usr/bin/slirp-helper], + [/usr/bin:/usr/libexec]) + AC_DEFINE_UNQUOTED([QEMU_SLIRP_HELPER], ["$QEMU_SLIRP_HELPER"], + [QEMU slirp helper]) ]) AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [ diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 2a99a0c55f..7d7844dc09 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -89,6 +89,7 @@ module Libvirtd_qemu = | bool_entry "clear_emulator_capabilities" | str_entry "bridge_helper" | str_entry "pr_helper" + | str_entry "slirp_helper" | 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 8cabeccacb..b3a3428e4c 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -833,6 +833,9 @@ # used whenever <reservations/> are enabled for SCSI LUN devices. #pr_helper = "/usr/bin/qemu-pr-helper" +# Path to the SLIRP networking helper. +#slirp_helper = "/usr/bin/slirp-helper" + # 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 2953893337..4b84cb6dea 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -282,7 +282,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 || - VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0) + VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0 || + VIR_STRDUP(cfg->slirpHelperName, QEMU_SLIRP_HELPER) < 0) goto error; cfg->clearEmulatorCapabilities = true; @@ -373,6 +374,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->hugetlbfs); VIR_FREE(cfg->bridgeHelperName); VIR_FREE(cfg->prHelperName); + VIR_FREE(cfg->slirpHelperName); VIR_FREE(cfg->saveImageFormat); VIR_FREE(cfg->dumpImageFormat); @@ -669,6 +671,9 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0) return -1; + if (virConfGetValueString(conf, "slirp_helper", &cfg->slirpHelperName) < 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 0cbddd7a9c..a85ae50e14 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -153,6 +153,7 @@ struct _virQEMUDriverConfig { char *bridgeHelperName; char *prHelperName; + char *slirpHelperName; bool macFilter; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index b3b44d42d9..d686d248ae 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" } { "swtpm_user" = "tss" } { "swtpm_group" = "tss" } { "capability_filters" -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
A slirp helper is a process that provides user-mode networking through a unix domain socket. It is expected to follow the following specification: https://gitlab.freedesktop.org/slirp/libslirp-rs/blob/master/src/bin/README....
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- m4/virt-driver-qemu.m4 | 5 +++++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 3 +++ src/qemu/qemu_conf.c | 7 ++++++- src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 17 insertions(+), 1 deletion(-)
Yo Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4b84cb6dea..7d2e84b5bb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -216,6 +216,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0) goto error; + if (virAsprintf(&cfg->slirpStateDir, "%s/slirp", cfg->stateDir) < 0) + goto error; + if (!(cfg->configBaseDir = virGetUserConfigDirectory())) goto error; @@ -335,6 +338,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->swtpmLogDir); VIR_FREE(cfg->stateDir); VIR_FREE(cfg->swtpmStateDir); + VIR_FREE(cfg->slirpStateDir); VIR_FREE(cfg->libDir); VIR_FREE(cfg->cacheDir); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a85ae50e14..8473d6d4ca 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -96,6 +96,7 @@ struct _virQEMUDriverConfig { char *swtpmLogDir; char *stateDir; char *swtpmStateDir; + char *slirpStateDir; /* These two directories are ones QEMU processes use (so must match * the QEMU user/group */ char *libDir; -- 2.23.0.rc1

On 8/8/19 4:55 PM, 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_conf.c | 4 ++++ src/qemu/qemu_conf.h | 1 + 2 files changed, 5 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4b84cb6dea..7d2e84b5bb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -216,6 +216,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0) goto error;
+ if (virAsprintf(&cfg->slirpStateDir, "%s/slirp", cfg->stateDir) < 0) + goto error; + if (!(cfg->configBaseDir = virGetUserConfigDirectory())) goto error;
Missing initialization for @privileged == true case. Although, if you do this outside of this if() statement, then you don't need to worry.
@@ -335,6 +338,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->swtpmLogDir); VIR_FREE(cfg->stateDir); VIR_FREE(cfg->swtpmStateDir); + VIR_FREE(cfg->slirpStateDir);
VIR_FREE(cfg->libDir); VIR_FREE(cfg->cacheDir); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a85ae50e14..8473d6d4ca 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -96,6 +96,7 @@ struct _virQEMUDriverConfig { char *swtpmLogDir; char *stateDir; char *swtpmStateDir; + char *slirpStateDir; /* These two directories are ones QEMU processes use (so must match * the QEMU user/group */ char *libDir;
Also, what is missing is the dir creation and chown() that should be done in qemuStateInitialize(). Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> The unit provides the functions associated with a slirp-helper: - probing / checking capabilities - opening the socketpair - starting / stoping the helper - registering for dbus-vmstate migration Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.h | 4 + src/qemu/qemu_slirp.c | 448 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_slirp.h | 81 +++++++ 4 files changed, 535 insertions(+) create mode 100644 src/qemu/qemu_slirp.c create mode 100644 src/qemu/qemu_slirp.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 94dd0e56ff..505b418fa5 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -58,6 +58,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_security.h \ qemu/qemu_qapi.c \ qemu/qemu_qapi.h \ + qemu/qemu_slirp.c \ + qemu/qemu_slirp.h \ qemu/qemu_tpm.c \ qemu/qemu_tpm.h \ $(NULL) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 560b01d80a..7293c87d7c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,6 +509,10 @@ struct _qemuDomainGraphicsPrivate { }; +typedef struct _qemuSlirp qemuSlirp; +typedef struct _qemuSlirp *qemuSlirpPtr; + + #define QEMU_DOMAIN_NETWORK_PRIVATE(dev) \ ((qemuDomainNetworkPrivatePtr) (dev)->privateData) diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c new file mode 100644 index 0000000000..36370b6be0 --- /dev/null +++ b/src/qemu/qemu_slirp.c @@ -0,0 +1,448 @@ +/* + * qemu_slirp.c: QEMU Slirp support + * + * 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_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_slirp.h" +#include "viralloc.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" +#include "virstring.h" +#include "virtime.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.slirp"); + +VIR_ENUM_IMPL(qemuSlirpFeature, + QEMU_SLIRP_FEATURE_LAST, + "", + "ipv4", + "ipv6", + "tftp", + "dbus-address", + "dbus-p2p", + "migrate", + "restrict", + "exit-with-parent", +); + + +void +qemuSlirpFree(qemuSlirpPtr slirp) +{ + VIR_FORCE_CLOSE(slirp->fd[0]); + VIR_FORCE_CLOSE(slirp->fd[1]); + virBitmapFree(slirp->features); + VIR_FREE(slirp); +} + + +void +qemuSlirpSetFeature(qemuSlirpPtr slirp, + qemuSlirpFeature feature) +{ + ignore_value(virBitmapSetBit(slirp->features, feature)); +} + + +bool +qemuSlirpHasFeature(const qemuSlirpPtr slirp, + qemuSlirpFeature feature) +{ + return virBitmapIsBitSet(slirp->features, feature); +} + + +qemuSlirpPtr +qemuSlirpNew(void) +{ + qemuSlirpPtr slirp = NULL; + + if (VIR_ALLOC(slirp) < 0) + return NULL; + + slirp->pid = (pid_t)-1; + slirp->fd[0] = slirp->fd[1] = -1; + slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST); + + return slirp; +} + + +qemuSlirpPtr +qemuSlirpNewForHelper(const char *helper) +{ + VIR_AUTOPTR(qemuSlirp) slirp = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) output = NULL; + VIR_AUTOPTR(virJSONValue) doc = NULL; + virJSONValuePtr featuresJSON; + size_t i, nfeatures; + + if (!helper) + return NULL; + + slirp = qemuSlirpNew(); + if (!slirp) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate slirp for '%s'"), helper); + return NULL; + } + + cmd = virCommandNewArgList(helper, "--print-capabilities", NULL); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + return NULL; + + if (!(doc = virJSONValueFromString(output)) || + !(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse json capabilities '%s'"), + helper); + return NULL; + } + + nfeatures = virJSONValueArraySize(featuresJSON); + for (i = 0; i < nfeatures; i++) { + virJSONValuePtr item = virJSONValueArrayGet(featuresJSON, i); + const char *tmpStr = virJSONValueGetString(item); + int tmp; + + if ((tmp = qemuSlirpFeatureTypeFromString(tmpStr)) <= 0) { + VIR_WARN("unknown slirp feature %s", tmpStr); + continue; + } + + qemuSlirpSetFeature(slirp, tmp); + } + + VIR_RETURN_PTR(slirp); +} + + +static char * +qemuSlirpCreatePidFilename(const char *stateDir, + const char *shortName, + const char *alias) +{ + VIR_AUTOFREE(char *) name = NULL; + + if (virAsprintf(&name, "%s-%s-slirp", shortName, alias) < 0) + return NULL; + + return virPidFileBuildPath(stateDir, name); +} + + +static int +qemuSlirpGetPid(const char *binPath, + const char *stateDir, + const char *shortName, + const char *alias, + pid_t *pid) +{ + VIR_AUTOFREE(char *) pidfile = qemuSlirpCreatePidFilename(stateDir, shortName, alias); + if (!pidfile) + return -ENOMEM; + + return virPidFileReadPathIfAlive(pidfile, pid, binPath); +} + + +int +qemuSlirpOpen(qemuSlirpPtr slirp, + virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + int rc, pair[2] = { -1, -1 }; + + if (qemuSecuritySetSocketLabel(driver->securityManager, def) < 0) + goto error; + + rc = socketpair(AF_UNIX, SOCK_DGRAM, 0, pair); + + if (qemuSecurityClearSocketLabel(driver->securityManager, def) < 0) + goto error; + + if (rc < 0) { + virReportSystemError(errno, "%s", _("failed to create socketpair")); + goto error; + } + + slirp->fd[0] = pair[0]; + slirp->fd[1] = pair[1]; + + return 0; + +error: + VIR_FORCE_CLOSE(pair[0]); + VIR_FORCE_CLOSE(pair[1]); + return -1; +} + + +int +qemuSlirpGetFD(qemuSlirpPtr slirp) +{ + int fd = slirp->fd[0]; + slirp->fd[0] = -1; + return fd; +} + + +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 */ + if (virAsprintf(&id, "slirp-%s", virMacAddrFormat(&net->mac, macstr)) < 0) + return NULL; + + return id; +} + + +static char * +qemuSlirpGetDBusAddress(const char *stateDir, + const char *shortName, + const char *alias) +{ + char *name = NULL; + + if (virAsprintf(&name, "unix:path=%s/%s-%s-slirp", stateDir, shortName, alias) < 0) + return NULL; + + return name; +} + + +void +qemuSlirpStop(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def); + VIR_AUTOFREE(char *) id = qemuSlirpGetDBusVMStateId(net); + virErrorPtr orig_err; + int rc; + pid_t pid; + + if (!(pidfile = qemuSlirpCreatePidFilename( + cfg->stateDir, shortName, net->info.alias))) { + VIR_WARN("Unable to construct slirp pidfile path"); + return; + } + + qemuDBusVMStateRemove(driver, vm, id, hot); + + rc = qemuSlirpGetPid(cfg->slirpHelperName, + cfg->stateDir, shortName, net->info.alias, &pid); + if (rc == 0 && pid != (pid_t)-1) { + char ebuf[1024]; + + VIR_DEBUG("Killing slirp process %lld", (long long)pid); + if (virProcessKill(pid, SIGKILL) < 0 && errno != ESRCH) + VIR_ERROR(_("Failed to kill process %lld: %s"), + (long long)pid, + virStrerror(errno, ebuf, sizeof(ebuf))); + } + + virErrorPreserveLast(&orig_err); + if (virPidFileForceCleanupPath(pidfile) < 0) { + VIR_WARN("Unable to kill slirp process"); + } else { + if (unlink(pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove stale pidfile %s"), + pidfile); + } + } + virErrorRestore(&orig_err); + slirp->pid = 0; +} + + +int +qemuSlirpStart(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hotplug, + qemuProcessIncomingDefPtr incoming) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) cmdstr = NULL; + VIR_AUTOFREE(char *) addr = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) dbus_addr = NULL; + VIR_AUTOFREE(char *) id = NULL; + VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def); + size_t i; + virTimeBackOffVar timebackoff; + const unsigned long long timeout = 500 * 1000; /* ms */ + const char *dbus_path = NULL; + pid_t pid; + int cmdret = 0, exitstatus = 0; + + if (incoming && + !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The slirp-helper doesn't support migration")); + } + + if (!(pidfile = qemuSlirpCreatePidFilename( + cfg->stateDir, shortName, net->info.alias))) + return -1; + + if (!(cmd = virCommandNew(cfg->slirpHelperName))) + return -1; + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + virCommandAddArgFormat(cmd, "--fd=%d", slirp->fd[1]); + virCommandPassFD(cmd, slirp->fd[1], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + slirp->fd[1] = -1; + + for (i = 0; i < net->guestIP.nips; i++) { + const virNetDevIPAddr *ip = net->guestIP.ips[i]; + const char *opt = ""; + + if (!(addr = virSocketAddrFormat(&ip->address))) + return -1; + + 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); + VIR_FREE(addr); + + if (ip->prefix) { + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { + virSocketAddr netmask; + VIR_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); + } + } + + if (qemuSlirpHasFeature(slirp, + QEMU_SLIRP_FEATURE_DBUS_P2P)) { + id = qemuSlirpGetDBusVMStateId(net); + dbus_addr = qemuSlirpGetDBusAddress(cfg->stateDir, + shortName, net->info.alias); + if (!id || !dbus_addr) { + return -1; + } + + dbus_path = dbus_addr + strlen("unix:path="); + + if (unlink(dbus_path) < 0 && errno != ENOENT) { + virReportSystemError(errno, _("Unable to unlink %s"), dbus_path); + return -1; + } + + 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"); + } + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0) + return -1; + + if (qemuSecurityCommandRun(driver, vm, cmd, + &exitstatus, &cmdret) < 0) + return -1; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'slirp'. exitstatus: %d"), exitstatus); + return -1; + } + + /* check that the helper has written its pid into the file */ + if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0) + return -1; + while (virTimeBackOffWait(&timebackoff)) { + int rc = qemuSlirpGetPid(cfg->slirpHelperName, + cfg->stateDir, shortName, net->info.alias, &pid); + if (rc < 0) + continue; + if (rc == 0 && pid == (pid_t)-1) + return -1; + break; + } + + if (dbus_path) { + while (virTimeBackOffWait(&timebackoff)) { + if (!virFileExists(dbus_path)) + continue; + break; + } + } + + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + if (qemuDBusVMStateAdd(driver, vm, id, dbus_addr, hotplug) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register slirp migration")); + return -1; + } + } + + slirp->pid = pid; + return 0; +} diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h new file mode 100644 index 0000000000..f44edce543 --- /dev/null +++ b/src/qemu/qemu_slirp.h @@ -0,0 +1,81 @@ +/* + * qemu_slirp.h: QEMU Slirp support + * + * 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" +#include "qemu_process.h" +#include "vircommand.h" + +typedef enum { + QEMU_SLIRP_FEATURE_NONE = 0, + QEMU_SLIRP_FEATURE_IPV4, + QEMU_SLIRP_FEATURE_IPV6, + QEMU_SLIRP_FEATURE_TFTP, + QEMU_SLIRP_FEATURE_DBUS_ADDRESS, + QEMU_SLIRP_FEATURE_DBUS_P2P, + QEMU_SLIRP_FEATURE_MIGRATE, + QEMU_SLIRP_FEATURE_RESTRICT, + QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT, + + QEMU_SLIRP_FEATURE_LAST, +} qemuSlirpFeature; + +VIR_ENUM_DECL(qemuSlirpFeature); + + +struct _qemuSlirp { + int fd[2]; + virBitmapPtr features; + pid_t pid; +}; + + +qemuSlirpPtr qemuSlirpNew(void); + +qemuSlirpPtr qemuSlirpNewForHelper(const char *helper); + +void qemuSlirpFree(qemuSlirpPtr slirp); + +void qemuSlirpSetFeature(qemuSlirpPtr slirp, + qemuSlirpFeature feature); + +bool qemuSlirpHasFeature(const qemuSlirpPtr slirp, + qemuSlirpFeature feature); + +int qemuSlirpOpen(qemuSlirpPtr slirp, + virQEMUDriverPtr driver, + virDomainDefPtr def); + +int qemuSlirpStart(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot, + qemuProcessIncomingDefPtr incoming); + +void qemuSlirpStop(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot); + +int qemuSlirpGetFD(qemuSlirpPtr slirp); + +VIR_DEFINE_AUTOPTR_FUNC(qemuSlirp, qemuSlirpFree); -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The unit provides the functions associated with a slirp-helper: - probing / checking capabilities - opening the socketpair - starting / stoping the helper - registering for dbus-vmstate migration
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_domain.h | 4 + src/qemu/qemu_slirp.c | 448 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_slirp.h | 81 +++++++ 4 files changed, 535 insertions(+) create mode 100644 src/qemu/qemu_slirp.c create mode 100644 src/qemu/qemu_slirp.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 94dd0e56ff..505b418fa5 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -58,6 +58,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_security.h \ qemu/qemu_qapi.c \ qemu/qemu_qapi.h \ + qemu/qemu_slirp.c \ + qemu/qemu_slirp.h \ qemu/qemu_tpm.c \ qemu/qemu_tpm.h \ $(NULL) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 560b01d80a..7293c87d7c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -509,6 +509,10 @@ struct _qemuDomainGraphicsPrivate { };
+typedef struct _qemuSlirp qemuSlirp; +typedef struct _qemuSlirp *qemuSlirpPtr; + +
I understand why you need this here, but it feels wrong. This must go into qemu_slirp.h and then from qemu_domain.h you include qemu_slirp.h. But that creates a circular dependency via qemu_process.h. But since you don'y really need qemuProcessIncomingDefPtr in qemu_slirp.h you only need to know if there's an incoming migration the qemuSlirpStart() argument can be turned into a boolean type and thus you qemu_process.h include can be dropped and the circular include is gone.
#define QEMU_DOMAIN_NETWORK_PRIVATE(dev) \ ((qemuDomainNetworkPrivatePtr) (dev)->privateData)
diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c new file mode 100644 index 0000000000..36370b6be0 --- /dev/null +++ b/src/qemu/qemu_slirp.c @@ -0,0 +1,448 @@ +/* + * qemu_slirp.c: QEMU Slirp support + * + * 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_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_slirp.h" +#include "viralloc.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" +#include "virstring.h" +#include "virtime.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.slirp"); + +VIR_ENUM_IMPL(qemuSlirpFeature, + QEMU_SLIRP_FEATURE_LAST, + "", + "ipv4", + "ipv6", + "tftp", + "dbus-address", + "dbus-p2p", + "migrate", + "restrict", + "exit-with-parent", +); + + +void +qemuSlirpFree(qemuSlirpPtr slirp) +{
A free function must accept NULL.
+ VIR_FORCE_CLOSE(slirp->fd[0]); + VIR_FORCE_CLOSE(slirp->fd[1]); + virBitmapFree(slirp->features); + VIR_FREE(slirp); +} + + +void +qemuSlirpSetFeature(qemuSlirpPtr slirp, + qemuSlirpFeature feature) +{ + ignore_value(virBitmapSetBit(slirp->features, feature)); +} + + +bool +qemuSlirpHasFeature(const qemuSlirpPtr slirp, + qemuSlirpFeature feature) +{ + return virBitmapIsBitSet(slirp->features, feature); +} + + +qemuSlirpPtr +qemuSlirpNew(void) +{ + qemuSlirpPtr slirp = NULL; + + if (VIR_ALLOC(slirp) < 0) + return NULL; + + slirp->pid = (pid_t)-1; + slirp->fd[0] = slirp->fd[1] = -1; + slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST);
Need to check if this hasn't failed.
+ + return slirp; +} + + +qemuSlirpPtr +qemuSlirpNewForHelper(const char *helper) +{ + VIR_AUTOPTR(qemuSlirp) slirp = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) output = NULL; + VIR_AUTOPTR(virJSONValue) doc = NULL; + virJSONValuePtr featuresJSON; + size_t i, nfeatures; + + if (!helper) + return NULL; + + slirp = qemuSlirpNew(); + if (!slirp) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate slirp for '%s'"), helper); + return NULL; + } + + cmd = virCommandNewArgList(helper, "--print-capabilities", NULL); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + return NULL; +
So this will run slirp-helper as root:root to query caps. I guess it's okay, as long as we don't run the actual slirp-helper that we start together with qemu as root:root.
+ if (!(doc = virJSONValueFromString(output)) || + !(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse json capabilities '%s'"), + helper); + return NULL; + } + + nfeatures = virJSONValueArraySize(featuresJSON); + for (i = 0; i < nfeatures; i++) { + virJSONValuePtr item = virJSONValueArrayGet(featuresJSON, i); + const char *tmpStr = virJSONValueGetString(item); + int tmp; + + if ((tmp = qemuSlirpFeatureTypeFromString(tmpStr)) <= 0) { + VIR_WARN("unknown slirp feature %s", tmpStr); + continue; + } + + qemuSlirpSetFeature(slirp, tmp); + } + + VIR_RETURN_PTR(slirp); +} + + +static char * +qemuSlirpCreatePidFilename(const char *stateDir, + const char *shortName, + const char *alias) +{ + VIR_AUTOFREE(char *) name = NULL; + + if (virAsprintf(&name, "%s-%s-slirp", shortName, alias) < 0) + return NULL; + + return virPidFileBuildPath(stateDir, name);
Since you're passing @stateDir as an argument, it's easy to miss that you're passing cfg->stateDir instead of cfg->slirpStateDir later in the code. So I suggest you drop @stateDir in favor of @cfg and use cfg->slirpStateDir directly. Also, @shortName can be replaced with vm->def so that we don't have to worry about getting the short name wrong.
+} + + +static int +qemuSlirpGetPid(const char *binPath, + const char *stateDir, + const char *shortName, + const char *alias, + pid_t *pid) +{ + VIR_AUTOFREE(char *) pidfile = qemuSlirpCreatePidFilename(stateDir, shortName, alias); + if (!pidfile) + return -ENOMEM; + + return virPidFileReadPathIfAlive(pidfile, pid, binPath); +}
This function is prettty useless. In the only place wher it's used we already have @pidfile constructed so why not use virPidFileReadPathIfAlive() directly? Which turns out is also not correct, but I'll get to that.
+ + +int +qemuSlirpOpen(qemuSlirpPtr slirp, + virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + int rc, pair[2] = { -1, -1 }; + + if (qemuSecuritySetSocketLabel(driver->securityManager, def) < 0) + goto error; + + rc = socketpair(AF_UNIX, SOCK_DGRAM, 0, pair); + + if (qemuSecurityClearSocketLabel(driver->securityManager, def) < 0) + goto error; + + if (rc < 0) { + virReportSystemError(errno, "%s", _("failed to create socketpair")); + goto error; + } + + slirp->fd[0] = pair[0]; + slirp->fd[1] = pair[1]; + + return 0; + +error:
s/^e/ e/
+ VIR_FORCE_CLOSE(pair[0]); + VIR_FORCE_CLOSE(pair[1]); + return -1; +} + + +int +qemuSlirpGetFD(qemuSlirpPtr slirp) +{ + int fd = slirp->fd[0]; + slirp->fd[0] = -1; + return fd; +} + + +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 */ + if (virAsprintf(&id, "slirp-%s", virMacAddrFormat(&net->mac, macstr)) < 0) + return NULL; + + return id; +} + + +static char * +qemuSlirpGetDBusAddress(const char *stateDir, + const char *shortName, + const char *alias) +{ + char *name = NULL; + + if (virAsprintf(&name, "unix:path=%s/%s-%s-slirp", stateDir, shortName, alias) < 0) + return NULL; + + return name;
The same comment applies here as for qemuSlirpCreatePidFilename(). Moreover, we are going to need the path a few more times and the prefixed version ("unix:path=...") only once. So this should be renamed to qemuSlirpGetDBusPath() and then virAsprintf() can be used again to construct the prefixed version.
+} + + +void +qemuSlirpStop(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
This grabs a reference (virObjectRef()) so in order to use returns below, this needs to be VIR_AUTOUNREF().
+ VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def); + VIR_AUTOFREE(char *) id = qemuSlirpGetDBusVMStateId(net); + virErrorPtr orig_err; + int rc; + pid_t pid; + + if (!(pidfile = qemuSlirpCreatePidFilename( + cfg->stateDir, shortName, net->info.alias))) { + VIR_WARN("Unable to construct slirp pidfile path"); + return; + } + + qemuDBusVMStateRemove(driver, vm, id, hot);
@id can be NULL here (we are still checking for OOM errors).
+ + rc = qemuSlirpGetPid(cfg->slirpHelperName, + cfg->stateDir, shortName, net->info.alias, &pid);
Here you can use virPidFileReadPathIfAlive() directly, sicne you already have @pidfile constructed.
+ if (rc == 0 && pid != (pid_t)-1) { + char ebuf[1024]; + + VIR_DEBUG("Killing slirp process %lld", (long long)pid); + if (virProcessKill(pid, SIGKILL) < 0 && errno != ESRCH)
virProcessKillPainfully() to ensure the process is really killed.
+ VIR_ERROR(_("Failed to kill process %lld: %s"), + (long long)pid, + virStrerror(errno, ebuf, sizeof(ebuf))); + } + + virErrorPreserveLast(&orig_err); + if (virPidFileForceCleanupPath(pidfile) < 0) {
This doesn't do what you think it does. I'll get to that a few lines below, but pidfile is not locked, therefore this doesn't unlink the pidfile and returns with success.
+ VIR_WARN("Unable to kill slirp process"); + } else { + if (unlink(pidfile) < 0 &&
Which means, we can call this directly.
+ errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove stale pidfile %s"), + pidfile); + } + }
The dbus socket is also worth removing at this point.
+ virErrorRestore(&orig_err); + slirp->pid = 0; +} + + +int +qemuSlirpStart(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hotplug, + qemuProcessIncomingDefPtr incoming) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) cmdstr = NULL; + VIR_AUTOFREE(char *) addr = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; + VIR_AUTOFREE(char *) dbus_addr = NULL; + VIR_AUTOFREE(char *) id = NULL; + VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def); + size_t i; + virTimeBackOffVar timebackoff; + const unsigned long long timeout = 500 * 1000; /* ms */ + const char *dbus_path = NULL; + pid_t pid; + int cmdret = 0, exitstatus = 0; + + if (incoming && + !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The slirp-helper doesn't support migration")); + } + + if (!(pidfile = qemuSlirpCreatePidFilename( + cfg->stateDir, shortName, net->info.alias)))
Ehm, s/stateDir/slirpStateDir/. But since I'm suggesting to pass @cfg directly, this change is bogus then.
+ return -1; + + if (!(cmd = virCommandNew(cfg->slirpHelperName))) + return -1; + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + virCommandAddArgFormat(cmd, "--fd=%d", slirp->fd[1]); + virCommandPassFD(cmd, slirp->fd[1], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + slirp->fd[1] = -1; + + for (i = 0; i < net->guestIP.nips; i++) { + const virNetDevIPAddr *ip = net->guestIP.ips[i]; + const char *opt = ""; + + if (!(addr = virSocketAddrFormat(&ip->address))) + return -1; + + 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); + VIR_FREE(addr); + + if (ip->prefix) { + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { + virSocketAddr netmask; + VIR_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); + } + } + + if (qemuSlirpHasFeature(slirp, + QEMU_SLIRP_FEATURE_DBUS_P2P)) { + id = qemuSlirpGetDBusVMStateId(net); + dbus_addr = qemuSlirpGetDBusAddress(cfg->stateDir, + shortName, net->info.alias); + if (!id || !dbus_addr) { + return -1; + } + + dbus_path = dbus_addr + strlen("unix:path=");
If we'd call qemuSlirpGetDBusPath() here, then we can avoid this string arithmetic and construct dbus_addr using an virAsprintf() call.
+ + if (unlink(dbus_path) < 0 && errno != ENOENT) { + virReportSystemError(errno, _("Unable to unlink %s"), dbus_path); + return -1; + } + + 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"); + } + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0) + return -1; + + if (qemuSecurityCommandRun(driver, vm, cmd, + &exitstatus, &cmdret) < 0) + return -1; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'slirp'. exitstatus: %d"), exitstatus); + return -1; + } + + /* check that the helper has written its pid into the file */ + if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0) + return -1; + while (virTimeBackOffWait(&timebackoff)) { + int rc = qemuSlirpGetPid(cfg->slirpHelperName, + cfg->stateDir, shortName, net->info.alias, &pid); + if (rc < 0) + continue; + if (rc == 0 && pid == (pid_t)-1) + return -1; + break; + }
While this works, it doesn't do what you think it does. Because of the virCommandSetPidFile() call above, the pidfile is created and written to before virCommandRun() (hidden under qemuSecurityCommandRun() here) returns. So we don't have to wait for the pidfile. This is different to how pr-helper process is spawned (which is where you get the inspiration from). The pr-helper is passed '-f $pidfile' to, which makes the pr-helper to create the pidfile (and lock it). So in that case, the pidfile might not exist upon return from virCommandRun() and thus there has to be a wait loop. Also, virCommandSetPidFile() does not cause the pidfile to be locked. This means, in the qemuSlirpStop() we have to use virPidFileReadPathIfAlive() which checks if the /proc/$pid/exe points to the expected binary to avoid us killing a different process with the same PID after PID wrap. If the pidfile was locked then we could use virPidFileForceCleanupPath() - just like pr-helper code does.
+ + if (dbus_path) { + while (virTimeBackOffWait(&timebackoff)) { + if (!virFileExists(dbus_path)) + continue;
What if slirp-helper dies meanwhile? We need to 'ping' it periodically here to check it's still alive (virProcessKill(pid, 0)). Also, we might set up errfd and read from it because I bet slirp-helper will produce some sensible error message on its error output just before quitting.
+ break; + } + } + + if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + if (qemuDBusVMStateAdd(driver, vm, id, dbus_addr, hotplug) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register slirp migration")); + return -1;
I don't think that we can just return here. At this point, the slirp-helper is up & running so we need to kill it and remove its pid file and dbus socket.
+ } + } + + slirp->pid = pid; + return 0; +} diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h new file mode 100644 index 0000000000..f44edce543 --- /dev/null +++ b/src/qemu/qemu_slirp.h @@ -0,0 +1,81 @@ +/* + * qemu_slirp.h: QEMU Slirp support + * + * 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" +#include "qemu_process.h" +#include "vircommand.h" +
No need to include qemu_domain.h or vircommand.h.
+typedef enum { + QEMU_SLIRP_FEATURE_NONE = 0, + QEMU_SLIRP_FEATURE_IPV4, + QEMU_SLIRP_FEATURE_IPV6, + QEMU_SLIRP_FEATURE_TFTP, + QEMU_SLIRP_FEATURE_DBUS_ADDRESS, + QEMU_SLIRP_FEATURE_DBUS_P2P, + QEMU_SLIRP_FEATURE_MIGRATE, + QEMU_SLIRP_FEATURE_RESTRICT, + QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT, + + QEMU_SLIRP_FEATURE_LAST, +} qemuSlirpFeature; + +VIR_ENUM_DECL(qemuSlirpFeature); + + +struct _qemuSlirp { + int fd[2]; + virBitmapPtr features; + pid_t pid; +}; + + +qemuSlirpPtr qemuSlirpNew(void); + +qemuSlirpPtr qemuSlirpNewForHelper(const char *helper); + +void qemuSlirpFree(qemuSlirpPtr slirp); + +void qemuSlirpSetFeature(qemuSlirpPtr slirp, + qemuSlirpFeature feature); + +bool qemuSlirpHasFeature(const qemuSlirpPtr slirp, + qemuSlirpFeature feature); + +int qemuSlirpOpen(qemuSlirpPtr slirp, + virQEMUDriverPtr driver, + virDomainDefPtr def); + +int qemuSlirpStart(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot, + qemuProcessIncomingDefPtr incoming);
If this is turned to bool, then you can drop qemu_process.h include too.
+ +void qemuSlirpStop(qemuSlirpPtr slirp, + virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainNetDefPtr net, + bool hot); + +int qemuSlirpGetFD(qemuSlirpPtr slirp); + +VIR_DEFINE_AUTOPTR_FUNC(qemuSlirp, qemuSlirpFree);
With all that addressed: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Save & restore the slirp helper PID associated with a network interface & the probed features. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 137 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 +- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7315fe103e..b6f7e8bacc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -32,6 +32,7 @@ #include "qemu_migration.h" #include "qemu_migration_params.h" #include "qemu_security.h" +#include "qemu_slirp.h" #include "qemu_extdevice.h" #include "qemu_blockjob.h" #include "viralloc.h" @@ -1304,6 +1305,11 @@ qemuDomainNetworkPrivateNew(void) static void qemuDomainNetworkPrivateDispose(void *obj ATTRIBUTE_UNUSED) { + qemuDomainNetworkPrivatePtr priv = obj; + + if (priv->slirp) { + qemuSlirpFree(priv->slirp); + } } @@ -2630,6 +2636,63 @@ qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf, } +static bool +qemuDomainHasSlirp(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + + if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + return true; + } + + return false; +} + + +static int +qemuDomainObjPrivateXMLFormatSlirp(virBufferPtr buf, + virDomainObjPtr vm) +{ + size_t i; + + if (!qemuDomainHasSlirp(vm)) + return 0; + + virBufferAddLit(buf, "<slirp>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + size_t j; + + if (!slirp) + continue; + + virBufferAsprintf(buf, "<helper alias='%s' pid='%d'>\n", + net->info.alias, slirp->pid); + + virBufferAdjustIndent(buf, 2); + for (j = 0; j < QEMU_SLIRP_FEATURE_LAST; j++) { + if (qemuSlirpHasFeature(slirp, j)) { + virBufferAsprintf(buf, "<feature name='%s'/>\n", + qemuSlirpFeatureTypeToString(j)); + } + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</helper>\n"); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</slirp>\n"); + + + return 0; +} + static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainObjPtr vm) @@ -2731,6 +2794,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, if (qemuDomainObjPrivateXMLFormatBlockjobs(buf, vm) < 0) return -1; + if (qemuDomainObjPrivateXMLFormatSlirp(buf, vm) < 0) + return -1; + return 0; } @@ -3258,6 +3324,46 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, } +static int +qemuDomainObjPrivateXMLParseSlirpFeatures(xmlXPathContextPtr ctxt, + size_t h, + qemuSlirpPtr slirp) +{ + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + size_t i; + int n; + + if (virAsprintf(&path, "./slirp/helper[%ld]/feature", h + 1) < 0) + return -1; + + if ((n = virXPathNodeSet(path, ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to parse slirp-helper features")); + return -1; + } + + for (i = 0; i < n; i++) { + VIR_AUTOFREE(char *) str = virXMLPropString(nodes[i], "name"); + int feature; + + if (!str) + continue; + + feature = qemuSlirpFeatureTypeFromString(str); + if (feature < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown slirp feature %s"), str); + return -1; + } + + qemuSlirpSetFeature(slirp, feature); + } + + return 0; +} + + static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, virDomainObjPtr vm, @@ -3396,6 +3502,37 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./slirp/helper", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse slirp helper list")); + goto error; + } + if (n > 0) { + for (i = 0; i < n; i++) { + VIR_AUTOFREE(char *) alias = virXMLPropString(nodes[i], "alias"); + VIR_AUTOFREE(char *) pid = virXMLPropString(nodes[i], "pid"); + VIR_AUTOPTR(qemuSlirp) slirp = qemuSlirpNew(); + virDomainDeviceDef dev; + + if (!alias || !pid || !slirp || + virStrToLong_i(pid, NULL, 10, &slirp->pid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse slirp helper list")); + goto error; + } + + if (virDomainDefFindDevice(vm->def, alias, &dev, true) < 0 || + dev.type != VIR_DOMAIN_DEVICE_NET) + goto error; + + if (qemuDomainObjPrivateXMLParseSlirpFeatures(ctxt, i, slirp) < 0) + goto error; + + VIR_STEAL_PTR(QEMU_DOMAIN_NETWORK_PRIVATE(dev.data.net)->slirp, slirp); + } + } + VIR_FREE(nodes); + if (qemuDomainObjPrivateXMLParseAutomaticPlacement(ctxt, priv, driver) < 0) goto error; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7293c87d7c..84c39dfb54 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -521,7 +521,7 @@ typedef qemuDomainNetworkPrivate *qemuDomainNetworkPrivatePtr; struct _qemuDomainNetworkPrivate { virObject parent; - bool tmp_to_be_larger_than_parent; + qemuSlirpPtr slirp; }; -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Save & restore the slirp helper PID associated with a network interface & the probed features.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 137 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 +- 2 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7315fe103e..b6f7e8bacc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -32,6 +32,7 @@ #include "qemu_migration.h" #include "qemu_migration_params.h" #include "qemu_security.h" +#include "qemu_slirp.h" #include "qemu_extdevice.h" #include "qemu_blockjob.h" #include "viralloc.h" @@ -1304,6 +1305,11 @@ qemuDomainNetworkPrivateNew(void) static void qemuDomainNetworkPrivateDispose(void *obj ATTRIBUTE_UNUSED) { + qemuDomainNetworkPrivatePtr priv = obj; + + if (priv->slirp) { + qemuSlirpFree(priv->slirp);
Needless NULL check.
+ } }
@@ -2630,6 +2636,63 @@ qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf, }
+static bool +qemuDomainHasSlirp(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + + if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + return true; + } + + return false; +} + + +static int +qemuDomainObjPrivateXMLFormatSlirp(virBufferPtr buf, + virDomainObjPtr vm) +{ + size_t i; + + if (!qemuDomainHasSlirp(vm)) + return 0; + + virBufferAddLit(buf, "<slirp>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + size_t j; + + if (!slirp) + continue; + + virBufferAsprintf(buf, "<helper alias='%s' pid='%d'>\n", + net->info.alias, slirp->pid); + + virBufferAdjustIndent(buf, 2); + for (j = 0; j < QEMU_SLIRP_FEATURE_LAST; j++) { + if (qemuSlirpHasFeature(slirp, j)) { + virBufferAsprintf(buf, "<feature name='%s'/>\n", + qemuSlirpFeatureTypeToString(j)); + } + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</helper>\n"); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</slirp>\n"); + + + return 0; +} + static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainObjPtr vm) @@ -2731,6 +2794,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, if (qemuDomainObjPrivateXMLFormatBlockjobs(buf, vm) < 0) return -1;
+ if (qemuDomainObjPrivateXMLFormatSlirp(buf, vm) < 0) + return -1; + return 0; }
@@ -3258,6 +3324,46 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, }
+static int +qemuDomainObjPrivateXMLParseSlirpFeatures(xmlXPathContextPtr ctxt, + size_t h, + qemuSlirpPtr slirp)
We tend to pass xmlNodePtr here among with ctxt and then use VIR_XPATH_NODE_AUTORESTORE(ctxt).
+{ + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + size_t i; + int n; + + if (virAsprintf(&path, "./slirp/helper[%ld]/feature", h + 1) < 0) + return -1; + + if ((n = virXPathNodeSet(path, ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to parse slirp-helper features")); + return -1; + } + + for (i = 0; i < n; i++) { + VIR_AUTOFREE(char *) str = virXMLPropString(nodes[i], "name"); + int feature; + + if (!str) + continue; + + feature = qemuSlirpFeatureTypeFromString(str); + if (feature < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown slirp feature %s"), str); + return -1; + } + + qemuSlirpSetFeature(slirp, feature); + } + + return 0; +} + + static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, virDomainObjPtr vm, @@ -3396,6 +3502,37 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, } VIR_FREE(nodes);
+ if ((n = virXPathNodeSet("./slirp/helper", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse slirp helper list")); + goto error; + } + if (n > 0) {
I know you put this check here to be consistent with the rest of the code, but that code needs to alloc something in case of n > 0 and only after that it execs for ().
+ for (i = 0; i < n; i++) { + VIR_AUTOFREE(char *) alias = virXMLPropString(nodes[i], "alias"); + VIR_AUTOFREE(char *) pid = virXMLPropString(nodes[i], "pid"); + VIR_AUTOPTR(qemuSlirp) slirp = qemuSlirpNew(); + virDomainDeviceDef dev; + + if (!alias || !pid || !slirp || + virStrToLong_i(pid, NULL, 10, &slirp->pid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse slirp helper list")); + goto error; + } + + if (virDomainDefFindDevice(vm->def, alias, &dev, true) < 0 || + dev.type != VIR_DOMAIN_DEVICE_NET) + goto error; + + if (qemuDomainObjPrivateXMLParseSlirpFeatures(ctxt, i, slirp) < 0) + goto error; + + VIR_STEAL_PTR(QEMU_DOMAIN_NETWORK_PRIVATE(dev.data.net)->slirp, slirp); + } + } + VIR_FREE(nodes); + if (qemuDomainObjPrivateXMLParseAutomaticPlacement(ctxt, priv, driver) < 0) goto error;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7293c87d7c..84c39dfb54 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -521,7 +521,7 @@ typedef qemuDomainNetworkPrivate *qemuDomainNetworkPrivatePtr; struct _qemuDomainNetworkPrivate { virObject parent;
- bool tmp_to_be_larger_than_parent; + qemuSlirpPtr slirp;
This now requires include of qemu_slirp.h Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> For VM started and migrated/saved without slirp-helpers, let's prevent the automatic setup (as it would fail to migrate otherwise). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 8 ++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b6f7e8bacc..04301c0a15 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2652,6 +2652,24 @@ qemuDomainHasSlirp(virDomainObjPtr vm) } +static bool +qemuDomainGetSlirpHelperOk(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + + /* if there is a builtin slirp, prevent slirp-helper */ + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + !QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + return false; + } + + return true; +} + + static int qemuDomainObjPrivateXMLFormatSlirp(virBufferPtr buf, virDomainObjPtr vm) @@ -14546,7 +14564,7 @@ qemuDomainSaveCookieDispose(void *obj) qemuDomainSaveCookiePtr -qemuDomainSaveCookieNew(virDomainObjPtr vm ATTRIBUTE_UNUSED) +qemuDomainSaveCookieNew(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainSaveCookiePtr cookie = NULL; @@ -14560,7 +14578,10 @@ qemuDomainSaveCookieNew(virDomainObjPtr vm ATTRIBUTE_UNUSED) if (priv->origCPU && !(cookie->cpu = virCPUDefCopy(vm->def->cpu))) goto error; - VIR_DEBUG("Save cookie %p, cpu=%p", cookie, cookie->cpu); + cookie->slirpHelper = qemuDomainGetSlirpHelperOk(vm); + + VIR_DEBUG("Save cookie %p, cpu=%p, slirpHelper=%d", + cookie, cookie->cpu, cookie->slirpHelper); return cookie; @@ -14586,6 +14607,8 @@ qemuDomainSaveCookieParse(xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, &cookie->cpu) < 0) goto error; + cookie->slirpHelper = virXPathBoolean("boolean(./slirpHelper)", ctxt) > 0; + *obj = (virObjectPtr) cookie; return 0; @@ -14605,6 +14628,9 @@ qemuDomainSaveCookieFormat(virBufferPtr buf, virCPUDefFormatBufFull(buf, cookie->cpu, NULL) < 0) return -1; + if (cookie->slirpHelper) + virBufferAddLit(buf, "<slirpHelper/>\n"); + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 84c39dfb54..caa2aff965 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -394,6 +394,7 @@ struct _qemuDomainObjPrivate { virHashTablePtr blockjobs; virHashTablePtr dbusVMStates; + bool disableSlirp; }; #define QEMU_DOMAIN_PRIVATE(vm) \ @@ -559,6 +560,7 @@ struct _qemuDomainSaveCookie { virObject parent; virCPUDefPtr cpu; + bool slirpHelper; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff83d1c024..4b49203738 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6971,6 +6971,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, bool start_paused, qemuDomainAsyncJob asyncJob) { + qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; bool restored = false; virObjectEventPtr event; @@ -7011,6 +7012,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) goto cleanup; + if (!cookie->slirpHelper) + priv->disableSlirp = true; + if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, asyncJob, "stdio", *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, @@ -16654,6 +16658,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virCPUDefFree(priv->origCPU); VIR_STEAL_PTR(priv->origCPU, origCPU); } + + if (cookie && !cookie->slirpHelper) + priv->disableSlirp = true; + } else { /* Transitions 2, 3 */ load: -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
For VM started and migrated/saved without slirp-helpers, let's prevent the automatic setup (as it would fail to migrate otherwise).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 8 ++++++++ 3 files changed, 38 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On 8/8/19 10:55 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
For VM started and migrated/saved without slirp-helpers, let's prevent the automatic setup (as it would fail to migrate otherwise).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 8 ++++++++ 3 files changed, 38 insertions(+), 2 deletions(-)
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff83d1c024..4b49203738 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6971,6 +6971,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, bool start_paused, qemuDomainAsyncJob asyncJob) { + qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; bool restored = false; virObjectEventPtr event; @@ -7011,6 +7012,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) goto cleanup;
+ if (!cookie->slirpHelper) + priv->disableSlirp = true; +
Coverity lets me know that the above will need to have a "cookie &&" in the if statement (similar to the lines just above it and of course the check just below for @cookie being NULL... John
if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, asyncJob, "stdio", *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, @@ -16654,6 +16658,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virCPUDefFree(priv->origCPU); VIR_STEAL_PTR(priv->origCPU, origCPU); } + + if (cookie && !cookie->slirpHelper) + priv->disableSlirp = true;
hmm... yeah, just like this ;-)
+ } else { /* Transitions 2, 3 */ load:

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_migration.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7c6be201b9..86b4bfd970 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1114,6 +1114,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, bool remote, unsigned int flags) { + qemuDomainObjPrivatePtr priv = vm->privateData; int nsnapshots; int pauseReason; size_t i; @@ -1140,7 +1141,13 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, _("cannot migrate domain with I/O error")); 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; } /* following checks don't make sense for offline migration */ -- 2.23.0.rc1

On 8/8/19 4:55 PM, 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_migration.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_migration.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 86b4bfd970..c607e9f909 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -39,6 +39,7 @@ #include "qemu_hotplug.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_slirp.h" #include "qemu_block.h" #include "domain_audit.h" @@ -1150,6 +1151,17 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, return false; } + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("a slirp-helper cannot be migrated")); + return false; + } + } + /* following checks don't make sense for offline migration */ if (!(flags & VIR_MIGRATE_OFFLINE)) { if (qemuProcessAutoDestroyActive(driver, vm)) { -- 2.23.0.rc1

On 8/8/19 4:55 PM, 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_migration.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> If a slirp-helper is associated with a network interface, prepare/start/stop the process via qemu-extdevice. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 47 +++++++++++++++++++++++++++++++++------ src/qemu/qemu_extdevice.h | 5 +++-- src/qemu/qemu_process.c | 4 ++-- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 0aa050cb0a..179552a3ae 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -23,6 +23,7 @@ #include "qemu_extdevice.h" #include "qemu_domain.h" #include "qemu_tpm.h" +#include "qemu_slirp.h" #include "viralloc.h" #include "virlog.h" @@ -87,14 +88,24 @@ qemuExtDevicesInitPaths(virQEMUDriverPtr driver, */ int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, - virDomainDefPtr def) + virDomainObjPtr vm) { - int ret = 0; + virDomainDefPtr def = vm->def; + size_t i; - if (def->tpm) - ret = qemuExtTPMPrepareHost(driver, def); + if (def->tpm && + qemuExtTPMPrepareHost(driver, def) < 0) + return -1; - return ret; + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && qemuSlirpOpen(slirp, driver, def) < 0) + return -1; + } + + return 0; } @@ -114,15 +125,26 @@ int qemuExtDevicesStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainLogContextPtr logCtxt, - bool incomingMigration) + qemuProcessIncomingDefPtr incoming) { + virDomainDefPtr def = vm->def; int ret = 0; + size_t i; if (qemuExtDevicesInitPaths(driver, vm->def) < 0) return -1; if (vm->def->tpm) - ret = qemuExtTPMStart(driver, vm, logCtxt, incomingMigration); + ret = qemuExtTPMStart(driver, vm, logCtxt, incoming != NULL); + + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && + qemuSlirpStart(slirp, vm, driver, net, false, incoming) < 0) + return -1; + } return ret; } @@ -132,11 +154,22 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainObjPtr vm) { + virDomainDefPtr def = vm->def; + size_t i; + if (qemuExtDevicesInitPaths(driver, vm->def) < 0) return; if (vm->def->tpm) qemuExtTPMStop(driver, vm); + + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp) + qemuSlirpStop(slirp, vm, driver, net, false); + } } diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index cdd20c28ef..a526308c51 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -22,6 +22,7 @@ #include "qemu_conf.h" #include "qemu_domain.h" +#include "qemu_process.h" int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -31,7 +32,7 @@ int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, ATTRIBUTE_RETURN_CHECK; int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, - virDomainDefPtr def) + virDomainObjPtr vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; @@ -42,7 +43,7 @@ void qemuExtDevicesCleanupHost(virQEMUDriverPtr driver, int qemuExtDevicesStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainLogContextPtr logCtxt, - bool incomingMigration) + qemuProcessIncomingDefPtr incoming) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8d740979d..d41ee0f25b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6602,7 +6602,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, goto cleanup; VIR_DEBUG("Preparing external devices"); - if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) + if (qemuExtDevicesPrepareHost(driver, vm) < 0) goto cleanup; if (qemuProcessPrepareSEVGuestInput(vm) < 0) @@ -6776,7 +6776,7 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessGenID(vm, flags) < 0) goto cleanup; - if (qemuExtDevicesStart(driver, vm, logCtxt, incoming != NULL) < 0) + if (qemuExtDevicesStart(driver, vm, logCtxt, incoming) < 0) goto cleanup; VIR_DEBUG("Building emulator command line"); -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
If a slirp-helper is associated with a network interface, prepare/start/stop the process via qemu-extdevice.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_extdevice.c | 47 +++++++++++++++++++++++++++++++++------ src/qemu/qemu_extdevice.h | 5 +++-- src/qemu/qemu_process.c | 4 ++-- 3 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 0aa050cb0a..179552a3ae 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -23,6 +23,7 @@ #include "qemu_extdevice.h" #include "qemu_domain.h" #include "qemu_tpm.h" +#include "qemu_slirp.h"
#include "viralloc.h" #include "virlog.h" @@ -87,14 +88,24 @@ qemuExtDevicesInitPaths(virQEMUDriverPtr driver, */ int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, - virDomainDefPtr def) + virDomainObjPtr vm) { - int ret = 0; + virDomainDefPtr def = vm->def; + size_t i;
- if (def->tpm) - ret = qemuExtTPMPrepareHost(driver, def); + if (def->tpm && + qemuExtTPMPrepareHost(driver, def) < 0) + return -1;
- return ret; + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && qemuSlirpOpen(slirp, driver, def) < 0) + return -1; + } + + return 0; }
@@ -114,15 +125,26 @@ int qemuExtDevicesStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainLogContextPtr logCtxt, - bool incomingMigration) + qemuProcessIncomingDefPtr incoming)
No need for this change.
{ + virDomainDefPtr def = vm->def; int ret = 0; + size_t i;
if (qemuExtDevicesInitPaths(driver, vm->def) < 0) return -1;
if (vm->def->tpm) - ret = qemuExtTPMStart(driver, vm, logCtxt, incomingMigration); + ret = qemuExtTPMStart(driver, vm, logCtxt, incoming != NULL); + + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && + qemuSlirpStart(slirp, vm, driver, net, false, incoming) < 0) + return -1; + }
return ret; } @@ -132,11 +154,22 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainObjPtr vm) { + virDomainDefPtr def = vm->def; + size_t i; + if (qemuExtDevicesInitPaths(driver, vm->def) < 0) return;
if (vm->def->tpm) qemuExtTPMStop(driver, vm); + + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp) + qemuSlirpStop(slirp, vm, driver, net, false); + } }
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index cdd20c28ef..a526308c51 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -22,6 +22,7 @@
#include "qemu_conf.h" #include "qemu_domain.h" +#include "qemu_process.h"
int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -31,7 +32,7 @@ int qemuExtDeviceLogCommand(virQEMUDriverPtr driver, ATTRIBUTE_RETURN_CHECK;
int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, - virDomainDefPtr def) + virDomainObjPtr vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
@@ -42,7 +43,7 @@ void qemuExtDevicesCleanupHost(virQEMUDriverPtr driver, int qemuExtDevicesStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainLogContextPtr logCtxt, - bool incomingMigration) + qemuProcessIncomingDefPtr incoming)
This change is not needed and thus so the include of qemu_process.h is needless too.
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8d740979d..d41ee0f25b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6602,7 +6602,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, goto cleanup;
VIR_DEBUG("Preparing external devices"); - if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) + if (qemuExtDevicesPrepareHost(driver, vm) < 0) goto cleanup;
if (qemuProcessPrepareSEVGuestInput(vm) < 0) @@ -6776,7 +6776,7 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessGenID(vm, flags) < 0) goto cleanup;
- if (qemuExtDevicesStart(driver, vm, logCtxt, incoming != NULL) < 0) + if (qemuExtDevicesStart(driver, vm, logCtxt, incoming) < 0)
Just keep this as is. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> If a slirp-helper is associated with a network interface (after probing & preparing succesfully), pass the socket fd to QEMU and use "-net socket,fd=". Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 35 +++++++++++++++++++++++++++++------ src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 4 +++- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4357aa2fe1..90e61a336e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -28,6 +28,7 @@ #include "qemu_alias.h" #include "qemu_security.h" #include "qemu_dbus.h" +#include "qemu_slirp.h" #include "qemu_block.h" #include "cpu/cpu.h" #include "dirname.h" @@ -4008,7 +4009,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, char **tapfd, size_t tapfdSize, char **vhostfd, - size_t vhostfdSize) + size_t vhostfdSize, + const char *slirpfd) { bool is_tap = false; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4079,6 +4081,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; case VIR_DOMAIN_NET_TYPE_USER: + if (slirpfd) { + virBufferAsprintf(&buf, "socket,fd=%s,", + slirpfd); + break; + } + virBufferAddLit(&buf, "user,"); for (i = 0; i < net->guestIP.nips; i++) { const virNetDevIPAddr *ip = net->guestIP.ips[i]; @@ -8634,10 +8642,10 @@ qemuInterfaceVhostuserConnect(virQEMUDriverPtr driver, static int qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, + virDomainObjPtr vm, virLogManagerPtr logManager, virSecurityManagerPtr secManager, virCommandPtr cmd, - virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, unsigned int bootindex, @@ -8646,6 +8654,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, size_t *nnicindexes, int **nicindexes) { + virDomainDefPtr def = vm->def; int ret = -1; char *nic = NULL; char *host = NULL; @@ -8656,9 +8665,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, size_t vhostfdSize = 0; char **tapfdName = NULL; char **vhostfdName = NULL; + VIR_AUTOFREE(char *) slirpfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); virNetDevBandwidthPtr actualBandwidth; bool requireNicdev = false; + qemuSlirpPtr slirp; size_t i; @@ -8884,6 +8895,16 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, goto cleanup; } + slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + if (slirp && !standalone) { + int slirpfd = qemuSlirpGetFD(slirp); + virCommandPassFD(cmd, slirpfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (virAsprintf(&slirpfdName, "%d", slirpfd) < 0) + goto cleanup; + } + + for (i = 0; i < tapfdSize; i++) { if (qemuSecuritySetTapFDLabel(driver->securityManager, def, tapfd[i]) < 0) @@ -8908,7 +8929,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, if (!(host = qemuBuildHostNetStr(net, driver, tapfdName, tapfdSize, - vhostfdName, vhostfdSize))) + vhostfdName, vhostfdSize, + slirpfdName))) goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); @@ -8976,10 +8998,10 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, */ static int qemuBuildNetCommandLine(virQEMUDriverPtr driver, + virDomainObjPtr vm, virLogManagerPtr logManager, virSecurityManagerPtr secManager, virCommandPtr cmd, - virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virNetDevVPortProfileOp vmop, bool standalone, @@ -8990,6 +9012,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, size_t i; int last_good_net = -1; virErrorPtr originalError = NULL; + virDomainDefPtr def = vm->def; if (def->nnets) { unsigned int bootNet = 0; @@ -9005,7 +9028,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (qemuBuildInterfaceCommandLine(driver, logManager, secManager, cmd, def, net, + if (qemuBuildInterfaceCommandLine(driver, vm, logManager, secManager, cmd, net, qemuCaps, bootNet, vmop, standalone, nnicindexes, nicindexes) < 0) @@ -10815,7 +10838,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildFilesystemCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildNetCommandLine(driver, logManager, secManager, cmd, def, + if (qemuBuildNetCommandLine(driver, vm, logManager, secManager, cmd, qemuCaps, vmop, standalone, nnicindexes, nicindexes, &bootHostdevNet) < 0) goto error; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3a957c52fc..6faa6f2b18 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -90,7 +90,8 @@ char *qemuBuildHostNetStr(virDomainNetDefPtr net, char **tapfd, size_t tapfdSize, char **vhostfd, - size_t vhostfdSize); + size_t vhostfdSize, + const char *slirpfd); /* Current, best practice */ char *qemuBuildNicDevStr(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 028921fb47..43c3f0755b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1135,6 +1135,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; virErrorPtr originalError = NULL; + VIR_AUTOFREE(char *) slirpfdName = NULL; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1373,7 +1374,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, if (!(netstr = qemuBuildHostNetStr(net, driver, tapfdName, tapfdSize, - vhostfdName, vhostfdSize))) + vhostfdName, vhostfdSize, + slirpfdName))) goto cleanup; qemuDomainObjEnterMonitor(driver, vm); -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
If a slirp-helper is associated with a network interface (after probing & preparing succesfully), pass the socket fd to QEMU and use "-net socket,fd=".
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 35 +++++++++++++++++++++++++++++------ src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 4 +++- 3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4357aa2fe1..90e61a336e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -28,6 +28,7 @@ #include "qemu_alias.h" #include "qemu_security.h" #include "qemu_dbus.h" +#include "qemu_slirp.h" #include "qemu_block.h" #include "cpu/cpu.h" #include "dirname.h" @@ -4008,7 +4009,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, char **tapfd, size_t tapfdSize, char **vhostfd, - size_t vhostfdSize) + size_t vhostfdSize, + const char *slirpfd) { bool is_tap = false; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4079,6 +4081,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break;
case VIR_DOMAIN_NET_TYPE_USER: + if (slirpfd) { + virBufferAsprintf(&buf, "socket,fd=%s,", + slirpfd); + break; + }
No, please don't put a break at random places within 'case' bodies.
+ virBufferAddLit(&buf, "user,"); for (i = 0; i < net->guestIP.nips; i++) { const virNetDevIPAddr *ip = net->guestIP.ips[i]; @@ -8634,10 +8642,10 @@ qemuInterfaceVhostuserConnect(virQEMUDriverPtr driver,
static int qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, + virDomainObjPtr vm, virLogManagerPtr logManager, virSecurityManagerPtr secManager, virCommandPtr cmd, - virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, unsigned int bootindex, @@ -8646,6 +8654,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, size_t *nnicindexes, int **nicindexes) { + virDomainDefPtr def = vm->def; int ret = -1; char *nic = NULL; char *host = NULL; @@ -8656,9 +8665,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, size_t vhostfdSize = 0; char **tapfdName = NULL; char **vhostfdName = NULL; + VIR_AUTOFREE(char *) slirpfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); virNetDevBandwidthPtr actualBandwidth; bool requireNicdev = false; + qemuSlirpPtr slirp; size_t i;
@@ -8884,6 +8895,16 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, goto cleanup; }
+ slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + if (slirp && !standalone) { + int slirpfd = qemuSlirpGetFD(slirp); + virCommandPassFD(cmd, slirpfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (virAsprintf(&slirpfdName, "%d", slirpfd) < 0) + goto cleanup; + } + + for (i = 0; i < tapfdSize; i++) { if (qemuSecuritySetTapFDLabel(driver->securityManager, def, tapfd[i]) < 0) @@ -8908,7 +8929,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
if (!(host = qemuBuildHostNetStr(net, driver, tapfdName, tapfdSize, - vhostfdName, vhostfdSize))) + vhostfdName, vhostfdSize, + slirpfdName))) goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL);
@@ -8976,10 +8998,10 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, */ static int qemuBuildNetCommandLine(virQEMUDriverPtr driver, + virDomainObjPtr vm, virLogManagerPtr logManager, virSecurityManagerPtr secManager, virCommandPtr cmd, - virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virNetDevVPortProfileOp vmop, bool standalone, @@ -8990,6 +9012,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, size_t i; int last_good_net = -1; virErrorPtr originalError = NULL; + virDomainDefPtr def = vm->def;
if (def->nnets) { unsigned int bootNet = 0; @@ -9005,7 +9028,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i];
- if (qemuBuildInterfaceCommandLine(driver, logManager, secManager, cmd, def, net, + if (qemuBuildInterfaceCommandLine(driver, vm, logManager, secManager, cmd, net, qemuCaps, bootNet, vmop, standalone, nnicindexes, nicindexes) < 0) @@ -10815,7 +10838,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildFilesystemCommandLine(cmd, def, qemuCaps) < 0) goto error;
- if (qemuBuildNetCommandLine(driver, logManager, secManager, cmd, def, + if (qemuBuildNetCommandLine(driver, vm, logManager, secManager, cmd, qemuCaps, vmop, standalone, nnicindexes, nicindexes, &bootHostdevNet) < 0) goto error; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3a957c52fc..6faa6f2b18 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -90,7 +90,8 @@ char *qemuBuildHostNetStr(virDomainNetDefPtr net, char **tapfd, size_t tapfdSize, char **vhostfd, - size_t vhostfdSize); + size_t vhostfdSize, + const char *slirpfd);
/* Current, best practice */ char *qemuBuildNicDevStr(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 028921fb47..43c3f0755b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1135,6 +1135,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; virErrorPtr originalError = NULL; + VIR_AUTOFREE(char *) slirpfdName = NULL; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1373,7 +1374,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (!(netstr = qemuBuildHostNetStr(net, driver, tapfdName, tapfdSize, - vhostfdName, vhostfdSize))) + vhostfdName, vhostfdSize, + slirpfdName)))
This looks like a spurious change. Just pass NULL and introduce the variable once you need it. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> When the network interface is of "user" type, and QEMU has the "-net socket,fd=" datagram support, call qemuInterfacePrepareSlirp() to probe and associate a slirp-helper with the interface. The usage of automated slirp-helper can be prevented with disableSlirp (in particular when resuming a VM that didn't start with slirp-helper before). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interface.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_interface.h | 4 ++++ src/qemu/qemu_process.c | 16 +++++++++++++--- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index c8effa68f4..e58f464d87 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -603,6 +603,33 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, } +qemuSlirpPtr +qemuInterfacePrepareSlirp(virQEMUDriverPtr driver, + virDomainNetDefPtr net) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOPTR(qemuSlirp) slirp = NULL; + size_t i; + + if (!(slirp = qemuSlirpNewForHelper(cfg->slirpHelperName))) + return NULL; + + for (i = 0; i < net->guestIP.nips; i++) { + const virNetDevIPAddr *ip = net->guestIP.ips[i]; + + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET) && + !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_IPV4)) + return NULL; + + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6) && + !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_IPV6)) + return NULL; + } + + VIR_RETURN_PTR(slirp); +} + + /** * qemuInterfaceOpenVhostNet: * @def: domain definition diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h index 5a2f87e532..0464b903d7 100644 --- a/src/qemu/qemu_interface.h +++ b/src/qemu/qemu_interface.h @@ -24,6 +24,7 @@ #include "domain_conf.h" #include "qemu_conf.h" #include "qemu_domain.h" +#include "qemu_slirp.h" int qemuInterfaceStartDevice(virDomainNetDefPtr net); int qemuInterfaceStartDevices(virDomainDefPtr def); @@ -54,3 +55,6 @@ int qemuInterfaceOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, int *vhostfd, size_t *vhostfdSize); + +qemuSlirpPtr qemuInterfacePrepareSlirp(virQEMUDriverPtr driver, + virDomainNetDefPtr net); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d41ee0f25b..d1d73d55c7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5688,8 +5688,11 @@ qemuProcessInit(virQEMUDriverPtr driver, * qemuProcessNetworkPrepareDevices */ static int -qemuProcessNetworkPrepareDevices(virDomainDefPtr def) +qemuProcessNetworkPrepareDevices(virQEMUDriverPtr driver, + virDomainObjPtr vm) { + virDomainDefPtr def = vm->def; + qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; size_t i; virConnectPtr conn = NULL; @@ -5734,7 +5737,14 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def) } if (virDomainHostdevInsert(def, hostdev) < 0) goto cleanup; - } + } else if (actualType == VIR_DOMAIN_NET_TYPE_USER && + !priv->disableSlirp && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM)) { + qemuSlirpPtr slirp = qemuInterfacePrepareSlirp(driver, net); + + QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; + } + } ret = 0; cleanup: @@ -6535,7 +6545,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, * will need to be setup. */ VIR_DEBUG("Preparing network devices"); - if (qemuProcessNetworkPrepareDevices(vm->def) < 0) + if (qemuProcessNetworkPrepareDevices(driver, vm) < 0) goto cleanup; /* Must be run before security labelling */ -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
When the network interface is of "user" type, and QEMU has the "-net socket,fd=" datagram support, call qemuInterfacePrepareSlirp() to probe and associate a slirp-helper with the interface.
The usage of automated slirp-helper can be prevented with disableSlirp (in particular when resuming a VM that didn't start with slirp-helper before).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_interface.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_interface.h | 4 ++++ src/qemu/qemu_process.c | 16 +++++++++++++--- 3 files changed, 44 insertions(+), 3 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 13 ++++++++++--- src/qemu/qemu_monitor.h | 3 ++- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 43c3f0755b..fcbf7a8aa9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1136,6 +1136,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; virErrorPtr originalError = NULL; VIR_AUTOFREE(char *) slirpfdName = NULL; + int slirpfd = -1; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1315,7 +1316,26 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, break; case VIR_DOMAIN_NET_TYPE_USER: - /* No preparation needed. */ + if (!priv->disableSlirp && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM)) { + qemuSlirpPtr slirp = qemuInterfacePrepareSlirp(driver, net); + + if (!slirp) + break; + + QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; + + if (qemuSlirpOpen(slirp, driver, vm->def) < 0 || + qemuSlirpStart(slirp, vm, driver, net, true, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to start slirp")); + goto cleanup; + } + + slirpfd = qemuSlirpGetFD(slirp); + if (virAsprintf(&slirpfdName, "slirpfd-%s", net->info.alias) < 0) + goto cleanup; + } break; case VIR_DOMAIN_NET_TYPE_SERVER: @@ -1391,7 +1411,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize) < 0) { + vhostfd, vhostfdName, vhostfdSize, + slirpfd, slirpfdName) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; @@ -1506,6 +1527,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, VIR_FREE(charDevAlias); virObjectUnref(conn); virDomainCCWAddressSetFree(ccwaddrs); + VIR_FORCE_CLOSE(slirpfd); return ret; @@ -1516,6 +1538,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virErrorPreserveLast(&originalError); if (virAsprintf(&netdev_name, "host%s", net->info.alias) >= 0) { qemuDomainObjEnterMonitor(driver, vm); + if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true); if (charDevPlugged && qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) VIR_WARN("Failed to remove associated chardev %s", charDevAlias); @@ -2201,7 +2225,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (guestfwd) { if (qemuMonitorAddNetdev(priv->mon, devstr, - NULL, NULL, 0, NULL, NULL, 0) < 0) + NULL, NULL, 0, NULL, NULL, 0, -1, NULL) < 0) goto exit_monitor; } else { if (qemuMonitorAddDevice(priv->mon, devstr) < 0) @@ -4674,6 +4698,9 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; + if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true); + virDomainAuditNet(vm, net, NULL, "detach", true); for (i = 0; i < vm->def->nnets; i++) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a880da3ab6..84af24958a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2844,15 +2844,17 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr, int *tapfd, char **tapfdName, int tapfdSize, - int *vhostfd, char **vhostfdName, int vhostfdSize) + int *vhostfd, char **vhostfdName, int vhostfdSize, + int slirpfd, char *slirpfdName) { int ret = -1; size_t i = 0, j = 0; VIR_DEBUG("netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d" - "vhostfd=%p vhostfdName=%p vhostfdSize=%d", + "vhostfd=%p vhostfdName=%p vhostfdSize=%d" + "slirpfd=%d slirpfdName=%s", netdevstr, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize); + vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName); QEMU_CHECK_MONITOR(mon); @@ -2865,6 +2867,11 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon, goto cleanup; } + if (slirpfd != -1) { + if (qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0) + goto cleanup; + } + ret = qemuMonitorJSONAddNetdev(mon, netdevstr); cleanup: diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 88c9702530..c675865e3f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -880,7 +880,8 @@ int qemuMonitorRemoveFd(qemuMonitorPtr mon, int fdset, int fd); int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr, int *tapfd, char **tapfdName, int tapfdSize, - int *vhostfd, char **vhostfdName, int vhostfdSize); + int *vhostfd, char **vhostfdName, int vhostfdSize, + int slirpfd, char *slirpfdName); int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias); -- 2.23.0.rc1

On 8/8/19 4:55 PM, 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_hotplug.c | 33 ++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 13 ++++++++++--- src/qemu/qemu_monitor.h | 3 ++- 3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 43c3f0755b..fcbf7a8aa9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1136,6 +1136,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; virErrorPtr originalError = NULL; VIR_AUTOFREE(char *) slirpfdName = NULL; + int slirpfd = -1; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1315,7 +1316,26 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, break;
case VIR_DOMAIN_NET_TYPE_USER: - /* No preparation needed. */ + if (!priv->disableSlirp && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM)) { + qemuSlirpPtr slirp = qemuInterfacePrepareSlirp(driver, net); + + if (!slirp) + break; + + QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; + + if (qemuSlirpOpen(slirp, driver, vm->def) < 0 || + qemuSlirpStart(slirp, vm, driver, net, true, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to start slirp")); + goto cleanup; + } + + slirpfd = qemuSlirpGetFD(slirp); + if (virAsprintf(&slirpfdName, "slirpfd-%s", net->info.alias) < 0) + goto cleanup; + } break;
case VIR_DOMAIN_NET_TYPE_SERVER: @@ -1391,7 +1411,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize) < 0) { + vhostfd, vhostfdName, vhostfdSize, + slirpfd, slirpfdName) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; @@ -1506,6 +1527,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, VIR_FREE(charDevAlias); virObjectUnref(conn); virDomainCCWAddressSetFree(ccwaddrs); + VIR_FORCE_CLOSE(slirpfd);
return ret;
@@ -1516,6 +1538,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virErrorPreserveLast(&originalError); if (virAsprintf(&netdev_name, "host%s", net->info.alias) >= 0) { qemuDomainObjEnterMonitor(driver, vm); + if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true);
This can be done outside of EnterMonitor(). qemuSlirpStop() will enter the monitor on its own.
if (charDevPlugged && qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) VIR_WARN("Failed to remove associated chardev %s", charDevAlias); @@ -2201,7 +2225,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (guestfwd) { if (qemuMonitorAddNetdev(priv->mon, devstr, - NULL, NULL, 0, NULL, NULL, 0) < 0) + NULL, NULL, 0, NULL, NULL, 0, -1, NULL) < 0) goto exit_monitor; } else { if (qemuMonitorAddDevice(priv->mon, devstr) < 0) @@ -4674,6 +4698,9 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup;
+ if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) + qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net, true); + virDomainAuditNet(vm, net, NULL, "detach", true);
for (i = 0; i < vm->def->nnets; i++) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a880da3ab6..84af24958a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2844,15 +2844,17 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr, int *tapfd, char **tapfdName, int tapfdSize, - int *vhostfd, char **vhostfdName, int vhostfdSize) + int *vhostfd, char **vhostfdName, int vhostfdSize, + int slirpfd, char *slirpfdName) { int ret = -1; size_t i = 0, j = 0;
VIR_DEBUG("netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d" - "vhostfd=%p vhostfdName=%p vhostfdSize=%d", + "vhostfd=%p vhostfdName=%p vhostfdSize=%d" + "slirpfd=%d slirpfdName=%s", netdevstr, tapfd, tapfdName, tapfdSize, - vhostfd, vhostfdName, vhostfdSize); + vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName);
QEMU_CHECK_MONITOR(mon);
@@ -2865,6 +2867,11 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon, goto cleanup; }
+ if (slirpfd != -1) { + if (qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0) + goto cleanup; + } + ret = qemuMonitorJSONAddNetdev(mon, netdevstr);
cleanup:
If this function fails, we need to let qemu close the slirpfd: if (qemuMonitorCloseFileHandle(mon, slirpfdName) < 0) VIR_WARN("failed to close device handle '%s'", slirpfdName); Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

From: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../net-user.x86_64-4.0.0.args | 34 +++++++++++++++++++ tests/qemuxml2argvtest.c | 16 +++++++++ tests/testutilsqemu.h | 1 + 3 files changed, 51 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args diff --git a/tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args b/tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args new file mode 100644 index 0000000000..ef4b77be92 --- /dev/null +++ b/tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-i440fx-4.0,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-netdev socket,fd=42,id=hostnet0 \ +-device rtl8139,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c166fd18d6..1d1c3292ec 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -17,6 +17,7 @@ # include "qemu/qemu_domain.h" # include "qemu/qemu_migration.h" # include "qemu/qemu_process.h" +# include "qemu/qemu_slirp.h" # include "datatypes.h" # include "conf/storage_conf.h" # include "cpu/cpu_map.h" @@ -398,6 +399,7 @@ testCheckExclusiveFlags(int flags) FLAG_FIPS | FLAG_REAL_CAPS | FLAG_SKIP_LEGACY_CPUS | + FLAG_SLIRP_HELPER | 0, -1); VIR_EXCLUSIVE_FLAGS_RET(FLAG_REAL_CAPS, FLAG_SKIP_LEGACY_CPUS, -1); @@ -526,6 +528,19 @@ testCompareXMLToArgv(const void *data) } } + if (flags & FLAG_SLIRP_HELPER) { + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_NET_SOCKET_DGRAM)) { + qemuSlirpPtr slirp = qemuSlirpNew(); + slirp->fd[0] = 42; + QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp; + } + } + } + if (!(cmd = qemuProcessCreatePretendCmd(&driver, vm, migrateURI, (flags & FLAG_FIPS), false, VIR_QEMU_PROCESS_START_COLD))) { @@ -1280,6 +1295,7 @@ mymain(void) DO_TEST_FAILURE("net-vhostuser-fail", QEMU_CAPS_VHOSTUSER_MULTIQUEUE); DO_TEST("net-user", NONE); + DO_TEST_CAPS_ARCH_VER_FULL("net-user", "x86_64", "4.0.0", ARG_FLAGS, FLAG_SLIRP_HELPER); DO_TEST("net-user-addr", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 0632141d68..a2f7bfcc26 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -51,6 +51,7 @@ typedef enum { FLAG_FIPS = 1 << 2, FLAG_REAL_CAPS = 1 << 3, FLAG_SKIP_LEGACY_CPUS = 1 << 4, + FLAG_SLIRP_HELPER = 1 << 5, } testQemuInfoFlags; struct testQemuInfo { -- 2.23.0.rc1

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../net-user.x86_64-4.0.0.args | 34 +++++++++++++++++++ tests/qemuxml2argvtest.c | 16 +++++++++ tests/testutilsqemu.h | 1 + 3 files changed, 51 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args
I needed to regenerate the .args file so that it is wrapped correctly, but that's a minor issue. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On 8/8/19 4:54 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
SLIRP networking can be running in a separate process. This allows for stricter security policies for QEMU & SLIRP, as SLIRP is notoriously not very safe (discussed on ML, various CVEs, and even the code says so explicitly in the comments), yet people rely on it for various reasons.
With this series, for a network interface "user", libvirt will: - check the slirp-helper presence and capabilites (see [1]) - setup a socket pair between qemu and the helper - use -net socket - setup migration thanks to dbus-vmstate
There are no changes required to domain configuration to benefit it. "guestfwd" isn't supported at this point, but it is known to be in a broken state with libvirt+qemu anyway.
The dbus-vmstate is being proposed to QEMU.
The libslirp-rs slirp-helper hasn't yet received a release. The current DBus p2p mode works ok, but is a hack. This is due to poor DBus support in Rust, and also relatively poor DBus p2p mode support in libdbus.
fwiw, I have been working on an alternative rust-only implementation of a slirp-helper that will also follow [1], but I am now wondering if netstack or vpnkit could do the job.
[1] https://gitlab.freedesktop.org/slirp/libslirp-rs/blob/master/src/bin/README....
Marc-André Lureau (23): Add .editorconfig tests: fix xml2xml tpm-emulator.xml test dbus: correctly build reply message qemu: replace logCtxt with qemuDomainLogAppendMessage() qemu: add socket datagram capability qemu: add dbus-vmstate capability qemu: reset VM id after external devices stop qemu-security: add qemuSecurityCommandRun() qemu: add dbus-vmstate domain-conf: add network def private data qemu: add qemuDomainNetworkPrivate qemu-conf: add configurable slirp-helper location qemu-conf: add slirp state dir qemu: add slirp helper unit qemu-domain: save and restore slirp state qemu: add a flag to the cookie to prevent slirp-helper setup qemu-migration: prevent migration if dbus-vmstate is required qemu-migration: prevent migration if slirp cannot be migrated qemu-extdevice: prepare, start and stop slirp-helper qemu-command: use -net socket,fd= with slirp-helper qemu-process: prepare slirp-helper qemu-hotplug: handle hotplugging of slirp-helper tests: add slirp-helper qemuxml2argv test
.editorconfig | 21 + m4/virt-driver-qemu.m4 | 5 + src/conf/domain_conf.c | 21 +- src/conf/domain_conf.h | 6 + src/qemu/Makefile.inc.am | 4 + 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_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 118 ++++- src/qemu/qemu_command.h | 6 +- src/qemu/qemu_conf.c | 11 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 94 ++++ src/qemu/qemu_dbus.h | 42 ++ src/qemu/qemu_domain.c | 220 ++++++++- src/qemu/qemu_domain.h | 20 + src/qemu/qemu_driver.c | 8 + src/qemu/qemu_extdevice.c | 82 ++-- src/qemu/qemu_extdevice.h | 10 +- src/qemu/qemu_hotplug.c | 112 ++++- src/qemu/qemu_hotplug.h | 11 + src/qemu/qemu_interface.c | 27 ++ src/qemu/qemu_interface.h | 4 + src/qemu/qemu_migration.c | 19 + src/qemu/qemu_monitor.c | 13 +- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_process.c | 24 +- src/qemu/qemu_security.c | 22 + src/qemu/qemu_security.h | 6 + src/qemu/qemu_slirp.c | 448 ++++++++++++++++++ src/qemu/qemu_slirp.h | 81 ++++ src/qemu/qemu_tpm.c | 4 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/virdbus.c | 18 +- src/util/virdbus.h | 6 +- .../caps_4.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml | 1 + .../caps_4.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../net-user.x86_64-4.0.0.args | 34 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 2 +- tests/qemuxml2argvtest.c | 16 + tests/testutilsqemu.h | 1 + tests/virfirewalltest.c | 9 +- tests/virpolkittest.c | 3 +- 51 files changed, 1498 insertions(+), 77 deletions(-) create mode 100644 .editorconfig create mode 100644 src/qemu/qemu_dbus.c create mode 100644 src/qemu/qemu_dbus.h create mode 100644 src/qemu/qemu_slirp.c create mode 100644 src/qemu/qemu_slirp.h create mode 100644 tests/qemuxml2argvdata/net-user.x86_64-4.0.0.args
I've made all the changes I'm suggesting, ACKed and pushed. Sorry for taking it so long to review. One thing though, this deserves a release note. Can you cook something please? See docs/news.xml for more info. Michal
participants (3)
-
John Ferlan
-
marcandre.lureau@redhat.com
-
Michal Privoznik