- Devel - libvirt List Archives

[PATCH 0/2] lxc: Implement lxcDomainInterfaceAddresses() function
by Julio Faracco 23 Jan '20

23 Jan '20

It is not able to retrieve IP addresses from LXC domains. This method was not implemented before. LXC uses the same operations to retrieve this information, but QEMU has their own functions to process this data. This series turn them into global functions, moving them into domain conf methods, to make them available for other drivers. The only source that LXC does not support is agent which is a specific source from QEMU. Julio Faracco (2): qemu: Converting DHCP and ARP functions to domain conf lxc: Add support to lxcDomainInterfaceAddresses() function src/conf/domain_conf.c | 134 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 2 + src/lxc/lxc_driver.c | 44 +++++++++++++ src/qemu/qemu_driver.c | 137 +-------------------------------------- 5 files changed, 184 insertions(+), 135 deletions(-) -- 2.20.1

2 5

[libvirt] [PATCH] nodedev: use autofree for more local variables
by Jonathon Jongsma 23 Jan '20

23 Jan '20

Simplify function logic by using g_autofree to free local variables so that we can remove some goto statements that are used for cleanup. Introduce a g_autoptr cleanup function for virNodeDeviceDef. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/conf/node_device_conf.h | 2 ++ src/node_device/node_device_driver.c | 47 +++++++++++----------------- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index bf7939fbb3..4ff616ee90 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -344,6 +344,8 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, void virNodeDeviceDefFree(virNodeDeviceDefPtr def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDeviceDef, virNodeDeviceDefFree); + void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps); diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b630be4399..fda77ede24 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -106,10 +106,9 @@ int nodeConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) static int nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def) { - char *driver_link = NULL; - char *devpath = NULL; + g_autofree char *driver_link = NULL; + g_autofree char *devpath = NULL; char *p; - int ret = -1; VIR_FREE(def->driver); @@ -117,26 +116,20 @@ nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def) /* Some devices don't have an explicit driver, so just return without a name */ - if (access(driver_link, R_OK) < 0) { - ret = 0; - goto cleanup; - } + if (access(driver_link, R_OK) < 0) + return 0; if (virFileResolveLink(driver_link, &devpath) < 0) { virReportSystemError(errno, _("cannot resolve driver link %s"), driver_link); - goto cleanup; + return -1; } p = strrchr(devpath, '/'); if (p) def->driver = g_strdup(p + 1); - ret = 0; - cleanup: - VIR_FREE(driver_link); - VIR_FREE(devpath); - return ret; + return 0; } #else /* XXX: Implement me for non-linux */ @@ -468,8 +461,9 @@ nodeDeviceCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { - virNodeDeviceDefPtr def = NULL; - char *wwnn = NULL, *wwpn = NULL; + g_autoptr(virNodeDeviceDef) def = NULL; + g_autofree char *wwnn = NULL; + g_autofree char *wwpn = NULL; int parent_host = -1; virNodeDevicePtr device = NULL; const char *virt_type = NULL; @@ -478,19 +472,19 @@ nodeDeviceCreateXML(virConnectPtr conn, virt_type = virConnectGetType(conn); if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type))) - goto cleanup; + return NULL; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) - goto cleanup; + return NULL; if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) - goto cleanup; + return NULL; if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def)) < 0) - goto cleanup; + return NULL; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) - goto cleanup; + return NULL; device = nodeDeviceFindNewDevice(conn, wwnn, wwpn); /* We don't check the return value, because one way or another, @@ -501,10 +495,7 @@ nodeDeviceCreateXML(virConnectPtr conn, _("no node device for '%s' with matching " "wwnn '%s' and wwpn '%s'"), def->name, wwnn, wwpn); - cleanup: - virNodeDeviceDefFree(def); - VIR_FREE(wwnn); - VIR_FREE(wwpn); + return device; } @@ -515,8 +506,9 @@ nodeDeviceDestroy(virNodeDevicePtr device) int ret = -1; virNodeDeviceObjPtr obj = NULL; virNodeDeviceDefPtr def; - char *parent = NULL; - char *wwnn = NULL, *wwpn = NULL; + g_autofree char *parent = NULL; + g_autofree char *wwnn = NULL; + g_autofree char *wwpn = NULL; unsigned int parent_host; if (!(obj = nodeDeviceObjFindByName(device->name))) @@ -554,9 +546,6 @@ nodeDeviceDestroy(virNodeDevicePtr device) cleanup: virNodeDeviceObjEndAPI(&obj); - VIR_FREE(parent); - VIR_FREE(wwnn); - VIR_FREE(wwpn); return ret; } -- 2.21.0

2 1

