[PATCH 0/2] Make savevm versioning compatible with upstream QEMU

[PATCH 0/2] Make savevm versioning compatible with upstream QEMU

[Anthony Liguori]

Right now, there is no way savevm versioning can be compatible with upstream
QEMU because KVM adds fields to existing savevm structures without incrementing
the versions.

If you assume that KVM will eventually merge into upstream QEMU, this means that
eventually KVM is going to have to break backwards compatibility with itself
to resolve this issue in a non-graceful way.

So let's do that now instead of doing it later when the situation is only worse.

I'm happy to allocate particular version identifiers for KVM to avoid future
conflicts.  I believe we should try to eliminate the existing differences so
that we can converge in the future on a common versioning scheme.

[PATCH 1/2] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Anthony Liguori]

When TAP_VNET_HDR eventually merges into upstream QEMU, it cannot change the
format of the version 6 savevm data.  This means that we're going to have to
bump the version up to 7.  I'm happy to reserve version 7 as having TAP_VNET_HDR
support to allow time to include this support in upstream QEMU.

For those shipping products based on KVM though, it's important that we do not
conflict with upstream QEMU versioning or else it's going to result in breakage
of backwards compatibility.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/virtio-net.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5f5f2f3..ac8e030 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -21,7 +21,12 @@
 
 #define TAP_VNET_HDR
 
-#define VIRTIO_NET_VM_VERSION    6
+/* Version 7 has TAP_VNET_HDR support.  This is reserved in upstream QEMU to
+ * avoid future conflict.
+ * We can't assume verisons > 7 have TAP_VNET_HDR support until this is merged
+ * in upstream QEMU.
+ */
+#define VIRTIO_NET_VM_VERSION    7
 
 #define MAC_TABLE_ENTRIES    32
 #define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
@@ -652,8 +657,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
 
 #ifdef TAP_VNET_HDR
-    if (qemu_get_be32(f))
+    if (version_id == 7 && qemu_get_be32(f)) {
         tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
+    }
 #endif
 
     if (n->tx_timer_active) {
-- 
1.6.0.6

Re: [PATCH 1/2] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Alex Williamson]

On Wed, 2009-04-29 at 15:53 -0500, Anthony Liguori wrote: 
> -#define VIRTIO_NET_VM_VERSION    6
> +/* Version 7 has TAP_VNET_HDR support.  This is reserved in upstream QEMU to
> + * avoid future conflict.
> + * We can't assume verisons > 7 have TAP_VNET_HDR support until this is merged
> + * in upstream QEMU.
> + */
> +#define VIRTIO_NET_VM_VERSION    7

It seems like you're saying you're only going to reserve version number
7, and not the 4 bytes of savevm we're using for version 7 here.
Couldn't we fix this by adding a dummy patch to qemu to bump to version
7, and push/pop a 4 byte zero from the savevm?  Then we could change the
code below to >= 7.  Qemu should probably puke on a savevm image with
non-zero in this location until the kvm code gets merged.  Looks like
one byte would be more than sufficient if we wanted to make that change
now too.  Thanks,

Alex
 
>  #define MAC_TABLE_ENTRIES    32
>  #define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
> @@ -652,8 +657,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>          qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
>  
>  #ifdef TAP_VNET_HDR
> -    if (qemu_get_be32(f))
> +    if (version_id == 7 && qemu_get_be32(f)) {
>          tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
> +    }
>  #endif
>  
>      if (n->tx_timer_active) {

Re: [PATCH 1/2] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Anthony Liguori]

Alex Williamson wrote:
> On Wed, 2009-04-29 at 15:53 -0500, Anthony Liguori wrote: 
>   
>> -#define VIRTIO_NET_VM_VERSION    6
>> +/* Version 7 has TAP_VNET_HDR support.  This is reserved in upstream QEMU to
>> + * avoid future conflict.
>> + * We can't assume verisons > 7 have TAP_VNET_HDR support until this is merged
>> + * in upstream QEMU.
>> + */
>> +#define VIRTIO_NET_VM_VERSION    7
>>     
>
> It seems like you're saying you're only going to reserve version number
> 7, and not the 4 bytes of savevm we're using for version 7 here.
> Couldn't we fix this by adding a dummy patch to qemu to bump to version
> 7, and push/pop a 4 byte zero from the savevm?  Then we could change the
> code below to >= 7.  Qemu should probably puke on a savevm image with
> non-zero in this location until the kvm code gets merged.  Looks like
> one byte would be more than sufficient if we wanted to make that change
> now too.  Thanks,
>   

