On Mon, Apr 09, 2012 at 21:52:23 -0600, Eric Blake wrote:
> This copies heavily from qemuDomainSnapshotCreateSingleDiskActive(),
> in order to set the SELinux label, obtain locking manager lease, and
> audit the fact that we hand a new file over to qemu. Alas, releasing
> the lease and label on failure or at the end of the mirroring is a
> trickier prospect.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set up labeling.
> ---
> src/qemu/qemu_driver.c | 63 +++++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 54 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 661ccb4..41f545f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11895,6 +11895,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
> int ret = -1;
> int idx;
> struct stat st;
> + bool need_unlink = false;
> + char *mirror = NULL;
> + char *mirrorFormat = NULL;
> + char *origsrc = NULL;
> + char *origdriver = NULL;
>
> /* Preliminaries: find the disk we are editing, sanity checks */
> virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> @@ -11976,29 +11981,69 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
> goto endjob;
> }
>
> - /* XXX We also need to add security labeling, lock manager lease,
> - * and auditing of those events. */
> - if (!format && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
> - format = disk->driverType;
> - if ((format && !(disk->mirrorFormat = strdup(format))) ||
> - !(disk->mirror = strdup(dest))) {
> + if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
> + int fd = qemuOpenFile(driver, dest, O_WRONLY | O_TRUNC | O_CREAT,
> + &need_unlink, NULL);
> + if (fd < 0)
> + goto endjob;
> + VIR_FORCE_CLOSE(fd);
> + if (!format)
> + format = disk->driverType;
> + }
> + if ((format && !(mirrorFormat = strdup(format))) ||
> + !(mirror = strdup(dest))) {
> virReportOOMError();
> goto endjob;
> }
>
> + /* Manipulate disk in place, in a way that can be reverted on
> + * failure, in order to set up labeling and locking. */
> + origsrc = disk->src;
> + disk->src = (char *) dest;
> + origdriver = disk->driverType;
> + disk->driverType = (char *) "raw"; /* Don't want to probe backing files */
> +
> + if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
> + goto endjob;
> + if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
> + disk) < 0) {
> + if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
> + VIR_WARN("Unable to release lock on %s", dest);
> + goto endjob;
> + }
Is there a reason for calling virDomainLockDiskDetach only when SetImageLabel
fails and not calling it if mirroring itself fails?
> +
> + disk->src = origsrc;
> + origsrc = NULL;
> + disk->driverType = origdriver;
> + origdriver = NULL;
> +
> /* Actually start the mirroring */
> qemuDomainObjEnterMonitorWithDriver(driver, vm);
> ret = qemuMonitorDriveMirror(priv->mon, NULL, device, dest, format, flags);
> + virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0);
> if (ret == 0 && bandwidth != 0)
> ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
> BLOCK_JOB_SPEED_INTERNAL);
> qemuDomainObjExitMonitorWithDriver(driver, vm);
> + if (ret < 0)
> + goto endjob;
> +
> + /* Update vm in place to match changes. */
> + need_unlink = false;
> + disk->mirror = mirror;
> + disk->mirrorFormat = mirrorFormat;
> + mirror = NULL;
> + mirrorFormat = NULL;
>
> endjob:
> - if (ret < 0) {
> - VIR_FREE(disk->mirror);
> - VIR_FREE(disk->mirrorFormat);
> + if (origsrc) {
> + disk->src = origsrc;
> + disk->driverType = origdriver;
> }
> + if (need_unlink && unlink(dest))
> + VIR_WARN("unable to unlink just-created %s", dest);
> + VIR_FREE(mirror);
> + VIR_FREE(mirrorFormat);
> if (qemuDomainObjEndJob(driver, vm) == 0) {
> vm = NULL;
> goto cleanup;
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list