[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] qemu: Make qemuMigrationIsAllowed more reusable



On 12/07/2012 04:30 AM, Peter Krempa wrote:
> This patch exports qemuMigrationIsAllowed and adds a new parameter to it
> to denote if it's a remote migration or a local migration. Local
> migrations are used in snapshots and saving of the machine state and
> have fewer restrictions. This patch also adjusts callers of the function
> and tweaks some error messages to be more universal.
> ---
>  src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++++++-------------------
>  src/qemu/qemu_migration.h |  2 ++
>  2 files changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 86060dc..7e5cf01 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1011,30 +1011,38 @@ error:
>   * assume that checking on source is sufficient to prevent ever
>   * talking to the destination in the first place, we are stuck with
>   * the fact that older servers did not do checks on the source. */
> -static bool
> +bool
>  qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm,
> -                       virDomainDefPtr def)
> +                       virDomainDefPtr def, bool remote)
>  {

> +        /* perform these checks only when migrating to remote hosts */
> +        if (remote) {
> +            if  (qemuProcessAutoDestroyActive(driver, vm)) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               "%s", _("domain is marked for auto destroy"));
> +                return false;
> +            }

The check for autodestroy should be universal - we don't want to permit
migration-to-file on an autodestroy guest.

> +
> +            nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0);
> +            if (nsnapshots < 0)
> +                return false;
> +
> +            if (nsnapshots > 0) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("cannot migrate domain with %d snapshots"),
> +                               nsnapshots);
> +                return false;
> +            }

Agree with making this remote-only (and even then, it would be nice to
someday lift this restriction, if I can figure out a way to migrate an
arbitrary amount of snapshot metadata during migration.  The problem we
have is that the migration cookie is fixed size, but if you take enough
snapshots, you can exceed that size).

ACK with the autodestroy hunk hoisted out of the 'if (remote)' block.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]