From 6a7adc1de1bc55404c1f11a6687f87045c3ef4025708513b247c9aeaadabb761 Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Thu, 10 Sep 2020 17:53:40 +0000 Subject: [PATCH] Accepting request 833545 from home:jfehlig:branches:Virtualization - spec: Increase test timeout to account for slower test execution on some architectures - qemu: Tolerate non-existing files such as /dev/kvm when populating domain private namespace 8abd1ffe-qemu-tolerate-non-existent-files.patch, 4a72b76b-qemu-namespace-memleak-fix.patch boo#1176380 OBS-URL: https://build.opensuse.org/request/show/833545 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=843 --- 4a72b76b-qemu-namespace-memleak-fix.patch | 118 ++++++++++++++++++ ...ffe-qemu-tolerate-non-existent-files.patch | 70 +++++++++++ libvirt.changes | 15 +++ libvirt.spec | 6 +- 4 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 4a72b76b-qemu-namespace-memleak-fix.patch create mode 100644 8abd1ffe-qemu-tolerate-non-existent-files.patch diff --git a/4a72b76b-qemu-namespace-memleak-fix.patch b/4a72b76b-qemu-namespace-memleak-fix.patch new file mode 100644 index 0000000..677e3e2 --- /dev/null +++ b/4a72b76b-qemu-namespace-memleak-fix.patch @@ -0,0 +1,118 @@ +commit 4a72b76b8a99ab6c33f468e767cb33cf1fcec843 +Author: Michal Prívozník +Date: Mon Sep 7 13:35:50 2020 +0200 + + qemu_namespace: Don't leak mknod items that are being skipped over + + When building and populating domain NS a couple of functions are + called that append paths to a string list. This string list is + then inspected, one item at the time by + qemuNamespacePrepareOneItem() which gathers all the info for + given path (stat buffer, possible link target, ACLs, SELinux + label) using qemuNamespaceMknodItemInit(). If the path needs to + be created in the domain's private /dev then it's added onto this + qemuNamespaceMknodData list which is freed later in the process. + But, if the path does not need to be created in the domain's + private /dev, then the memory allocated by + qemuNamespaceMknodItemInit() is not freed anywhere leading to a + leak. + + Signed-off-by: Michal Privoznik + Reviewed-by: Ján Tomko + +Index: libvirt-6.7.0/src/qemu/qemu_namespace.c +=================================================================== +--- libvirt-6.7.0.orig/src/qemu/qemu_namespace.c ++++ libvirt-6.7.0/src/qemu/qemu_namespace.c +@@ -871,7 +871,7 @@ qemuDomainNamespaceAvailable(qemuDomainN + typedef struct _qemuNamespaceMknodItem qemuNamespaceMknodItem; + typedef qemuNamespaceMknodItem *qemuNamespaceMknodItemPtr; + struct _qemuNamespaceMknodItem { +- const char *file; ++ char *file; + char *target; + bool bindmounted; + GStatBuf sb; +@@ -892,6 +892,7 @@ struct _qemuNamespaceMknodData { + static void + qemuNamespaceMknodItemClear(qemuNamespaceMknodItemPtr item) + { ++ VIR_FREE(item->file); + VIR_FREE(item->target); + virFileFreeACLs(&item->acl); + #ifdef WITH_SELINUX +@@ -900,6 +901,8 @@ qemuNamespaceMknodItemClear(qemuNamespac + } + + ++G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuNamespaceMknodItem, qemuNamespaceMknodItemClear); ++ + static void + qemuNamespaceMknodDataClear(qemuNamespaceMknodDataPtr data) + { +@@ -1091,7 +1094,7 @@ qemuNamespaceMknodItemInit(qemuNamespace + bool isLink; + bool needsBindMount; + +- item->file = file; ++ item->file = g_strdup(file); + + if (g_lstat(file, &item->sb) < 0) { + if (errno == ENOENT) +@@ -1166,11 +1169,13 @@ qemuNamespacePrepareOneItem(qemuNamespac + size_t ndevMountsPath) + { + long ttl = sysconf(_SC_SYMLOOP_MAX); +- const char *next = file; ++ g_autofree char *next = g_strdup(file); + size_t i; + + while (1) { +- qemuNamespaceMknodItem item = { 0 }; ++ g_auto(qemuNamespaceMknodItem) item = { 0 }; ++ bool isLink; ++ bool addToData = false; + int rc; + + rc = qemuNamespaceMknodItemInit(&item, cfg, vm, next); +@@ -1182,6 +1187,8 @@ qemuNamespacePrepareOneItem(qemuNamespac + return -1; + } + ++ isLink = S_ISLNK(item.sb.st_mode); ++ + if (STRPREFIX(next, QEMU_DEVPREFIX)) { + for (i = 0; i < ndevMountsPath; i++) { + if (STREQ(devMountsPath[i], "/dev")) +@@ -1190,12 +1197,18 @@ qemuNamespacePrepareOneItem(qemuNamespac + break; + } + +- if (i == ndevMountsPath && +- VIR_APPEND_ELEMENT_COPY(data->items, data->nitems, item) < 0) +- return -1; ++ if (i == ndevMountsPath) ++ addToData = true; + } + +- if (!S_ISLNK(item.sb.st_mode)) ++ g_free(next); ++ next = g_strdup(item.target); ++ ++ if (addToData && ++ VIR_APPEND_ELEMENT(data->items, data->nitems, item) < 0) ++ return -1; ++ ++ if (!isLink) + break; + + if (ttl-- == 0) { +@@ -1204,8 +1217,6 @@ qemuNamespacePrepareOneItem(qemuNamespac + next); + return -1; + } +- +- next = item.target; + } + + return 0; diff --git a/8abd1ffe-qemu-tolerate-non-existent-files.patch b/8abd1ffe-qemu-tolerate-non-existent-files.patch new file mode 100644 index 0000000..f41f9f6 --- /dev/null +++ b/8abd1ffe-qemu-tolerate-non-existent-files.patch @@ -0,0 +1,70 @@ +commit 8abd1ffed18394a6212c469cb2c7b6cc28a122d2 +Author: Michal Prívozník +Date: Thu Sep 3 18:07:43 2020 +0200 + + qemu_namespace: Be tolerant to non-existent files when populating /dev + + In 6.7.0 release I've changed how domain namespace is built and + populated. Previously it used to be done from a pre-exec hook + (ran in the forked off child, just before dropping all privileges + and exec()-ing QEMU), which not only meant we had to have two + different code paths for creating a node in domain's namespace + (one for this pre-exec hook, the other for hotplug ran from the + daemon), it also proved problematic because it was leaking FDs + into QEMU process. + + To mitigate this problem, we've not only ditched libdevmapper + from the NS population process, I've also dropped the pre-exec + code and let the NS be populated from the daemon (using the + hotplug code). But, I was not careful when doing so, because the + pre-exec code was tolerant to files that doesn't exist, while + this new code isn't. For instance, the very first thing that is + done when the new NS is created is it's populated with + @defaultDeviceACL which contain files like /dev/null, /dev/zero, + /dev/random and /dev/kvm (and others). While the rest will + probably exist every time, /dev/kvm might not and thus the new + code I wrote has to be tolerant to that. + + Of course, users can override the @defaultDeviceACL (by setting + cgroup_device_acl in qemu.conf) and remove /dev/kvm (which is + acceptable workaround), but we definitely want libvirt to work + out of the box even on hosts without KVM. + + Fixes: 9048dc4e627ddf33996084167bece7b5fb83b0bc + Reported-by: Daniel P. Berrangé + Signed-off-by: Michal Privoznik + Reviewed-by: Daniel P. Berrangé + Reviewed-by: Ján Tomko + +Index: libvirt-6.7.0/src/qemu/qemu_namespace.c +=================================================================== +--- libvirt-6.7.0.orig/src/qemu/qemu_namespace.c ++++ libvirt-6.7.0/src/qemu/qemu_namespace.c +@@ -1094,6 +1094,9 @@ qemuNamespaceMknodItemInit(qemuNamespace + item->file = file; + + if (g_lstat(file, &item->sb) < 0) { ++ if (errno == ENOENT) ++ return -2; ++ + virReportSystemError(errno, + _("Unable to access %s"), file); + return -1; +@@ -1168,9 +1171,16 @@ qemuNamespacePrepareOneItem(qemuNamespac + + while (1) { + qemuNamespaceMknodItem item = { 0 }; ++ int rc; + +- if (qemuNamespaceMknodItemInit(&item, cfg, vm, next) < 0) ++ rc = qemuNamespaceMknodItemInit(&item, cfg, vm, next); ++ if (rc == -2) { ++ /* @file doesn't exist. We can break here. */ ++ break; ++ } else if (rc < 0) { ++ /* Some other (critical) error. */ + return -1; ++ } + + if (STRPREFIX(next, QEMU_DEVPREFIX)) { + for (i = 0; i < ndevMountsPath; i++) { diff --git a/libvirt.changes b/libvirt.changes index 596e62d..0feb113 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,18 @@ +------------------------------------------------------------------- +Thu Sep 10 17:49:45 UTC 2020 - James Fehlig + +- spec: Increase test timeout to account for slower test execution + on some architectures + +------------------------------------------------------------------- +Thu Sep 10 16:23:12 UTC 2020 - James Fehlig + +- qemu: Tolerate non-existing files such as /dev/kvm when + populating domain private namespace + 8abd1ffe-qemu-tolerate-non-existent-files.patch, + 4a72b76b-qemu-namespace-memleak-fix.patch + boo#1176380 + ------------------------------------------------------------------- Wed Sep 2 17:18:34 UTC 2020 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index 06e06b1..85498e1 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -310,6 +310,8 @@ Source99: baselibs.conf Source100: %{name}-rpmlintrc # Upstream patches Patch0: 2ad009ea-qemu-check-modules-dir.patch +Patch1: 8abd1ffe-qemu-tolerate-non-existent-files.patch +Patch2: 4a72b76b-qemu-namespace-memleak-fix.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch @@ -847,6 +849,8 @@ libvirt plugin for NSS for translating domain names into IP addresses. %prep %setup -q %patch0 -p1 +%patch1 -p1 +%patch2 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1 @@ -1187,7 +1191,7 @@ mv %{buildroot}/%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \ %fdupes -s %{buildroot} %check -VIR_TEST_DEBUG=1 %meson_test --no-suite syntax-check +VIR_TEST_DEBUG=1 %meson_test -t 5 --no-suite syntax-check %pre daemon %{_bindir}/getent group libvirt >/dev/null || %{_sbindir}/groupadd -r libvirt