January 2020 - Devel - libvirt List Archives

[PATCH] libxl: initialize shutdown inhibit callback
by Marek Marczykowski-Górecki 22 Jan '20

22 Jan '20

The libxl driver already tries to call shutdown inhibit callback in the right places, but only if it's set. That last part was missing, resulting in premature shutdown when running libvirtd --timeout=... Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com> --- src/libxl/libxl_driver.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bece313ec5..d45e42c100 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -648,8 +648,8 @@ libxlAddDom0(libxlDriverPrivatePtr driver) static int libxlStateInitialize(bool privileged, - virStateInhibitCallback callback G_GNUC_UNUSED, - void *opaque G_GNUC_UNUSED) + virStateInhibitCallback callback, + void *opaque) { libxlDriverConfigPtr cfg; char *driverConf = NULL; @@ -670,6 +670,9 @@ libxlStateInitialize(bool privileged, return VIR_DRV_STATE_INIT_ERROR; } + libxl_driver->inhibitCallback = callback; + libxl_driver->inhibitOpaque = opaque; + /* Allocate bitmap for vnc port reservation */ if (!(libxl_driver->reservedGraphicsPorts = virPortAllocatorRangeNew(_("VNC"), -- 2.21.0

2 1

[libvirt-tck PATCH 0/2] Convert to SSH pubkey auth rather than password-based auth
by Erik Skultety 22 Jan '20

22 Jan '20

Most of the nwfilter tests utilize SSH connections to execute some commands to cross reference whether the requested change in libvirt took effect. However, fedora 31 disables password-based auth for root login which breaks the test suite. Erik Skultety (2): lib: TCK.pm: Favour pubkey auth over passwords on SSH connections nwfilter: Make use of the SSH pubkey auth rather than password-based auth lib/Sys/Virt/TCK.pm | 30 ++++++++++++++++++++++++- scripts/nwfilter/210-no-mac-spoofing.t | 2 +- scripts/nwfilter/220-no-ip-spoofing.t | 2 +- scripts/nwfilter/230-no-mac-broadcast.t | 2 +- scripts/nwfilter/240-no-arp-spoofing.t | 2 +- 5 files changed, 33 insertions(+), 5 deletions(-) -- 2.24.1

2 8

[libvirt] [tck PATCH 0/3] A few network related fixes to get the network suite running
by Erik Skultety 22 Jan '20

22 Jan '20

Erik Skultety (3): network: Fix the iptables FORWARD chain name being queried network: Fix the dhcp range output being matched nwfilter: Fix the expected output from ebtables .../networks/networkxml2hostout/tck-testnet-1.dat | 5 +++-- .../networks/networkxml2hostout/tck-testnet-2.dat | 5 +++-- .../networks/networkxml2hostout/tck-testnet-3.dat | 14 ++++++++------ .../nwfilter/nwfilterxml2fwallout/ipv6-test.fwall | 12 ++++++------ 4 files changed, 20 insertions(+), 16 deletions(-) -- 2.24.1

2 7

[libvirt] [PATCH] create a thread to handle MigrationParamReset to avoid deadlock
by Yi Wang 22 Jan '20

22 Jan '20

