When using block copy to pivot over to a new chain, the backing files for the new chain might still need labeling (particularly if the user passes --reuse-ext with a relative backing file name). Relabeling a file that is already labeled won't hurt, so this just labels the entire chain at the point of the pivot. But to do the relabel, we must probe the file type of an external file (the API already promised to do this, and the probe is safe; we are just moving the probe from qemu into libvirt). https://bugzilla.redhat.com/show_bug.cgi?id=856247 * src/qemu/qemu_driver.c (qemuDomainBlockPivot): Relabel chain before asking qemu to pivot. (qemuDomainBlockCopy): When reusing external file, probe its type. --- Changes since v7.6: hoist the safe format probing out of qemu into libvirt when reusing an external file in shallow mode; this version finally passed testing from VDSM folks src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6db72ee..acc3807 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12491,6 +12491,9 @@ qemuDomainBlockPivot(virConnectPtr conn, virDomainBlockJobInfo info; bool reopen = qemuCapsGet(priv->caps, QEMU_CAPS_DRIVE_REOPEN); bool resume = false; + virCgroupPtr cgroup = NULL; + char *oldsrc = NULL; + char *olddriver = NULL; /* Probe the status, if needed. */ if (!disk->mirroring) { @@ -12538,6 +12541,33 @@ qemuDomainBlockPivot(virConnectPtr conn, } } + /* We previously labeled only the top-level image; but if the + * image includes a relative backing file, the pivot may result in + * qemu needing to open the entire backing chain, so we need to + * label the entire chain. This action is safe even if the + * backing chain has already been labeled; but only necessary when + * we know for sure that there is a backing chain. */ + if (disk->mirrorFormat && + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && + virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s"), + vm->def->name); + goto cleanup; + } + oldsrc = disk->src; + olddriver = disk->driverType; + disk->src = disk->mirror; + disk->driverType = disk->mirrorFormat; + if (disk->mirrorFormat && + ((cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) || + virSecurityManagerSetImageLabel(driver->securityManager, vm->def, + disk) < 0)) { + disk->src = oldsrc; + disk->driverType = olddriver; + goto cleanup; + } + /* Attempt the pivot. */ qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror, @@ -12557,10 +12587,8 @@ qemuDomainBlockPivot(virConnectPtr conn, * portion of the chain, and is made more difficult by the * fact that we aren't tracking the full chain ourselves; so * for now, we leak the access to the original. */ - VIR_FREE(disk->src); - VIR_FREE(disk->driverType); - disk->src = disk->mirror; - disk->driverType = disk->mirrorFormat; + VIR_FREE(oldsrc); + VIR_FREE(olddriver); disk->mirror = NULL; disk->mirrorFormat = NULL; disk->mirroring = false; @@ -12572,12 +12600,16 @@ qemuDomainBlockPivot(virConnectPtr conn, * 'query-block', to see what state we really got left in * before killing the mirroring job? And just as on the * success case, there's security labeling to worry about. */ + disk->src = oldsrc; + disk->driverType = olddriver; VIR_FREE(disk->mirror); VIR_FREE(disk->mirrorFormat); disk->mirroring = false; } cleanup: + if (cgroup) + virCgroupFree(&cgroup); if (resume && virDomainObjIsActive(vm) && qemuProcessStartCPUs(driver, vm, conn, VIR_DOMAIN_RUNNING_UNPAUSED, @@ -12913,6 +12945,18 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, VIR_FORCE_CLOSE(fd); if (!format) format = disk->driverType; + } else if (!format && + !(flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) && + (flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW)) { + /* When reusing an existing shallow file, we need to know the + * file type in order to relabel the chain later in + * qemuDomainBlockPivot. Normally, probing is unsafe because + * raw files can be misidentified as non-raw, but in this + * particular case, the user said the file is not raw, and + * having a backing file implies non-raw. */ + int type = virStorageFileProbeFormat(dest); + if (type >= 0) + format = virStorageFileFormatTypeToString(type); } if ((format && !(mirrorFormat = strdup(format))) || !(mirror = strdup(dest))) { -- 1.7.11.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list