- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Fix migration without parameters
by Jiri Denemark 28 Nov '19

28 Nov '19

The virTypedParamsFilter function doesn't mind params == NULL if nparams is zero. And there's no need to check for params == NULL && nparams > 0 because this is checked higher in the stack. In fact all the virCheckNonNull* checks in virTypedParamsFilter are useless. https://bugzilla.redhat.com/show_bug.cgi?id=1777094 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/util/virtypedparam.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 603fcf213a..be3dac1571 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -403,10 +403,6 @@ virTypedParamsFilter(virTypedParameterPtr params, { size_t i, n = 0; - virCheckNonNullArgGoto(params, error); - virCheckNonNullArgGoto(name, error); - virCheckNonNullArgGoto(ret, error); - if (VIR_ALLOC_N(*ret, nparams) < 0) goto error; -- 2.24.0

3 2

[libvirt] [PATCH 1/1] qemu: sync blockjob finishing in qemuDomainGetBlockJobInfo
by Nikolay Shirokovskiy 28 Nov '19

28 Nov '19

Due to race qemuDomainGetBlockJobInfo can return there is no block job for disk but later call to spawn new blockjob can fail because libvirt internally still not process blockjob finishing. Thus let's wait for blockjob finishing if we report there is no more blockjob. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/qemu/qemu_driver.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 669c12d6ca..b148df3a57 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17785,12 +17785,26 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, goto endjob; } + qemuBlockJobSyncBegin(job); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), job->name, &rawInfo); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - if (ret <= 0) + if (ret < 0) + goto endjob; + + if (ret == 0) { + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + while (qemuBlockJobIsRunning(job)) { + if (virDomainObjWait(vm) < 0) { + ret = -1; + goto endjob; + } + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + } goto endjob; + } if (qemuBlockJobInfoTranslate(&rawInfo, info, disk, flags & VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES) < 0) { @@ -17799,6 +17813,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, } endjob: + if (job) + qemuBlockJobSyncEnd(vm, job, QEMU_ASYNC_JOB_NONE); qemuDomainObjEndJob(driver, vm); cleanup: -- 2.23.0

2 6

[libvirt] [go PATCH 0/2] add missing API and constants
by Pavel Hrdina 28 Nov '19

28 Nov '19

Pavel Hrdina (2): Add VIR_FROM_BPF error constant Add support for virDomainAgentSetResponseTimeout API domain.go | 23 +++++++++++++++++++++++ domain_compat.h | 14 ++++++++++++++ domain_wrapper.go | 17 +++++++++++++++++ domain_wrapper.h | 6 ++++++ error.go | 3 +++ error_compat.h | 6 ++++++ 6 files changed, 69 insertions(+) -- 2.23.0

2 5

[libvirt] [PATCH] tests: improve build time for qemuxml2argvtest
by Daniel P. Berrangé 28 Nov '19

28 Nov '19

On Fedora 31 with GCC 9.2.1, compiling qemuxml2argvtest takes about 36 seconds since commit 30c6d99209cd36bba8e14795622f658827698522 Author: Jiri Denemark <jdenemar(a)redhat.com> Date: Thu Oct 24 17:51:42 2019 +0200 qemuxml2argvtest: Update host arch for DO_TEST*ARCH* tests The optimizer is hitting some pathelogical performance behaviour due to the high number of branches in the mymain() method. Pushing the branch tests down into the testCompareXMLToArgv method brings the compile time down to 3 seconds. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tests/qemuxml2argvtest.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a2791d0460..86b8899921 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -412,6 +412,9 @@ testCompareXMLToArgv(const void *data) size_t i; qemuDomainObjPrivatePtr priv = NULL; + if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64) + qemuTestSetHostArch(driver.caps, info->arch); + memset(&monitor_chr, 0, sizeof(monitor_chr)); if (!(conn = virGetConnect())) @@ -580,6 +583,9 @@ testCompareXMLToArgv(const void *data) virSetConnectStorage(NULL); virObjectUnref(conn); VIR_FREE(migrateURI); + if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64) + qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); + return ret; } @@ -699,13 +705,9 @@ mymain(void) __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ testInfoSetPaths(&info, _suffix); \ - if (info.arch != VIR_ARCH_NONE && info.arch != VIR_ARCH_X86_64) \ - qemuTestSetHostArch(driver.caps, info.arch); \ if (virTestRun("QEMU XML-2-ARGV " _name _suffix, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ - if (info.arch != VIR_ARCH_NONE && info.arch != VIR_ARCH_X86_64) \ - qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); \ testQemuInfoClear(&info); \ } while (0) -- 2.23.0

4 4

[libvirt] [PATCH] selinux: Drop needless getfilecon()-s
by Michal Privoznik 28 Nov '19

28 Nov '19

