Re: [PATCH 2/7] qemu: Simplify qemuMigrationFinish

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

 



On Wed, Jul 08, 2015 at 19:36:01 +0200, Jiri Denemark wrote:
> Offline migration migration is quite special because we don't really
> need to do anything but make the domain persistent. Let's do it
> separately from normal migration to avoid cluttering the code with
> !(flags & VIR_MIGRATE_OFFLINE).
> 
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>  src/qemu/qemu_migration.c | 72 +++++++++++++++++++++++------------------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 6b06e79..6a3e2e6 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5625,8 +5625,14 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>      /* Did the migration go as planned?  If yes, return the domain
>       * object, but if no, clean up the empty qemu process.
>       */
> -    if (retcode == 0) {
> -        if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE)) {
> +    if (flags & VIR_MIGRATE_OFFLINE) {
> +        if (retcode != 0 ||
> +            qemuMigrationPersist(driver, vm, mig) < 0)
> +            goto endjob;

This isn't entirely equivalend to the previous impl, since if
qemuMigrationPersist fails at a later stage the vm->persistent flag does
get set. The code below cleared it in some cases, but in this case it
would not get cleared.

Either qemuMigrationPersist should get updated so that the flag is set
only when everything went well, or you should clear if afterwards.

Later on in the endjob section there is a check if the object is active
so you'd get a persistent VM object with no definition in some cases.

> +
> +        dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
> +    } else if (retcode == 0) {
> +        if (!virDomainObjIsActive(vm)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("guest unexpectedly quit"));
>              qemuMigrationErrorReport(driver, vm->def->name);

ACK if you fix the offline migration error handling.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]