[libvirt] [PATCH] migrate VMs between different-endian hosts
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host. I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include <sys/wait.h> #include <sys/ioctl.h> #include <sys/un.h> +#include <byteswap.h> #include "qemu_driver.h" @@ -1881,13 +1882,22 @@ VIR_ENUM_IMPL(qemudSaveCompression, QEMU struct qemud_save_header { char magic[sizeof(QEMUD_SAVE_MAGIC)-1]; - int version; - int xml_len; - int was_running; - int compressed; - int unused[15]; + uint32_t version; + uint32_t xml_len; + uint32_t was_running; + uint32_t compressed; + uint32_t unused[15]; }; +static inline void +bswap_header(struct qemud_save_header *hdr) { + hdr->version = bswap_32(hdr->version); + hdr->xml_len = bswap_32(hdr->xml_len); + hdr->was_running = bswap_32(hdr->was_running); + hdr->compressed = bswap_32(hdr->compressed); +} + + /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, char *xml, @@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri } if (header.version > QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } + + if (header.version > QEMUD_SAVE_VERSION) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d > %d)"), header.version, QEMUD_SAVE_VERSION);
On Sat, Apr 09, 2011 at 11:48:41AM -0400, Stefan Berger wrote:
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host.
OMG, there is really a use case for this :-) ?
I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include <sys/wait.h> #include <sys/ioctl.h> #include <sys/un.h> +#include <byteswap.h>
#include "qemu_driver.h" @@ -1881,13 +1882,22 @@ VIR_ENUM_IMPL(qemudSaveCompression, QEMU
struct qemud_save_header { char magic[sizeof(QEMUD_SAVE_MAGIC)-1]; - int version; - int xml_len; - int was_running; - int compressed; - int unused[15]; + uint32_t version; + uint32_t xml_len; + uint32_t was_running; + uint32_t compressed; + uint32_t unused[15]; };
+static inline void +bswap_header(struct qemud_save_header *hdr) { + hdr->version = bswap_32(hdr->version); + hdr->xml_len = bswap_32(hdr->xml_len); + hdr->was_running = bswap_32(hdr->was_running); + hdr->compressed = bswap_32(hdr->compressed); +} + + /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, char *xml, @@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version > QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + }
Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-)
+ if (header.version > QEMUD_SAVE_VERSION) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d > %d)"), header.version, QEMUD_SAVE_VERSION);
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 04/10/2011 11:09 PM, Daniel Veillard wrote:
On Sat, Apr 09, 2011 at 11:48:41AM -0400, Stefan Berger wrote:
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host. OMG, there is really a use case for this :-) ? :-) There may be other architectures that run more efficiently than an x86 one. Btw, my use case is 'testing'. I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux.
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include<sys/wait.h> #include<sys/ioctl.h> #include<sys/un.h> +#include<byteswap.h>
#include "qemu_driver.h" @@ -1881,13 +1882,22 @@ VIR_ENUM_IMPL(qemudSaveCompression, QEMU
struct qemud_save_header { char magic[sizeof(QEMUD_SAVE_MAGIC)-1]; - int version; - int xml_len; - int was_running; - int compressed; - int unused[15]; + uint32_t version; + uint32_t xml_len; + uint32_t was_running; + uint32_t compressed; + uint32_t unused[15]; };
+static inline void +bswap_header(struct qemud_save_header *hdr) { + hdr->version = bswap_32(hdr->version); + hdr->xml_len = bswap_32(hdr->xml_len); + hdr->was_running = bswap_32(hdr->was_running); + hdr->compressed = bswap_32(hdr->compressed); +} + + /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, char *xml, @@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted.
Stefan
+ if (header.version> QEMUD_SAVE_VERSION) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d> %d)"), header.version, QEMUD_SAVE_VERSION); Daniel
On Mon, Apr 11, 2011 at 06:32:21AM -0400, Stefan Berger wrote:
On 04/10/2011 11:09 PM, Daniel Veillard wrote:
On Sat, Apr 09, 2011 at 11:48:41AM -0400, Stefan Berger wrote:
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host. OMG, there is really a use case for this :-) ? :-) There may be other architectures that run more efficiently than an x86 one.
Haha, no doubt :-)
Btw, my use case is 'testing'.
Okay, I was wondering, thanks !
@@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted.
yeah, I understand, okay, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 04/11/2011 07:36 AM, Daniel Veillard wrote:
@@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted.
yeah, I understand, okay,
Should we be writing the header in a particular byte order, regardless of host endianness? Or does that require bumping the header version to 3 anyways? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/11/2011 01:46 PM, Eric Blake wrote:
On 04/11/2011 07:36 AM, Daniel Veillard wrote:
@@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted. yeah, I understand, okay, Should we be writing the header in a particular byte order, regardless of host endianness? Or does that require bumping the header version to 3 anyways?
Would version 3 then simply mean to convert to version-3-standard-header-endianess and anything below just would fail if the hosts had different endianess ? No matter what, we'll have to have code to convert the version indicator alone. Stefan
On Mon, Apr 11, 2011 at 11:46:55AM -0600, Eric Blake wrote:
On 04/11/2011 07:36 AM, Daniel Veillard wrote:
@@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted.
yeah, I understand, okay,
Should we be writing the header in a particular byte order, regardless of host endianness? Or does that require bumping the header version to 3 anyways?
The thing I'm worried about is that by bumping older versions won't be able to restore the new dumps, and that could be a serious issue on shared storage. We are not introducing a new feature, so I thin we need to preserve forward compatibility. The patch as is is minimal but achieves it. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 04/12/2011 02:05 AM, Daniel Veillard wrote:
On Mon, Apr 11, 2011 at 11:46:55AM -0600, Eric Blake wrote:
On 04/11/2011 07:36 AM, Daniel Veillard wrote:
@@ -3097,6 +3107,11 @@ qemuDomainSaveImageOpen(struct qemud_dri }
if (header.version> QEMUD_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(&header); + } Hum, isn't there a more reliable way to detect the change of endianness ? That's a bit fishy IMHO :-) The problem is that the header should not have been written in a hosts' native format. So what can go wrong? QEMUD_SAVE_VERSION is '2'. Either we find 1 or 2 here and go ahead and accept it 'as-is'. Otherwise anything bigger than 3 is not accepted and swapped. 3 then becomes 0x03 00 00 00 and is discarded. 0x 02 00 00 00 would be swapped to '2' and accepted. yeah, I understand, okay, Should we be writing the header in a particular byte order, regardless of host endianness? Or does that require bumping the header version to 3 anyways? The thing I'm worried about is that by bumping older versions won't be able to restore the new dumps, and that could be a serious issue on shared storage. We are not introducing a new feature, so I thin we need to preserve forward compatibility. The patch as is is minimal but achieves it.
Do you want to accept the patch ? Stefan
On 04/14/2011 06:28 AM, Stefan Berger wrote:
The thing I'm worried about is that by bumping older versions won't be able to restore the new dumps, and that could be a serious issue on shared storage. We are not introducing a new feature, so I thin we need to preserve forward compatibility. The patch as is is minimal but achieves it.
Do you want to accept the patch ?
See my conditional ACK to the actual patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/09/2011 09:48 AM, Stefan Berger wrote:
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host.
I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include <sys/wait.h> #include <sys/ioctl.h> #include <sys/un.h> +#include <byteswap.h>
This won't work unless you also modify bootstrap.conf to include the gnulib byteswap module. ACK with that nit fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/14/2011 12:53 PM, Eric Blake wrote:
On 04/09/2011 09:48 AM, Stefan Berger wrote:
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host.
I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux.
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- src/qemu/qemu_driver.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include<sys/wait.h> #include<sys/ioctl.h> #include<sys/un.h> +#include<byteswap.h> This won't work unless you also modify bootstrap.conf to include the gnulib byteswap module.
ACK with that nit fixed.
I don't think it's necessary to include it there: # rpm -q --whatprovides /usr/include/byteswap.h glibc-headers-2.12.90-21.x86_64 Stefan
On 04/14/2011 11:50 AM, Stefan Berger wrote:
+++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include<sys/wait.h> #include<sys/ioctl.h> #include<sys/un.h> +#include<byteswap.h> This won't work unless you also modify bootstrap.conf to include the gnulib byteswap module.
ACK with that nit fixed.
I don't think it's necessary to include it there:
# rpm -q --whatprovides /usr/include/byteswap.h glibc-headers-2.12.90-21.x86_64
Not all the world is glibc. BSD and mingw lack byteswap.h, and qemu compiles on more than just Linux. While I'm not sure if any non-Linux hosts default to compiling the qemu driver at the current moment, that does not mean that we should not be thinking about the portability aspects. Adding a one-liner inclusion of the byteswap module to bootstrap.conf will avoid any questions of portability pitfalls. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/14/2011 02:20 PM, Eric Blake wrote:
On 04/14/2011 11:50 AM, Stefan Berger wrote:
+++ libvirt-acl/src/qemu/qemu_driver.c @@ -43,6 +43,7 @@ #include<sys/wait.h> #include<sys/ioctl.h> #include<sys/un.h> +#include<byteswap.h> This won't work unless you also modify bootstrap.conf to include the gnulib byteswap module.
ACK with that nit fixed.
I don't think it's necessary to include it there:
# rpm -q --whatprovides /usr/include/byteswap.h glibc-headers-2.12.90-21.x86_64 Not all the world is glibc. BSD and mingw lack byteswap.h, and qemu compiles on more than just Linux. While I'm not sure if any non-Linux hosts default to compiling the qemu driver at the current moment, that does not mean that we should not be thinking about the portability aspects. Adding a one-liner inclusion of the byteswap module to bootstrap.conf will avoid any questions of portability pitfalls.
Push with that nit fixed. Stefan
participants (3)
-
Daniel Veillard -
Eric Blake -
Stefan Berger