Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 08:48:48 -0600, Eric Blake wrote: On 10/04/2011 07:48 AM, Jiri Denemark wrote: IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) Quick questions (from a latecomer to the thread): what happens if I use both the @dname and @dxml arguments? Are we properly requiring that the new name in both arguments match, and rejecting the migration as impossible otherwise (since you can't request two different names), or are we having one of the two names take priority over the other? These are actually very good questions :-) If dname is provided, it overrides the name from XML (no matter if that's from GetXMLDesc or dxml). Also, if @dname is NULL but @dxml is provided, I think that we currently refuse migration to a server that only understands v2 migration (since only v3 can take @dxml). Can @dxml in isolation be used to change the name, without the use of @dname? However, if dname is not provided by dxml is and it contains domain name which is different, the destination libvirtd will have the name from dxml while source libvirtd will use the original name and migration will not work if those names do not match. But we don't currently have an explicit check for dxml name. I'm thinking about requiring the name to match either original name or dname (if set). Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On 10/05/2011 02:29 AM, Jiri Denemark wrote: Quick questions (from a latecomer to the thread): what happens if I use both the @dname and @dxml arguments? Are we properly requiring that the new name in both arguments match, and rejecting the migration as impossible otherwise (since you can't request two different names), or are we having one of the two names take priority over the other? These are actually very good questions :-) If dname is provided, it overrides the name from XML (no matter if that's from GetXMLDesc or dxml). Sounds like that is worth documenting. Also, if @dname is NULL but @dxml is provided, I think that we currently refuse migration to a server that only understands v2 migration (since only v3 can take @dxml). Can @dxml in isolation be used to change the name, without the use of @dname? However, if dname is not provided by dxml is and it contains domain name which is different, the destination libvirtd will have the name from dxml while source libvirtd will use the original name and migration will not work if those names do not match. But we don't currently have an explicit check for dxml name. I'm thinking about requiring the name to match either original name or dname (if set). Yeah, after more thought, I think the following semantics are worth enforcing: no dname or dxml - remote name is unchanged dname but no dxml - remote name becomes dname dname present, dxml with no name change - valid, remote uses dname dname present, dxml with name change - dxml MUST match dname dxml with no name change, no dname - remote name is unchanged dxml with name change, no dname - remote name changes to dxml (*) (*) if we can't easily change destination name using just dxml, then we could change this case to instead be an error, on the grounds that only dname can change the remote name (but once dname is present to change the name, then dxml can match either source or dname) -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix migration with dname
Destination libvirtd remembers the original name in the prepare phase
and clears it in the finish phase. The original name is used when
comparing domain name in migration cookie.
---
Notes:
Originally, I wanted to transfer the new name in migration cookie but
that appeared to be much more complicated and it would require adding
new Confirm API since the current version does not have 'dname'
parameter.
src/qemu/qemu_domain.c|1 +
src/qemu/qemu_domain.h|1 +
src/qemu/qemu_migration.c | 19 ---
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3ad192..65f721a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -231,6 +231,7 @@ static void qemuDomainObjPrivateFree(void *data)
qemuDomainObjFreeJob(priv);
VIR_FREE(priv-vcpupids);
VIR_FREE(priv-lockState);
+VIR_FREE(priv-origname);
/* This should never be non-NULL if we get here, but just in case... */
if (priv-mon) {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cdf1375..d9f323c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -126,6 +126,7 @@ struct _qemuDomainObjPrivate {
int jobs_queued;
unsigned long migMaxBandwidth;
+char *origname;
};
struct qemuDomainWatchdogEvent
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1122dab..4516231 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -254,12 +254,18 @@ error:
static qemuMigrationCookiePtr
qemuMigrationCookieNew(virDomainObjPtr dom)
{
+qemuDomainObjPrivatePtr priv = dom-privateData;
qemuMigrationCookiePtr mig = NULL;
+const char *name;
if (VIR_ALLOC(mig) 0)
goto no_memory;
-if (!(mig-name = strdup(dom-def-name)))
+if (priv-origname)
+name = priv-origname;
+else
+name = dom-def-name;
+if (!(mig-name = strdup(name)))
goto no_memory;
memcpy(mig-uuid, dom-def-uuid, VIR_UUID_BUFLEN);
@@ -1064,6 +1070,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
unsigned long long now;
qemuMigrationCookiePtr mig = NULL;
bool tunnel = !!st;
+char *origname = NULL;
if (virTimeMs(now) 0)
return -1;
@@ -1078,7 +1085,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
/* Target domain name, maybe renamed. */
if (dname) {
-VIR_FREE(def-name);
+origname = def-name;
def-name = strdup(dname);
if (def-name == NULL)
goto cleanup;
@@ -1095,6 +1102,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
}
def = NULL;
priv = vm-privateData;
+priv-origname = origname;
+origname = NULL;
if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
QEMU_MIGRATION_COOKIE_LOCKSTATE)))
@@ -1175,6 +1184,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
ret = 0;
cleanup:
+VIR_FREE(origname);
virDomainDefFree(def);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
@@ -2542,6 +2552,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
qemuMigrationCookiePtr mig = NULL;
virErrorPtr orig_err = NULL;
int cookie_flags = 0;
+qemuDomainObjPrivatePtr priv = vm-privateData;
VIR_DEBUG(driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d,
cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d,
@@ -2695,8 +2706,10 @@ endjob:
}
cleanup:
-if (vm)
+if (vm) {
+VIR_FREE(priv-origname);
virDomainObjUnlock(vm);
+}
if (event)
qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
--
1.7.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? --- Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? ACK, if my two questions here are correct 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 14:03:34 +0100, Daniel P. Berrange wrote: On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? It's actually the Prepare method that fails but the error is the same (obviously since it comes from EatCookie). Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 15:16:28 +0200, Jiri Denemark wrote: On Tue, Oct 04, 2011 at 14:03:34 +0100, Daniel P. Berrange wrote: On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? It's actually the Prepare method that fails but the error is the same (obviously since it comes from EatCookie). Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) So I pushed this, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On 10/04/2011 07:48 AM, Jiri Denemark wrote: IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) Quick questions (from a latecomer to the thread): what happens if I use both the @dname and @dxml arguments? Are we properly requiring that the new name in both arguments match, and rejecting the migration as impossible otherwise (since you can't request two different names), or are we having one of the two names take priority over the other? Also, if @dname is NULL but @dxml is provided, I think that we currently refuse migration to a server that only understands v2 migration (since only v3 can take @dxml). Can @dxml in isolation be used to change the name, without the use of @dname? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
