[libvirt] [PATCH] qemu: Properly rename persistent def after migration
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations. Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too. https://bugzilla.redhat.com/show_bug.cgi?id=1076354 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error; + if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT && + mig->persistent && + STRNEQ(dom->def->name, mig->persistent->name)) { + VIR_FREE(mig->persistent->name); + if (VIR_STRDUP(mig->persistent->name, dom->def->name) < 0) + goto error; + } + if (mig->flags & QEMU_MIGRATION_COOKIE_LOCKSTATE) { if (!mig->lockDriver) { if (virLockManagerPluginUsesState(driver->lockManager)) { -- 2.4.0
On Mon, May 04, 2015 at 11:02:18PM +0200, Jiri Denemark wrote:
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations.
Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too.
https://bugzilla.redhat.com/show_bug.cgi?id=1076354
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT &&
Shouldn't you rather use mig->flags here? Then you don't have to check for mig->persistent down here either (but that doesn't hurt, of course).
+ mig->persistent && + STRNEQ(dom->def->name, mig->persistent->name)) { + VIR_FREE(mig->persistent->name); + if (VIR_STRDUP(mig->persistent->name, dom->def->name) < 0) + goto error; + } + if (mig->flags & QEMU_MIGRATION_COOKIE_LOCKSTATE) { if (!mig->lockDriver) { if (virLockManagerPluginUsesState(driver->lockManager)) { -- 2.4.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 05, 2015 at 08:30:30 +0200, Martin Kletzander wrote:
On Mon, May 04, 2015 at 11:02:18PM +0200, Jiri Denemark wrote:
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations.
Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too.
https://bugzilla.redhat.com/show_bug.cgi?id=1076354
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT &&
Shouldn't you rather use mig->flags here?
Yeah, perhaps, after fixing the code to properly update mig->flags when persistent definition is parsed from the cookie :-) Jirka
On Tue, May 05, 2015 at 08:50:10 +0200, Jiri Denemark wrote:
On Tue, May 05, 2015 at 08:30:30 +0200, Martin Kletzander wrote:
On Mon, May 04, 2015 at 11:02:18PM +0200, Jiri Denemark wrote:
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations.
Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too.
https://bugzilla.redhat.com/show_bug.cgi?id=1076354
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT &&
Shouldn't you rather use mig->flags here?
Yeah, perhaps, after fixing the code to properly update mig->flags when persistent definition is parsed from the cookie :-)
After looking at the code once more, I'd rather avoid changing this. The mig object created by qemuMigrationEatCookie is then reused by qemuMigrationBakeCookie. Thus, if we changed the XML cookie parser to set QEMU_MIGRATION_COOKIE_PERSISTENT in mig->flags when it parses the persistent definition, it would then be also formated into the outgoing cookie. This is all weired but I think using just flags is safer than touching the way cookies are processed. Jirka
On Tue, May 05, 2015 at 09:10:30AM +0200, Jiri Denemark wrote:
On Tue, May 05, 2015 at 08:50:10 +0200, Jiri Denemark wrote:
On Tue, May 05, 2015 at 08:30:30 +0200, Martin Kletzander wrote:
On Mon, May 04, 2015 at 11:02:18PM +0200, Jiri Denemark wrote:
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations.
Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too.
https://bugzilla.redhat.com/show_bug.cgi?id=1076354
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT &&
Shouldn't you rather use mig->flags here?
Yeah, perhaps, after fixing the code to properly update mig->flags when persistent definition is parsed from the cookie :-)
After looking at the code once more, I'd rather avoid changing this. The mig object created by qemuMigrationEatCookie is then reused by qemuMigrationBakeCookie. Thus, if we changed the XML cookie parser to set QEMU_MIGRATION_COOKIE_PERSISTENT in mig->flags when it parses the persistent definition, it would then be also formated into the outgoing cookie. This is all weired but I think using just flags is safer than touching the way cookies are processed.
OK then, ACK as-is. It looks like it needs more cleaning up and simply fixing this is enough for now. Martin
On Tue, May 05, 2015 at 09:14:39 +0200, Martin Kletzander wrote:
On Tue, May 05, 2015 at 09:10:30AM +0200, Jiri Denemark wrote:
On Tue, May 05, 2015 at 08:50:10 +0200, Jiri Denemark wrote:
On Tue, May 05, 2015 at 08:30:30 +0200, Martin Kletzander wrote:
On Mon, May 04, 2015 at 11:02:18PM +0200, Jiri Denemark wrote:
When migrating a domain while changing its name and using VIR_MIGRATE_PERSIST_DEST flag, libvirt would fail to properly change the name in the persistent definition. The inconsistency results in weird behavior when dumping domain XML, destroying the domain, restarting libvirtd and likely in several other situations.
Since the new name is already stored in vm->def->name, we just need to make sure the persistent definition uses this new name too.
https://bugzilla.redhat.com/show_bug.cgi?id=1076354
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20c2193..c1af704 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1392,6 +1392,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, flags) < 0) goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT &&
Shouldn't you rather use mig->flags here?
Yeah, perhaps, after fixing the code to properly update mig->flags when persistent definition is parsed from the cookie :-)
After looking at the code once more, I'd rather avoid changing this. The mig object created by qemuMigrationEatCookie is then reused by qemuMigrationBakeCookie. Thus, if we changed the XML cookie parser to set QEMU_MIGRATION_COOKIE_PERSISTENT in mig->flags when it parses the persistent definition, it would then be also formated into the outgoing cookie. This is all weired but I think using just flags is safer than touching the way cookies are processed.
OK then, ACK as-is. It looks like it needs more cleaning up and simply fixing this is enough for now.
Pushed, thanks. Jirka
participants (2)
-
Jiri Denemark -
Martin Kletzander