I'd rather just merge vnet into upstream QEMU as quickly as possible.  
All I have to do to reserve a field is just hope noone submits a patch 
incrementing version id until we submit vnet support :-)

--
Regards,

Anthony Liguori

[PATCH 2/2] Increment version id for CPU save state

[Anthony Liguori]

9 is reserved for KVM.  KVM cannot support migration from any other version.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 target-i386/cpu.h     |    4 +++-
 target-i386/machine.c |    3 +++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f054af1..af0ee18 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -838,7 +838,9 @@ static inline int cpu_get_time_fast(void)
 #define cpu_signal_handler cpu_x86_signal_handler
 #define cpu_list x86_cpu_list
 
-#define CPU_SAVE_VERSION 8
+/* CPU_SAVE_VERSION 9 is reserved for KVM.  This is to avoid breakage as KVM
+ * merges into upstream QEMU */
+#define CPU_SAVE_VERSION 9
 
 /* MMU modes definitions */
 #define MMU_MODE0_SUFFIX _kernel
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 399204d..7f75d31 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -196,6 +196,9 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     if (version_id != 3 && version_id != 4 && version_id != 5
         && version_id != 6 && version_id != 7 && version_id != 8)
         return -EINVAL;
+    /* KVM cannot accept migrations from QEMU today */
+    if (version_id != 9)
+        return -EINVAL;
     for(i = 0; i < CPU_NB_REGS; i++)
         qemu_get_betls(f, &env->regs[i]);
     qemu_get_betls(f, &env->eip);
-- 
1.6.0.6

Re: [PATCH 0/2] Make savevm versioning compatible with upstream QEMU

[Avi Kivity]

Anthony Liguori wrote:
> Right now, there is no way savevm versioning can be compatible with upstream
> QEMU because KVM adds fields to existing savevm structures without incrementing
> the versions.
>
> If you assume that KVM will eventually merge into upstream QEMU, this means that
> eventually KVM is going to have to break backwards compatibility with itself
> to resolve this issue in a non-graceful way.
>
> So let's do that now instead of doing it later when the situation is only worse.
>
> I'm happy to allocate particular version identifiers for KVM to avoid future
> conflicts.  I believe we should try to eliminate the existing differences so
> that we can converge in the future on a common versioning scheme.
>   

Applied both, thanks.

I think we can avoid the need to synchronize too much by saving 
kvm-specific state for device "x" using id "x-kvm"; this allows the two 
to evolve independently.  Of course it's much better to avoid divergence 
in the first place, but this isn't always possible.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/2] Make savevm versioning compatible with upstream QEMU

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> Right now, there is no way savevm versioning can be compatible with 
>> upstream
>> QEMU because KVM adds fields to existing savevm structures without 
>> incrementing
>> the versions.
>>
>> If you assume that KVM will eventually merge into upstream QEMU, this 
>> means that
>> eventually KVM is going to have to break backwards compatibility with 
>> itself
>> to resolve this issue in a non-graceful way.
>>
>> So let's do that now instead of doing it later when the situation is 
>> only worse.
>>
>> I'm happy to allocate particular version identifiers for KVM to avoid 
>> future
>> conflicts.  I believe we should try to eliminate the existing 
>> differences so
>> that we can converge in the future on a common versioning scheme.
>>   
>
> Applied both, thanks.
>
> I think we can avoid the need to synchronize too much by saving 
> kvm-specific state for device "x" using id "x-kvm"; this allows the 
> two to evolve independently.

I need to add save/restore support to upstream QEMU so this is a good 
excuse to just merge the changes in KVM upstream.  So hopefully this 
will become a non issue.  If something arises and you need more savevm 
state, introduce a new section suffixed or prefixed with kvm.  
Alternatively, ask and I can reserve an ID upstream.

For virtio-net, we just need to get the vnet stuff merged upstream.

-- 
Regards,

Anthony Liguori