When SELinux support was first introduced the libselinux library wasn't that advanced and setfilecon_raw() or fsetfilecon_raw() could fail even when the target context was set. Looking at the current code [1][2] this is no longer the case. We can drop our workarounds. 1: https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/setfil… 2: https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/fsetfi… Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/security_selinux.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e05ef7593e..0bfb6a7fa6 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1249,8 +1249,6 @@ virSecuritySELinuxSetFileconImpl(const char *path, const char *tcon, bool privileged) { - security_context_t econ; - /* Be aware that this function might run in a separate process. * Therefore, any driver state changes would be thrown away. */ @@ -1259,15 +1257,6 @@ virSecuritySELinuxSetFileconImpl(const char *path, if (setfilecon_raw(path, (const char *)tcon) < 0) { int setfilecon_errno = errno; - if (getfilecon_raw(path, &econ) >= 0) { - if (STREQ(tcon, econ)) { - freecon(econ); - /* It's alright, there's nothing to change anyway. */ - return 1; - } - freecon(econ); - } - /* If the error complaint is related to an image hosted on a (possibly * read-only) NFS mount, or a usbfs/sysfs filesystem not supporting * labelling, then just ignore it & hope for the best. The user @@ -1401,22 +1390,11 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr, static int virSecuritySELinuxFSetFilecon(int fd, char *tcon) { - security_context_t econ; - VIR_INFO("Setting SELinux context on fd %d to '%s'", fd, tcon); if (fsetfilecon_raw(fd, tcon) < 0) { int fsetfilecon_errno = errno; - if (fgetfilecon_raw(fd, &econ) >= 0) { - if (STREQ(tcon, econ)) { - freecon(econ); - /* It's alright, there's nothing to change anyway. */ - return 0; - } - freecon(econ); - } - /* if the error complaint is related to an image hosted on * an nfs mount, or a usbfs/sysfs filesystem not supporting * labelling, then just ignore it & hope for the best. -- 2.23.0

2 1

[libvirt] Change virQEMUDriverConfig defaults?
by Michal Privoznik 28 Nov '19

28 Nov '19

Dear list, lately I was confronted with the way how KubeVirt uses libvirt. They run it inside a docker container with UID mapping and some capabilities dropped. See [1] for more info. Problem is that because of the way we initialize our defaults they had to disable some features. Specifically, virQEMUDriverConfigNew() is passed @privileged argument which equals geteuid() == 0. Apparently, due to UID mapping this leads us to create not valid defaults. For instance, rememberOwner will be set to true in this case, but trying to write any XATTR is going to fail because the container and subsequently libvirtd does have CAP_SYS_ADMIN (and writing trusted.* XATTRs is reserve to CAP_SYS_ADMIN). Similarly, dynamicOwnership won't work unless CAP_CHOWN is granted and if SELinux driver is enabled then we will also need CAP_SYS_ADMIN because setfilecon_raw() does nothing more than write XATTR [2]. So instead of using @privileged we could use capabilities to enable/disable features by default. There is one problem with this though - do we know the full set of capabilities we need for given feature? For instance, does rememberOwner needs just CAP_SYS_ADMIN or also some other capability? Users could still override our defaults through qemu.conf, but I can see us throwing a warning if they are trying to enable say rememberOwner while the libvird lacks CAP_SYS_ADMIN. What is your opinion? Michal 1: https://bugzilla.redhat.com/show_bug.cgi?id=1774373#c17 2: https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/setfil…

2 1

[libvirt] [PATCH for 5.10.0] docs: don't run xmllint when generating API HTML files
by Daniel P. Berrangé 28 Nov '19

28 Nov '19

