qemu: don't check whether offline migration is safe

[libvirt] [PATCH] qemu: don't check whether offline migration is safe

Posted by Pavel Hrdina 6 years, 8 months ago

Offline migration transfers only the domain definition.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449715

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_migration.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 056c051b3e..ca1f67146b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1946,7 +1946,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
         goto cleanup;
 
-    if (!(flags & VIR_MIGRATE_UNSAFE) &&
+    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
         goto cleanup;
 
@@ -4809,7 +4809,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
         goto endjob;
 
-    if (!(flags & VIR_MIGRATE_UNSAFE) &&
+    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
         goto endjob;
 
-- 
2.13.5

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

Re: [libvirt] [PATCH] qemu: don't check whether offline migration is safe

Posted by Martin Kletzander 6 years, 8 months ago

On Thu, Aug 17, 2017 at 06:53:45PM +0200, Pavel Hrdina wrote:
>Offline migration transfers only the domain definition.
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449715
>
>Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>---
> src/qemu/qemu_migration.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>index 056c051b3e..ca1f67146b 100644
>--- a/src/qemu/qemu_migration.c
>+++ b/src/qemu/qemu_migration.c
>@@ -1946,7 +1946,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
>     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
>         goto cleanup;
>
>-    if (!(flags & VIR_MIGRATE_UNSAFE) &&
>+    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
>         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
>         goto cleanup;
>
>@@ -4809,7 +4809,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
>     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
>         goto endjob;
>
>-    if (!(flags & VIR_MIGRATE_UNSAFE) &&
>+    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
>         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
>         goto endjob;
>

IMHO it would make a bit more sense if that check was in
qemuMigrationIsSafe(), but I'm okay with both.  Your choice.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>


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

Re: [libvirt] [PATCH] qemu: don't check whether offline migration is safe

Posted by Pavel Hrdina 6 years, 8 months ago

On Fri, Aug 18, 2017 at 12:52:50PM +0200, Martin Kletzander wrote:
> On Thu, Aug 17, 2017 at 06:53:45PM +0200, Pavel Hrdina wrote:
> > Offline migration transfers only the domain definition.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449715
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> > src/qemu/qemu_migration.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 056c051b3e..ca1f67146b 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -1946,7 +1946,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
> >     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
> >         goto cleanup;
> > 
> > -    if (!(flags & VIR_MIGRATE_UNSAFE) &&
> > +    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
> >         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
> >         goto cleanup;
> > 
> > @@ -4809,7 +4809,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
> >     if (!qemuMigrationIsAllowed(driver, vm, true, flags))
> >         goto endjob;
> > 
> > -    if (!(flags & VIR_MIGRATE_UNSAFE) &&
> > +    if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
> >         !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks, flags))
> >         goto endjob;
> > 
> 
> IMHO it would make a bit more sense if that check was in
> qemuMigrationIsSafe(), but I'm okay with both.  Your choice.

I had similar idea to put the VIR_MIGRATE_OFFLINE check inside that
function but when I saw that we already check for _UNSAFE outside of
that function I put it there as well.

Now that you've pointed it out, it would be better to put both checks
inside that function.  Since this patch is already pushed, I'll send
a follow up patch.

Thanks

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

Re: [libvirt] [PATCH] qemu: don't check whether offline migration is safe

Posted by Erik Skultety 6 years, 8 months ago

On Thu, Aug 17, 2017 at 06:53:45PM +0200, Pavel Hrdina wrote:
> Offline migration transfers only the domain definition.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449715
>
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

ACK

Erik

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