diff mbox

slirp: Fix migration from older versions of QEMU to the current one

Message ID 1459435721-21351-1-git-send-email-thuth@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Huth March 31, 2016, 2:48 p.m. UTC
While adding the IPv6 support, the commit eae303ff23f51259eddc8856c71453d8
("slirp: Make Socket structure IPv6 compatible") changed the format of
the migration stream, without taking into account that we might still
receive an old migration stream layout when upgrading from QEMU version
2.5 (or older) to QEMU 2.6. Currently, QEMU bails out when doing a
migration from QEMU 2.5 to the recent master version when it has
been started with a "-net user,guestfwd=..." network. So let's fix
this by checking the version ID of the migration stream and by using
the old behavior if we've detected version 3 or less.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 slirp/slirp.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

Comments

Samuel Thibault March 31, 2016, 10 p.m. UTC | #1
Thomas Huth, on Thu 31 Mar 2016 16:48:41 +0200, wrote:
> While adding the IPv6 support, the commit eae303ff23f51259eddc8856c71453d8
> ("slirp: Make Socket structure IPv6 compatible") changed the format of
> the migration stream, without taking into account that we might still
> receive an old migration stream layout when upgrading from QEMU version
> 2.5 (or older) to QEMU 2.6. Currently, QEMU bails out when doing a
> migration from QEMU 2.5 to the recent master version when it has
> been started with a "-net user,guestfwd=..." network. So let's fix
> this by checking the version ID of the migration stream and by using
> the old behavior if we've detected version 3 or less.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Just realizing... We'd need to add AF_INET6 cases here too, to be able
to save/restore a VM using ipv6 connections.

> ---
>  slirp/slirp.c | 44 ++++++++++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 3481fcc..998f278 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -1233,31 +1233,39 @@ static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
>      return 0;
>  }
>  
> -static int slirp_socket_load(QEMUFile *f, struct socket *so)
> +static int slirp_socket_load(QEMUFile *f, struct socket *so, int version_id)
>  {
>      if (tcp_attach(so) < 0)
>          return -ENOMEM;
>  
>      so->so_urgc = qemu_get_be32(f);
> -    so->so_ffamily = qemu_get_be16(f);
> -    switch (so->so_ffamily) {
> -    case AF_INET:
> +    if (version_id <= 3) {
> +        so->so_ffamily = AF_INET;
>          so->so_faddr.s_addr = qemu_get_be32(f);
> -        so->so_fport = qemu_get_be16(f);
> -        break;
> -    default:
> -        error_report(
> -                "so_ffamily unknown, unable to restore so_faddr and so_lport\n");
> -    }
> -    so->so_lfamily = qemu_get_be16(f);
> -    switch (so->so_lfamily) {
> -    case AF_INET:
>          so->so_laddr.s_addr = qemu_get_be32(f);
> +        so->so_fport = qemu_get_be16(f);
>          so->so_lport = qemu_get_be16(f);
> -        break;
> -    default:
> -        error_report(
> -                "so_ffamily unknown, unable to restore so_laddr and so_lport\n");
> +    } else {
> +        so->so_ffamily = qemu_get_be16(f);
> +        switch (so->so_ffamily) {
> +        case AF_INET:
> +            so->so_faddr.s_addr = qemu_get_be32(f);
> +            so->so_fport = qemu_get_be16(f);
> +            break;
> +        default:
> +            error_report(
> +                "so_ffamily unknown, unable to restore so_faddr and so_lport");
> +        }
> +        so->so_lfamily = qemu_get_be16(f);
> +        switch (so->so_lfamily) {
> +        case AF_INET:
> +            so->so_laddr.s_addr = qemu_get_be32(f);
> +            so->so_lport = qemu_get_be16(f);
> +            break;
> +        default:
> +            error_report(
> +                "so_ffamily unknown, unable to restore so_laddr and so_lport");
> +        }
>      }
>      so->so_iptos = qemu_get_byte(f);
>      so->so_emu = qemu_get_byte(f);
> @@ -1294,7 +1302,7 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
>          if (!so)
>              return -ENOMEM;
>  
> -        ret = slirp_socket_load(f, so);
> +        ret = slirp_socket_load(f, so, version_id);
>  
>          if (ret < 0)
>              return ret;
> -- 
> 1.8.3.1
>
Samuel Thibault March 31, 2016, 10:22 p.m. UTC | #2
Samuel Thibault, on Fri 01 Apr 2016 00:00:43 +0200, wrote:
> Just realizing... We'd need to add AF_INET6 cases here too, to be able
> to save/restore a VM using ipv6 connections.

That seems quite involved however, maybe we should postpone that to
later?

Samuel
Thomas Huth April 1, 2016, 8 a.m. UTC | #3
On 01.04.2016 00:22, Samuel Thibault wrote:
> Samuel Thibault, on Fri 01 Apr 2016 00:00:43 +0200, wrote:
>> Just realizing... We'd need to add AF_INET6 cases here too, to be able
>> to save/restore a VM using ipv6 connections.
> 
> That seems quite involved however, maybe we should postpone that to
> later?

I think it's ok to do it later. The code there is only invoked when the
user specified the "guestfwd=..." option, so that's likely rather a rare
case, I think, which could be postponed to QEMU 2.7.

 Thomas
diff mbox

Patch

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 3481fcc..998f278 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1233,31 +1233,39 @@  static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
     return 0;
 }
 
-static int slirp_socket_load(QEMUFile *f, struct socket *so)
+static int slirp_socket_load(QEMUFile *f, struct socket *so, int version_id)
 {
     if (tcp_attach(so) < 0)
         return -ENOMEM;
 
     so->so_urgc = qemu_get_be32(f);
-    so->so_ffamily = qemu_get_be16(f);
-    switch (so->so_ffamily) {
-    case AF_INET:
+    if (version_id <= 3) {
+        so->so_ffamily = AF_INET;
         so->so_faddr.s_addr = qemu_get_be32(f);
-        so->so_fport = qemu_get_be16(f);
-        break;
-    default:
-        error_report(
-                "so_ffamily unknown, unable to restore so_faddr and so_lport\n");
-    }
-    so->so_lfamily = qemu_get_be16(f);
-    switch (so->so_lfamily) {
-    case AF_INET:
         so->so_laddr.s_addr = qemu_get_be32(f);
+        so->so_fport = qemu_get_be16(f);
         so->so_lport = qemu_get_be16(f);
-        break;
-    default:
-        error_report(
-                "so_ffamily unknown, unable to restore so_laddr and so_lport\n");
+    } else {
+        so->so_ffamily = qemu_get_be16(f);
+        switch (so->so_ffamily) {
+        case AF_INET:
+            so->so_faddr.s_addr = qemu_get_be32(f);
+            so->so_fport = qemu_get_be16(f);
+            break;
+        default:
+            error_report(
+                "so_ffamily unknown, unable to restore so_faddr and so_lport");
+        }
+        so->so_lfamily = qemu_get_be16(f);
+        switch (so->so_lfamily) {
+        case AF_INET:
+            so->so_laddr.s_addr = qemu_get_be32(f);
+            so->so_lport = qemu_get_be16(f);
+            break;
+        default:
+            error_report(
+                "so_ffamily unknown, unable to restore so_laddr and so_lport");
+        }
     }
     so->so_iptos = qemu_get_byte(f);
     so->so_emu = qemu_get_byte(f);
@@ -1294,7 +1302,7 @@  static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
         if (!so)
             return -ENOMEM;
 
-        ret = slirp_socket_load(f, so);
+        ret = slirp_socket_load(f, so, version_id);
 
         if (ret < 0)
             return ret;