[libvirt PATCH] src: remove WITH_GNUTLS usage
by Pavel Hrdina 22 Jan '20

22 Jan '20

Since commit <60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24> we require GnuTLS so it doesn't make sense to ifdef the code. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/libvirt.c | 6 +----- src/remote/remote_driver.c | 19 ------------------- src/rpc/virnetclient.c | 20 ++++---------------- src/rpc/virnetclient.h | 8 +------- src/rpc/virnetsocket.c | 16 ---------------- src/rpc/virnetsocket.h | 6 +----- src/util/virrandom.c | 36 ++---------------------------------- 7 files changed, 9 insertions(+), 102 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index f1ffc97261..86bb6551ed 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -45,9 +45,7 @@ #include "viralloc.h" #include "configmake.h" #include "virconf.h" -#if WITH_GNUTLS -# include "rpc/virnettlscontext.h" -#endif +#include "rpc/virnettlscontext.h" #include "vircommand.h" #include "virfile.h" #include "virrandom.h" @@ -233,9 +231,7 @@ virGlobalInit(void) virLogSetFromEnv(); -#ifdef WITH_GNUTLS virNetTLSInit(); -#endif #if WITH_CURL curl_global_init(CURL_GLOBAL_DEFAULT); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 66472a6cc1..f6e725dcbf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -124,9 +124,7 @@ struct private_data { int counter; /* Serial number for RPC */ -#ifdef WITH_GNUTLS virNetTLSContextPtr tls; -#endif int is_secure; /* Secure if TLS or SASL or UNIX sockets */ char *type; /* Cached return from remoteType. */ @@ -1132,7 +1130,6 @@ doRemoteOpen(virConnectPtr conn, virConfGetValueString(conf, "tls_priority", &tls_priority) < 0) goto failed; -#ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false, tls_priority, @@ -1141,27 +1138,17 @@ doRemoteOpen(virConnectPtr conn, goto failed; priv->is_secure = 1; G_GNUC_FALLTHROUGH; -#else - (void)tls_priority; - (void)sanity; - (void)verify; - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("GNUTLS support not available in this build")); - goto failed; -#endif case REMOTE_DRIVER_TRANSPORT_TCP: priv->client = virNetClientNewTCP(priv->hostname, port, AF_UNSPEC); if (!priv->client) goto failed; -#ifdef WITH_GNUTLS if (priv->tls) { VIR_DEBUG("Starting TLS session"); if (virNetClientSetTLSSession(priv->client, priv->tls) < 0) goto failed; } -#endif break; @@ -1388,10 +1375,8 @@ doRemoteOpen(virConnectPtr conn, priv->client = NULL; virObjectUnref(priv->closeCallback); priv->closeCallback = NULL; -#ifdef WITH_GNUTLS virObjectUnref(priv->tls); priv->tls = NULL; -#endif VIR_FREE(priv->hostname); return VIR_DRV_OPEN_ERROR; @@ -1533,10 +1518,8 @@ doRemoteClose(virConnectPtr conn, struct private_data *priv) (xdrproc_t) xdr_void, (char *) NULL) == -1) ret = -1; -#ifdef WITH_GNUTLS virObjectUnref(priv->tls); priv->tls = NULL; -#endif virNetClientSetCloseCallback(priv->client, NULL, @@ -4271,7 +4254,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, /* saslcb is now owned by sasl */ saslcb = NULL; -# ifdef WITH_GNUTLS /* Initialize some connection props we care about */ if (priv->tls) { if ((ssf = virNetClientGetTLSKeySize(priv->client)) < 0) @@ -4283,7 +4265,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, if (virNetSASLSessionExtKeySize(sasl, ssf) < 0) goto cleanup; } -# endif /* If we've got a secure channel (TLS or UNIX sock), we don't care about SSF */ /* If we're not secure, then forbid any anonymous or trivially crackable auth */ diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 50489b754c..000b937cdd 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -70,9 +70,7 @@ struct _virNetClient { virNetSocketPtr sock; bool asyncIO; -#if WITH_GNUTLS virNetTLSSessionPtr tls; -#endif char *hostname; virNetClientProgramPtr *programs; @@ -708,9 +706,7 @@ void virNetClientDispose(void *obj) if (client->sock) virNetSocketRemoveIOCallback(client->sock); virObjectUnref(client->sock); -#if WITH_GNUTLS virObjectUnref(client->tls); -#endif #if WITH_SASL virObjectUnref(client->sasl); #endif @@ -750,10 +746,8 @@ virNetClientCloseLocked(virNetClientPtr client) virObjectUnref(client->sock); client->sock = NULL; -#if WITH_GNUTLS virObjectUnref(client->tls); client->tls = NULL; -#endif #if WITH_SASL virObjectUnref(client->sasl); client->sasl = NULL; @@ -837,7 +831,6 @@ void virNetClientSetSASLSession(virNetClientPtr client, #endif -#if WITH_GNUTLS int virNetClientSetTLSSession(virNetClientPtr client, virNetTLSContextPtr tls) { @@ -848,12 +841,12 @@ int virNetClientSetTLSSession(virNetClientPtr client, sigset_t oldmask, blockedsigs; sigemptyset(&blockedsigs); -# ifdef SIGWINCH +#ifdef SIGWINCH sigaddset(&blockedsigs, SIGWINCH); -# endif -# ifdef SIGCHLD +#endif +#ifdef SIGCHLD sigaddset(&blockedsigs, SIGCHLD); -# endif +#endif sigaddset(&blockedsigs, SIGPIPE); virObjectLock(client); @@ -940,16 +933,13 @@ int virNetClientSetTLSSession(virNetClientPtr client, virObjectUnlock(client); return -1; } -#endif bool virNetClientIsEncrypted(virNetClientPtr client) { bool ret = false; virObjectLock(client); -#if WITH_GNUTLS if (client->tls) ret = true; -#endif #if WITH_SASL if (client->sasl) ret = true; @@ -1041,7 +1031,6 @@ const char *virNetClientRemoteAddrStringSASL(virNetClientPtr client) return virNetSocketRemoteAddrStringSASL(client->sock); } -#if WITH_GNUTLS int virNetClientGetTLSKeySize(virNetClientPtr client) { int ret = 0; @@ -1051,7 +1040,6 @@ int virNetClientGetTLSKeySize(virNetClientPtr client) virObjectUnlock(client); return ret; } -#endif static int virNetClientCallDispatchReply(virNetClientPtr client) diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 07b399c1a9..778910b575 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -20,9 +20,7 @@ #pragma once -#ifdef WITH_GNUTLS -# include "virnettlscontext.h" -#endif +#include "virnettlscontext.h" #include "virnetmessage.h" #ifdef WITH_SASL # include "virnetsaslcontext.h" @@ -120,10 +118,8 @@ void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl); #endif -#ifdef WITH_GNUTLS int virNetClientSetTLSSession(virNetClientPtr client, virNetTLSContextPtr tls); -#endif bool virNetClientIsEncrypted(virNetClientPtr client); bool virNetClientIsOpen(virNetClientPtr client); @@ -131,9 +127,7 @@ bool virNetClientIsOpen(virNetClientPtr client); const char *virNetClientLocalAddrStringSASL(virNetClientPtr client); const char *virNetClientRemoteAddrStringSASL(virNetClientPtr client); -#ifdef WITH_GNUTLS int virNetClientGetTLSKeySize(virNetClientPtr client); -#endif void virNetClientClose(virNetClientPtr client); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 973827ebde..23384e5250 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -93,9 +93,7 @@ struct _virNetSocket { char *remoteAddrStrSASL; char *remoteAddrStrURI; -#if WITH_GNUTLS virNetTLSSessionPtr tlsSession; -#endif #if WITH_SASL virNetSASLSessionPtr saslSession; @@ -1288,13 +1286,11 @@ virJSONValuePtr virNetSocketPreExecRestart(virNetSocketPtr sock) goto error; } #endif -#if WITH_GNUTLS if (sock->tlsSession) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Unable to save socket state when TLS session is active")); goto error; } -#endif if (!(object = virJSONValueNewObject())) goto error; @@ -1358,12 +1354,10 @@ void virNetSocketDispose(void *obj) unlink(sock->localAddr.data.un.sun_path); #endif -#if WITH_GNUTLS /* Make sure it can't send any more I/O during shutdown */ if (sock->tlsSession) virNetTLSSessionSetIOCallbacks(sock->tlsSession, NULL, NULL, NULL); virObjectUnref(sock->tlsSession); -#endif #if WITH_SASL virObjectUnref(sock->saslSession); #endif @@ -1660,7 +1654,6 @@ const char *virNetSocketRemoteAddrStringURI(virNetSocketPtr sock) return sock->remoteAddrStrURI; } -#if WITH_GNUTLS static ssize_t virNetSocketTLSSessionWrite(const char *buf, size_t len, void *opaque) @@ -1691,7 +1684,6 @@ void virNetSocketSetTLSSession(virNetSocketPtr sock, sock); virObjectUnlock(sock); } -#endif #if WITH_SASL void virNetSocketSetSASLSession(virNetSocketPtr sock, @@ -1789,17 +1781,13 @@ static ssize_t virNetSocketReadWire(virNetSocketPtr sock, char *buf, size_t len) #endif reread: -#if WITH_GNUTLS if (sock->tlsSession && virNetTLSSessionGetHandshakeStatus(sock->tlsSession) == VIR_NET_TLS_HANDSHAKE_COMPLETE) { ret = virNetTLSSessionRead(sock->tlsSession, buf, len); } else { -#endif ret = read(sock->fd, buf, len); -#if WITH_GNUTLS } -#endif if ((ret < 0) && (errno == EINTR)) goto reread; @@ -1862,17 +1850,13 @@ static ssize_t virNetSocketWriteWire(virNetSocketPtr sock, const char *buf, size #endif rewrite: -#if WITH_GNUTLS if (sock->tlsSession && virNetTLSSessionGetHandshakeStatus(sock->tlsSession) == VIR_NET_TLS_HANDSHAKE_COMPLETE) { ret = virNetTLSSessionWrite(sock->tlsSession, buf, len); } else { -#endif ret = write(sock->fd, buf, len); -#if WITH_GNUTLS } -#endif if (ret < 0) { if (errno == EINTR) diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 8d075464cb..f2b74f3ccb 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -23,9 +23,7 @@ #include "virsocketaddr.h" #include "vircommand.h" -#ifdef WITH_GNUTLS -# include "virnettlscontext.h" -#endif +#include "virnettlscontext.h" #include "virobject.h" #ifdef WITH_SASL # include "virnetsaslcontext.h" @@ -152,10 +150,8 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len); int virNetSocketSendFD(virNetSocketPtr sock, int fd); int virNetSocketRecvFD(virNetSocketPtr sock, int *fd); -#ifdef WITH_GNUTLS void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess); -#endif #ifdef WITH_SASL void virNetSocketSetSASLSession(virNetSocketPtr sock, diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 39ac36a76e..eae7f3db3c 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -24,10 +24,8 @@ #include <fcntl.h> #include <sys/stat.h> #include <sys/types.h> -#ifdef WITH_GNUTLS -# include <gnutls/gnutls.h> -# include <gnutls/crypto.h> -#endif +#include <gnutls/gnutls.h> +#include <gnutls/crypto.h> #include "virrandom.h" #include "virthread.h" @@ -116,7 +114,6 @@ int virRandomBytes(unsigned char *buf, size_t buflen) { -#if WITH_GNUTLS int rv; /* Generate the byte stream using gnutls_rnd() if possible */ @@ -127,35 +124,6 @@ virRandomBytes(unsigned char *buf, return -1; } -#else /* !WITH_GNUTLS */ - - int fd; - - if ((fd = open(RANDOM_SOURCE, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("unable to open %s"), - RANDOM_SOURCE); - return -1; - } - - while (buflen > 0) { - ssize_t n; - - if ((n = saferead(fd, buf, buflen)) <= 0) { - virReportSystemError(errno, - _("unable to read from %s"), - RANDOM_SOURCE); - VIR_FORCE_CLOSE(fd); - return n < 0 ? -errno : -ENODATA; - } - - buf += n; - buflen -= n; - } - - VIR_FORCE_CLOSE(fd); -#endif /* !WITH_GNUTLS */ - return 0; } -- 2.24.1

3 2

[libvirt] [PATCH v2 0/2] fix segfaults on qemuhotplugtest
by Christian Ehrhardt 22 Jan '20

22 Jan '20

After debugging for a while and eventually finding the root cause we discussed this on IRC today: <cpaelzer> danpb: BTW the test crash in 6.0 that I'm tracing (asked yesterday) is virHostdevManagerInitialize failing <cpaelzer> the test goes on with mgr set to 0x0 and that is the segfault later on <cpaelzer> virSetError was helpful as "Could not initialize HostdevManager operation failed: Failed to create state dir '/sbuild-nonexistent/.cache/libvirt/hostdevmgr'" sounds like a very testbed-specific issue that I might be able to solve when invoking the tests <danpb> hmm, seems like we're missing a mock for mkdir() <danpb> i guess we should mock virFileMakePath in this case This series got v6.0.0 building in Ubuntu, so I thought it is worth to submit it for review. Updates in v2: - improve indent of virGetLastErrorMessage - drop superfluous curly brackets - added the Reviewed-by of Daniel Henrique Barboza (Thanks !) Christian Ehrhardt (2): test: let qemuhotplugtest report details of init fails test: qemuhotplugtest mock virFileMakePath tests/qemuhotplugmock.c | 17 +++++++++++++++++ tests/qemuhotplugtest.c | 5 +++++ 2 files changed, 22 insertions(+) -- 2.25.0

2 6

[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 21 Jan '20

21 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 21 Jan '20

21 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 21 Jan '20

21 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