[libvirt] [PATCHv4 14/18] blockjob: allow mirroring under SELinux

[Jiri Denemark]

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