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 */