After generating the API HTML files we run xmllint in docs/html/*.html to validate the correctness. Since commit 0aa8536f14b08dfb5188d0bb3aba532680500d38 Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Wed Nov 20 14:49:26 2019 +0000 docs: generate API reference pages for admin, qemu & lxc libraries we have many rules generating files into docs/html/. The xmllint calls for each rule are picking up files which are part-generated by other parallel build rules resulting in transient errors like: GEN html/index.html GEN html/index-admin.html GEN html/index-qemu.html GEN html/index-lxc.html GEN hvsupport.html.in html/index-lxc.html:1: parser error : Document is empty ^ make[4]: *** [Makefile:2407: html/index-qemu.html] Error 1 The easiest solution is to move the xmllint rules to the 'make check' phase of the build. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- docs/Makefile.am | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 2a104bc837..0e43f382b1 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -311,16 +311,19 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP) $(AM_V_GEN)$(XSLTPROC) --nonet -o ./ \ --stringparam builddir '$(abs_top_builddir)' \ --stringparam timestamp $(timestamp) \ - $(srcdir)/newapi.xsl libvirt-api.xml && \ - $(XMLLINT) --nonet --noout html/*.html + $(srcdir)/newapi.xsl libvirt-api.xml html/index-%.html: libvirt-%-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP) $(AM_V_GEN)$(XSLTPROC) --nonet -o ./ \ --stringparam builddir '$(abs_top_builddir)' \ --stringparam timestamp $(timestamp) \ --stringparam indexfile $(@:html/%=%) \ - $(srcdir)/newapi.xsl $< && \ - $(XMLLINT) --nonet --noout html/*.html + $(srcdir)/newapi.xsl $< + +check-html: + $(XMLLINT) --nonet --noout html/*.html + +check-local: check-html python_generated_files = \ html/libvirt-libvirt-lxc.html \ -- 2.23.0

2 1

[libvirt] [python PATCH] generator.py: add mapping for VIR_DOMAIN_QEMU_AGENT_COMMAND_*
by Pavel Hrdina 28 Nov '19

28 Nov '19

Libvirt commit <95f5ac9ae52455e9da47afc95fa31c9456ac27ae> changed the VIR_DOMAIN_QEMU_AGENT_COMMAND_* enum values to use different enum values instead of direct numbers. We need to translate it back. Traceback (most recent call last): File "generator.py", line 2143, in <module> qemuBuildWrappers(sys.argv[1]) File "generator.py", line 2008, in qemuBuildWrappers items.sort(key=lambda i: (int(i[1]), i[0])) File "generator.py", line 2008, in <lambda> items.sort(key=lambda i: (int(i[1]), i[0])) ValueError: invalid literal for int() with base 10: 'VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK' Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- generator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/generator.py b/generator.py index 913dab8..3352521 100755 --- a/generator.py +++ b/generator.py @@ -261,6 +261,12 @@ def lxc_enum(type, name, value): def qemu_enum(type, name, value): if type not in qemu_enums: qemu_enums[type] = {} + if value == 'VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK': + value = -2 + elif value == 'VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_DEFAULT': + value = -1 + elif value == 'VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_NOWAIT': + value = 0 if onlyOverrides and name not in qemu_enums[type]: return qemu_enums[type][name] = value -- 2.23.0

2 3

[libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
by LanceLiu 28 Nov '19

28 Nov '19

--- src/libvirt_remote.syms | 1 + src/remote/remote_daemon_stream.c | 10 +++++++++- src/rpc/virnetserverclient.c | 12 ++++++++++++ src/rpc/virnetserverclient.h | 2 ++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 0493467..c32e234 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -173,6 +173,7 @@ virNetServerClientPreExecRestart; virNetServerClientRemoteAddrStringSASL; virNetServerClientRemoteAddrStringURI; virNetServerClientRemoveFilter; +virNetServerClientCheckFilterExist; virNetServerClientSendMessage; virNetServerClientSetAuthLocked; virNetServerClientSetAuthPendingLocked; diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index 82cadb6..de0dca3 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -292,10 +292,18 @@ daemonStreamFilter(virNetServerClientPtr client, { daemonClientStream *stream = opaque; int ret = 0; + daemonClientPrivatePtr priv = NULL; + int filter_id = stream->filterID; virObjectUnlock(client); + priv = virNetServerClientGetPrivateData(client); virMutexLock(&stream->priv->lock); virObjectLock(client); + if (!virNetServerClientCheckFilterExist(client, filter_id)) { + VIR_WARN("this daemon stream filter: %d have been deleted!", filter_id); + ret = -1; + goto cleanup; + } if (msg->header.type != VIR_NET_STREAM && msg->header.type != VIR_NET_STREAM_HOLE) @@ -317,7 +325,7 @@ daemonStreamFilter(virNetServerClientPtr client, ret = 1; cleanup: - virMutexUnlock(&stream->priv->lock); + virMutexUnlock(&priv->lock); return ret; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 67b3bf9..f80f493 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -287,6 +287,18 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, virObjectUnlock(client); } +int virNetServerClientCheckFilterExist(virNetServerClientPtr client, + int filterID) +{ + virNetServerClientFilterPtr tmp; + + tmp = client->filters; + while(tmp && tmp->id != filterID) { + tmp = tmp->next; + } + + return (tmp != NULL); +} /* Check the client's access. */ static int diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 7a3061d..85fda39 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -93,6 +93,8 @@ int virNetServerClientAddFilter(virNetServerClientPtr client, void virNetServerClientRemoveFilter(virNetServerClientPtr client, int filterID); +int virNetServerClientCheckFilterExist(virNetServerClientPtr client, + int filterID); int virNetServerClientGetAuth(virNetServerClientPtr client); void virNetServerClientSetAuthLocked(virNetServerClientPtr client, int auth); -- 1.8.3.1

3 7

[libvirt] [PATCH] travis: explicitly require xcode 10.3 image for macOS
by Daniel P. Berrangé 27 Nov '19

27 Nov '19

The default macOS image in travis is broken, throwing python exceptions when trying to install glib. Explicitly ask for the newer 10.3 image which works correctly. We now need to also point to the homebrew installed libxml2 rather than the OS distro provided one, since the OS distro one has a pkg-config file present, but no actual header files. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 75f32e53b9..80f3be5490 100644 --- a/.travis.yml +++ b/.travis.yml @@ -62,8 +62,10 @@ matrix: - compiler: clang language: c os: osx + osx_image: xcode10.3 env: - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/ccache/libexec:/usr/local/opt/rpcgen/bin:$PATH" + - PKG_CONFIG_PATH="/usr/local/opt/libxml2/lib/pkgconfig" script: # We can't run 'distcheck' or 'syntax-check' because they fail on # macOS, but doing 'install' and 'dist' gives us some useful coverage -- 2.21.0

2 1