[PATCHv2] snapshot: allow block devices past cgroup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It turns out that when cgroups are enabled, the use of a block device
for a snapshot target was failing with EPERM due to libvirt failing
to add the block device to the cgroup whitelist.  See also
https://bugzilla.redhat.com/show_bug.cgi?id=810200

* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive): Account for cgroup.
(qemuDomainSnapshotCreateDiskActive): Update caller.
---

v2: fix resource leak on error path
See interdiff below the actual patch...

 src/qemu/qemu_driver.c |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2bec617..92535b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9895,6 +9895,7 @@ cleanup:
 static int
 qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
                                          virDomainObjPtr vm,
+                                         virCgroupPtr cgroup,
                                          virDomainSnapshotDiskDefPtr snap,
                                          virDomainDiskDefPtr disk,
                                          virDomainDiskDefPtr persistDisk,
@@ -9948,8 +9949,15 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,

     if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
         goto cleanup;
+    if (cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) {
+        if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+            VIR_WARN("Unable to release lock on %s", source);
+        goto cleanup;
+    }
     if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
                                         disk) < 0) {
+        if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+            VIR_WARN("Failed to teardown cgroup for disk path %s", source);
         if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
             VIR_WARN("Unable to release lock on %s", source);
         goto cleanup;
@@ -10009,6 +10017,7 @@ cleanup:
 static void
 qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
                                        virDomainObjPtr vm,
+                                       virCgroupPtr cgroup,
                                        virDomainDiskDefPtr origdisk,
                                        virDomainDiskDefPtr disk,
                                        virDomainDiskDefPtr persistDisk,
@@ -10033,6 +10042,8 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
     if (virSecurityManagerRestoreImageLabel(driver->securityManager,
                                             vm->def, disk) < 0)
         VIR_WARN("Unable to restore security label on %s", disk->src);
+    if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+        VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src);
     if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
         VIR_WARN("Unable to release lock on %s", disk->src);
     if (need_unlink && stat(disk->src, &st) == 0 &&
@@ -10084,6 +10095,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
     int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */
     bool atomic = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0;
     bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
+    virCgroupPtr cgroup = NULL;

     if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
         return -1;
@@ -10094,6 +10106,14 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
         goto endjob;
     }

+    if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) &&
+        virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) {
+        qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                        _("Unable to find cgroup for %s"),
+                        vm->def->name);
+        goto endjob;
+    }
+
     /* If quiesce was requested, then issue a freeze command, and a
      * counterpart thaw command, no matter what.  The command will
      * fail if the guest is paused or the guest agent is not
@@ -10156,7 +10176,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
             }
         }

-        ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
+        ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, cgroup,
                                                        &snap->def->disks[i],
                                                        vm->def->disks[i],
                                                        persistDisk, actions,
@@ -10184,7 +10204,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
                         persistDisk = vm->newDef->disks[indx];
                 }

-                qemuDomainSnapshotUndoSingleDiskActive(driver, vm,
+                qemuDomainSnapshotUndoSingleDiskActive(driver, vm, cgroup,
                                                        snap->def->dom->disks[i],
                                                        vm->def->disks[i],
                                                        persistDisk,
@@ -10214,6 +10234,8 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
     }

 cleanup:
+    if (cgroup)
+        virCgroupFree(&cgroup);
     if (resume && virDomainObjIsActive(vm)) {
         if (qemuProcessStartCPUs(driver, vm, conn,
                                  VIR_DOMAIN_RUNNING_UNPAUSED,
-- 
1.7.7.6


Interdiff from v1:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 92535b9..c9e047e 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -10113,6 +10113,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
                         vm->def->name);
         goto endjob;
     }
+    /* 'cgroup' is still NULL if cgroups are disabled.  */

     /* If quiesce was requested, then issue a freeze command, and a
      * counterpart thaw command, no matter what.  The command will
@@ -10234,8 +10235,6 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
     }

 cleanup:
-    if (cgroup)
-        virCgroupFree(&cgroup);
     if (resume && virDomainObjIsActive(vm)) {
         if (qemuProcessStartCPUs(driver, vm, conn,
                                  VIR_DOMAIN_RUNNING_UNPAUSED,
@@ -10256,6 +10255,8 @@ cleanup:
     }

 endjob:
+    if (cgroup)
+        virCgroupFree(&cgroup);
     if (vm && thaw != 0 &&
         qemuDomainSnapshotFSThaw(driver, vm, thaw > 0) < 0) {
         /* helper reported the error, if it was needed */

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]