From: Li XueLei <li.xuelei1(a)zte.com.cn> Libvirtd no longer receives external requests, after I do the following. Steps to reproduce: 1.Virsh tool initiates a non-p2p migrations. 2.After the phase of qemuDomainMigratePerform3 and before the phase of qemuDomainMigrateConfirm3, kill the virsh client which initiated the migration. What happens: Libvirtd will be blocked in the next call stack because it can't get the condition variables, and it will not be able to receive new requests. Thread 1 (Thread 0x7f36a7b9f8c0 (LWP 27556)): #0 0x00007f36a45799f5 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007f36a6e7a97e in virCondWait () from /lib64/libvirt.so.0 #2 0x00007f3679c6a1b3 in qemuMonitorSend () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #3 0x00007f3679c837de in qemuMonitorJSONCommandWithFd () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #4 0x00007f3679c93c5a in qemuMonitorJSONSetMigrationCapabilities () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #5 0x00007f3679c786ed in qemuMonitorSetMigrationCapabilities () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #6 0x00007f3679c67857 in qemuMigrationParamsApply () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #7 0x00007f3679c67eff in qemuMigrationParamsReset () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #8 0x00007f3679c5198a in qemuMigrationSrcCleanup () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #9 0x00007f36a6dcd862 in virCloseCallbacksRun () from /lib64/libvirt.so.0 #10 0x00007f3679cc28f6 in qemuConnectClose () from /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so #11 0x00007f36a70a0870 in virConnectDispose () from /lib64/libvirt.so.0 #12 0x00007f36a6e3f43b in virObjectUnref () from /lib64/libvirt.so.0 #13 0x00007f36a70a55c4 in virConnectClose () from /lib64/libvirt.so.0 #14 0x000056058a206f92 in remoteClientFree () #15 0x00007f36a6fa49a0 in virNetServerClientDispose () from /lib64/libvirt.so.0 #16 0x00007f36a6e3f43b in virObjectUnref () from /lib64/libvirt.so.0 #17 0x00007f36a6e3fd21 in virObjectFreeCallback () from /lib64/libvirt.so.0 #18 0x00007f36a6f9942e in virNetSocketEventFree () from /lib64/libvirt.so.0 #19 0x00007f36a6de8439 in virEventPollCleanupHandles () from /lib64/libvirt.so.0 #20 0x00007f36a6de9ab6 in virEventPollRunOnce () from /lib64/libvirt.so.0 #21 0x00007f36a6de817a in virEventRunDefaultImpl () from /lib64/libvirt.so.0 #22 0x00007f36a6faadb5 in virNetDaemonRun () from /lib64/libvirt.so.0 #23 0x000056058a1c5cc1 in main () Here's a patch to solve this bug, that is to create a thread to do MigrationParamReset. Signed-off-by: Li XueLei <li.xuelei1(a)zte.com.cn> Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn> --- src/remote/remote_daemon_dispatch.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 76c676c..3962ee3 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1741,9 +1741,10 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv) * We keep the libvirt connection open until any async * jobs have finished, then clean it up elsewhere */ -void remoteClientFree(void *data) + +static void remoteClientFreeFun(void *opaque) { - struct daemonClientPrivate *priv = data; + struct daemonClientPrivate *priv = opaque; if (priv->conn) virConnectClose(priv->conn); @@ -1760,7 +1761,16 @@ void remoteClientFree(void *data) if (priv->storageConn) virConnectClose(priv->storageConn); - VIR_FREE(priv); + VIR_FREE(priv); +} + +void remoteClientFree(void *data) +{ + virThread thread; + + if (virThreadCreate(&thread, false, remoteClientFreeFun, data) < 0) { + VIR_ERROR("Unable to create remoteClientFreeFun thread"); + } } -- 1.8.3.1

4 6

RFC - (re)design of PCI multifunction hot-unplug
by Daniel Henrique Barboza 21 Jan '20

21 Jan '20

