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

@@ -0,0 +1,118 @@
+commit 4a72b76b8a99ab6c33f468e767cb33cf1fcec843
+Author: Michal Prívozník <mprivozn@redhat.com>
+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 <mprivozn@redhat.com>
+    Reviewed-by: Ján Tomko <jtomko@redhat.com>
+
+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;

8abd1ffe-qemu-tolerate-non-existent-files.patch

@@ -0,0 +1,70 @@
+commit 8abd1ffed18394a6212c469cb2c7b6cc28a122d2
+Author: Michal Prívozník <mprivozn@redhat.com>
+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é <berrange@redhat.com>
+    Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
+    Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
+    Reviewed-by: Ján Tomko <jtomko@redhat.com>
+
+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++) {

libvirt.changes

@@ -1,3 +1,18 @@
+-------------------------------------------------------------------
+Thu Sep 10 17:49:45 UTC 2020 - James Fehlig <jfehlig@suse.com>
+
+- spec: Increase test timeout to account for slower test execution
+  on some architectures
+
+-------------------------------------------------------------------
+Thu Sep 10 16:23:12 UTC 2020 - James Fehlig <jfehlig@suse.com>
+
+- 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 <jfehlig@suse.com>
 

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