2:54 p.m.
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 */