Hi, This is a request for comments in the design of the PCI multifunction hotplug/hot-unplug feature for the QEMU driver that hopefully I'll be sending shortly for review. The feature went through code changes since [1] mostly because of Libvirt changes itself, but Shiva's 2016 original design [2] was preserved. The design consists of a new 'devices' XML element that will contain all hostdevs of the multifunction device to be hotplugged: <devices> --- hostdev function 0 XML --- --- hostdev function 1 XML --- (...) --- hostdev function N XML --- </devices> The only requirement is function 0 to be present. We'll not force all functions to be hotplugged. Internally, Libvirt will hotplug each non-zero function first, then the zero function for last, which is what QEMU expects for both x86 and Pseries (the 2 archs that supports this feature AFAIK). For unplug, the same <devices> XML is required. And this is where the archs starts to behave differently. x86 guests will hot-unplug all functions, regardless of which function is unplugged first. In the example above, hot-unplugging function 1 will trigger all functions to be unplugged. Pseries does not work like that - all non-zero functions need to be unplugged, then the function zero unplug finally triggers the unplug of the slot. And the Libvirt side was doing exactly that. Back in 2019 I interpreted this as a case of a x86 feature paying the price for a Pseries behavior - if Pseries hot-unplug worked the same way, we could hot-unplug all functions with a single call like x86 does, and the user won't need the <devices> XML for hot-unplug. I ended up tweaking the Pseries guest in QEMU to execute hot-unplug of all functions in case function 0 is hot-unplugged, emulating the x86 behavior for function 0 [3]. This change alone already eases the unplug code in Libvirt side for Pseries, which is good. But my idea was to not use the <devices> XML for hot-unplug. Instead, use regular hot-unplug of function 0 to hot-unplug the whole slot, for both archs. However, recent discussions when contributing the new address='unassigned' type made me re-think the decision I made above: - using <devices> XML for hot-unplug isn't too much of a deal, given that the user would need it for hot-plugging the slot anyway. The gain is marginal, at best. - making a single hot-unplug command 'magically' unplug more than one hostdev is not a good idea. First, there's always the chance that the user hot-unplugs the function 0 of a multifunction device, thinking that it's a regular hostdev, just to see whole slot disappearing from the guest. Granted, I can put more code in this case, warning the user about the removal of the whole slot instead of a single hostdev. But in the end, this breaks Laine's golden rule [4] of do not tamper with devices unless the user is explicitly telling to do so, which aims to avoid these cases of devices appearing/disappearing without user consent. All this said, what I'm leaning up to do is to keep Shiva's design, but simplifying the hot-unplug mechanic considering the changes I made in QEMU for Pseries guests. I'm not adamant on it though, so I'd like to hear if I'm missing something and I should go the 'single unplug removes it all' route instead. Thanks, DHB [1] https://www.redhat.com/archives/libvir-list/2018-March/msg00729.html [2] https://www.redhat.com/archives/libvir-list/2016-April/msg01057.html [3] https://lists.gnu.org/archive/html/qemu-ppc/2019-08/msg00447.html [4] this is something Laine talks about here and there in threads I happen to be involved. If someone wants to claim authorship let me know

1 0

[libvirt] [PATCH 0/4] virsh: secret: Improve handling of secret value
by Peter Krempa 21 Jan '20

21 Jan '20

The currently existing virsh APIs for secrets are awful for human use and don't promote security. Peter Krempa (4): virsh: secret: Add 'secret-passwd' command virsh: secret: Allow getting secret's value without base64 encoding virsh: secret: Allow setting secrets from file docs: secret: Unify and sanitize examples on how to set secret value docs/formatsecret.html.in | 86 ++++++++++++++++++---------- docs/manpages/virsh.rst | 26 ++++++++- tools/virsh-secret.c | 116 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 189 insertions(+), 39 deletions(-) -- 2.24.1

4 16

[libvirt] [PATCH 0/2] implement crashloaded event for pvpanic
by zhenwei pi 21 Jan '20

21 Jan '20

Guest may handle panic by itself, then just reboot without pvpanic notification. Then We can't separate the abnormal reboot from normal operation. Declear bit 1 for pvpanic as crashloaded event. It should work with guest kernel side. Link: https://lkml.org/lkml/2019/12/14/265 Before running kexec, guest could wirte this bit to notify host side. Host side handles crashloaded event, posts event to upper layer. Then guest side continues to run kexec. Test with libvirt, libvirt could recieve the new event. The patch of libvirt will be sent soon. Zhenwei Pi (2): pvpanic: introduce crashloaded for pvpanic pvpanic: implement crashloaded event handling docs/specs/pvpanic.txt | 8 ++++++-- hw/misc/pvpanic.c | 11 +++++++++-- include/sysemu/runstate.h | 1 + qapi/run-state.json | 22 +++++++++++++++++++++- vl.c | 12 ++++++++++++ 5 files changed, 49 insertions(+), 5 deletions(-) -- 2.11.0

