10:48 a.m.
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(a)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);
10:09 p.m.
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(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5:32 a.m.
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(a)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
8:36 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
12:46 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:56 p.m.
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
1:05 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:28 a.m.
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
11:54 a.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:53 a.m.
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:50 p.m.
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(a)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
1:20 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:53 p.m.
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
4971
days inactive
4976
days old
12 comments
3 participants
participants (3)
-
Daniel Veillard -
Eric Blake -
Stefan Berger