[libvirt] [PATCH v2 2/2] qemu: Implement migration optional disk

[Daniel P. Berrange]

On Mon, Oct 03, 2011 at 04:07:24PM +0200, Michal Privoznik wrote:
> This patch implements previous extension in qemu driver.
> That is, during prepare phase check for every source to be accessible.
> If not, but marked as optional, simply VIR_FREE the source.
> Moreover, if migration is persistent, we must check inactive xml,
> which is transfered at the finish phase, as well.
> ---
>  src/qemu/qemu_migration.c |   51 +++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 1122dab..4aa2532 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1039,6 +1039,48 @@ cleanup:
>      return rv;
>  }
>  
> +/* qemuCheckDisksPresence:
> + * @def domain definition
> + *
> + * Iterate over domain disks and check if source exists.
> + * If not and:
> + * - it's marked as optional, free() it.
> + * - it's marked as required, throw an error.
> + *
> + * Returns 0 on success (all remaining disks/sources exist
> + * or have been dropped),
> + * -1 on failure.
> + */
> +static int
> +qemuCheckDisksPresence(virDomainDefPtr def) {
> +    int ret = -1;
> +    int i;
> +    virDomainDiskDefPtr disk;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        disk = def->disks[i];
> +
> +        if (virFileExists(disk->src)) {
> +            if (disk->migration.optional == VIR_DOMAIN_DEVICE_MIGRATION_OPT_DROP)
> +                VIR_FREE(disk->src);
> +
> +            continue;
> +        }

This is not going to play nice with RHEVM, where images are stored on
a root squashing NFS server, where libvirtd has no visibility, but
QEMU can access.

At the very least you need to run this check in a separate process
which is running as the QEMU user/group ID.

> +
> +        if (disk->migration.optional == VIR_DOMAIN_DEVICE_MIGRATION_OPT_REQ) {
> +            qemuReportError(VIR_ERR_NO_SOURCE,
> +                            _("no such file %s"),
> +                            disk->src);
> +            goto cleanup;
> +        }
> +
> +        VIR_FREE(disk->src);
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    return ret;
> +}
>  
>  /* Prepare is the first step, and it runs on the destination host.
>   */
> @@ -1087,6 +1129,9 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
>      if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
>          goto cleanup;
>  
> +    if (qemuCheckDisksPresence(def) < 0)
> +        goto cleanup;
> +
>      if (!(vm = virDomainAssignDef(driver->caps,
>                                    &driver->domains,
>                                    def, true))) {
> @@ -2579,9 +2624,11 @@ qemuMigrationFinish(struct qemud_driver *driver,
>              if (vm->persistent)
>                  newVM = 0;
>              vm->persistent = 1;
> -            if (mig->persistent)
> +            if (mig->persistent) {
> +                if (qemuCheckDisksPresence(mig->persistent) < 0)
> +                    goto endjob;

What's the point in checking here ?  You've already checked at te
start of migration, and if it has gone away since then, it is too
late for this check todo anything useful


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|