3 6

[libvirt] [PATCH v4 0/7] Fix virConnectRegisterCloseCallback and get rid of global variables
by Marc Hartmayer 21 Jan '20

21 Jan '20

The second patch of this patch series fixes the behavior of virConnectRegisterCloseCallback. The subsequent patches remove the need to have the global variables 'qemuProgram' and 'remoteProgram' in libvirtd.[ch]. They only work in combination with the fixed behavior of virConnectRegisterCloseCallback. Changelog: + v3->v4: - Rebased to current master - Added Pavel's r-bs - Worked in Pavel's comments - Added patch "remote: shrink the critical sections" in preparation for patch "remote/rpc: Use virNetServerGetProgram() to determine the program" + v2->v3: - Rebased to current master - Added Johns r-b to the first patch, all other r-b's I have dropped as there were to many changes in the meantime - Removed accepted patches - Dropped patches 8 and 9 + v1->v2: - Removed accepted patches - Removed NACKed patches - Added r-b to patch 5 - Worked in comments - Rebased - Added patches 7-9 Marc Hartmayer (7): rpc: use the return value of virObjectRef directly virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback remote: Save reference to program in daemonClientEventCallback remote: Use domainClientEventCallbacks for remoteReplayConnectionClosedEvent rpc: Introduce virNetServerGetProgramLocked helper function remote: shrink the critical sections remote/rpc: Use virNetServerGetProgram() to determine the program src/libvirt-host.c | 16 +- src/libvirt_remote.syms | 1 + src/remote/remote_daemon.c | 4 +- src/remote/remote_daemon.h | 2 - src/remote/remote_daemon_dispatch.c | 363 +++++++++++++++++----------- src/rpc/gendispatch.pl | 6 + src/rpc/virnetserver.c | 53 +++- src/rpc/virnetserver.h | 2 + 8 files changed, 293 insertions(+), 154 deletions(-) -- 2.21.0

3 18

[PATCH 0/2] Simple fixes for run.in
by Richard W.M. Jones 21 Jan '20

21 Jan '20

The ./run script is a very useful way to run the libguestfs and virt-v2v test suites against a locally compiled copy of libvirt. These two patches contain some minor fixes. Rich.

3 6

[PATCH] Fix syntax-check 'always true header test' for virvsock.c
by Daniel Henrique Barboza 21 Jan '20

21 Jan '20

Commit 05a38d4c4a ("src: conditionalize / remove use of sys/ioctl.h") aimed to remove imports of ioctl.h and conditionalize them for compilation on Windows. However, the change made in virvsock.c: -#include <sys/ioctl.h> +#ifdef HAVE_SYS_IOCTL_H +# include <sys/ioctl.h> +#endif Makes the 'sc_prohibit_always_true_header_tests' rule complain about the redundancy of this test when building it on Linux. ../src/util/virvsock.c:20:#ifdef HAVE_SYS_IOCTL_H build-aux/syntax-check.mk: do not test the above HAVE_<header>_H symbol(s); with the corresponding gnulib module, they are always true make: *** [../build-aux/syntax-check.mk:1679: sc_prohibit_always_true_header_tests] Error 1 make: *** Waiting for unfinished jobs.... This patch fixes it by changing the conditional to #ifndef WIN32 instead, like the other changes made by that patch. CC: Daniel P. Berrangé <berrange(a)redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com> --- src/util/virvsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virvsock.c b/src/util/virvsock.c index 2638c5095a..1e2eb553d2 100644 --- a/src/util/virvsock.c +++ b/src/util/virvsock.c @@ -17,7 +17,7 @@ #include <config.h> -#ifdef HAVE_SYS_IOCTL_H +#ifndef WIN32 # include <sys/ioctl.h> #endif -- 2.24.1

1 1