Skip to contentSkip to conversation

SLIRP VMStatification

PATCH Superseded Dr. David Alan Gilbert (git) 6 participants 25 messages Feb 20, 2017 – Feb 28, 2017

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Dr. David Alan Gilbert (git)

author
#0 lore raw

v4 cover SLIRP VMStatification

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
This is an update to my previous SLIRP vmstatification, but finally
the dependent patches have gone in, so this can now follow.

Dave

v4
Fix 'VMStatify socket level' for mingw64 build; it has some different choices for the type of IPv4 addresses.

Dr. David Alan Gilbert (5):
slirp: VMState conversion; tcpcb
slirp: VMStatify sbuf
slirp: Common lhost/fhost union
slirp: VMStatify socket level
slirp: VMStatify remaining except for loop

 slirp/sbuf.h    |   4 +-
 slirp/slirp.c   | 449 ++++++++++++++++++++++++++++----------------------------
 slirp/socket.h  |  24 ++-
 slirp/tcp_var.h |   6 +-
 4 files changed, 238 insertions(+), 245 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH v4 1/5 slirp: VMState conversion; tcpcb

From: "Dr. David Alan Gilbert" <[email protected]>

Convert the migration of the struct tcpcb to use a VMStateDescription, the rest of it will come later.

Mostly mechanical, except for conversion of some 'char' to uint8_t to ensure portability.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
Reviewed-by Juan Quintela <[email protected]>
---
 slirp/slirp.c   | 149 ++++++++++++++++++++------------------------------------
 slirp/tcp_var.h |   6 +--
 2 files changed, 57 insertions(+), 98 deletions(-)
slirp/slirp.c +54 -95
index 60539de..276d8cb 100644
@@ -1129,53 +1129,62 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int guest_port,
         tcp_output(sototcpcb(so));
 }
 
-static void slirp_tcp_save(QEMUFile *f, struct tcpcb *tp)
+static int slirp_tcp_post_load(void *opaque, int version)
 {
-    int i;
+    tcp_template((struct tcpcb *)opaque);
 
-    qemu_put_sbe16(f, tp->t_state);
-    for (i = 0; i < TCPT_NTIMERS; i++)
-        qemu_put_sbe16(f, tp->t_timer[i]);
-    qemu_put_sbe16(f, tp->t_rxtshift);
-    qemu_put_sbe16(f, tp->t_rxtcur);
-    qemu_put_sbe16(f, tp->t_dupacks);
-    qemu_put_be16(f, tp->t_maxseg);
-    qemu_put_sbyte(f, tp->t_force);
-    qemu_put_be16(f, tp->t_flags);
-    qemu_put_be32(f, tp->snd_una);
-    qemu_put_be32(f, tp->snd_nxt);
-    qemu_put_be32(f, tp->snd_up);
-    qemu_put_be32(f, tp->snd_wl1);
-    qemu_put_be32(f, tp->snd_wl2);
-    qemu_put_be32(f, tp->iss);
-    qemu_put_be32(f, tp->snd_wnd);
-    qemu_put_be32(f, tp->rcv_wnd);
-    qemu_put_be32(f, tp->rcv_nxt);
-    qemu_put_be32(f, tp->rcv_up);
-    qemu_put_be32(f, tp->irs);
-    qemu_put_be32(f, tp->rcv_adv);
-    qemu_put_be32(f, tp->snd_max);
-    qemu_put_be32(f, tp->snd_cwnd);
-    qemu_put_be32(f, tp->snd_ssthresh);
-    qemu_put_sbe16(f, tp->t_idle);
-    qemu_put_sbe16(f, tp->t_rtt);
-    qemu_put_be32(f, tp->t_rtseq);
-    qemu_put_sbe16(f, tp->t_srtt);
-    qemu_put_sbe16(f, tp->t_rttvar);
-    qemu_put_be16(f, tp->t_rttmin);
-    qemu_put_be32(f, tp->max_sndwnd);
-    qemu_put_byte(f, tp->t_oobflags);
-    qemu_put_byte(f, tp->t_iobc);
-    qemu_put_sbe16(f, tp->t_softerror);
-    qemu_put_byte(f, tp->snd_scale);
-    qemu_put_byte(f, tp->rcv_scale);
-    qemu_put_byte(f, tp->request_r_scale);
-    qemu_put_byte(f, tp->requested_s_scale);
-    qemu_put_be32(f, tp->ts_recent);
-    qemu_put_be32(f, tp->ts_recent_age);
-    qemu_put_be32(f, tp->last_ack_sent);
+    return 0;
 }
 
+static const VMStateDescription vmstate_slirp_tcp = {
+    .name = "slirp-tcp",
+    .version_id = 0,
+    .post_load = slirp_tcp_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT16(t_state, struct tcpcb),
+        VMSTATE_INT16_ARRAY(t_timer, struct tcpcb, TCPT_NTIMERS),
+        VMSTATE_INT16(t_rxtshift, struct tcpcb),
+        VMSTATE_INT16(t_rxtcur, struct tcpcb),
+        VMSTATE_INT16(t_dupacks, struct tcpcb),
+        VMSTATE_UINT16(t_maxseg, struct tcpcb),
+        VMSTATE_UINT8(t_force, struct tcpcb),
+        VMSTATE_UINT16(t_flags, struct tcpcb),
+        VMSTATE_UINT32(snd_una, struct tcpcb),
+        VMSTATE_UINT32(snd_nxt, struct tcpcb),
+        VMSTATE_UINT32(snd_up, struct tcpcb),
+        VMSTATE_UINT32(snd_wl1, struct tcpcb),
+        VMSTATE_UINT32(snd_wl2, struct tcpcb),
+        VMSTATE_UINT32(iss, struct tcpcb),
+        VMSTATE_UINT32(snd_wnd, struct tcpcb),
+        VMSTATE_UINT32(rcv_wnd, struct tcpcb),
+        VMSTATE_UINT32(rcv_nxt, struct tcpcb),
+        VMSTATE_UINT32(rcv_up, struct tcpcb),
+        VMSTATE_UINT32(irs, struct tcpcb),
+        VMSTATE_UINT32(rcv_adv, struct tcpcb),
+        VMSTATE_UINT32(snd_max, struct tcpcb),
+        VMSTATE_UINT32(snd_cwnd, struct tcpcb),
+        VMSTATE_UINT32(snd_ssthresh, struct tcpcb),
+        VMSTATE_INT16(t_idle, struct tcpcb),
+        VMSTATE_INT16(t_rtt, struct tcpcb),
+        VMSTATE_UINT32(t_rtseq, struct tcpcb),
+        VMSTATE_INT16(t_srtt, struct tcpcb),
+        VMSTATE_INT16(t_rttvar, struct tcpcb),
+        VMSTATE_UINT16(t_rttmin, struct tcpcb),
+        VMSTATE_UINT32(max_sndwnd, struct tcpcb),
+        VMSTATE_UINT8(t_oobflags, struct tcpcb),
+        VMSTATE_UINT8(t_iobc, struct tcpcb),
+        VMSTATE_INT16(t_softerror, struct tcpcb),
+        VMSTATE_UINT8(snd_scale, struct tcpcb),
+        VMSTATE_UINT8(rcv_scale, struct tcpcb),
+        VMSTATE_UINT8(request_r_scale, struct tcpcb),
+        VMSTATE_UINT8(requested_s_scale, struct tcpcb),
+        VMSTATE_UINT32(ts_recent, struct tcpcb),
+        VMSTATE_UINT32(ts_recent_age, struct tcpcb),
+        VMSTATE_UINT32(last_ack_sent, struct tcpcb),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
 {
     uint32_t off;

@@ -1218,7 +1227,7 @@ static void slirp_socket_save(QEMUFile *f, struct socket *so)
     qemu_put_be32(f, so->so_state);
     slirp_sbuf_save(f, &so->so_rcv);
     slirp_sbuf_save(f, &so->so_snd);
-    slirp_tcp_save(f, so->so_tcpcb);
+    vmstate_save_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
 }
 
 static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)

@@ -1254,54 +1263,6 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
     slirp_bootp_save(f, slirp);
 }
 
-static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp)
-{
-    int i;
-
-    tp->t_state = qemu_get_sbe16(f);
-    for (i = 0; i < TCPT_NTIMERS; i++)
-        tp->t_timer[i] = qemu_get_sbe16(f);
-    tp->t_rxtshift = qemu_get_sbe16(f);
-    tp->t_rxtcur = qemu_get_sbe16(f);
-    tp->t_dupacks = qemu_get_sbe16(f);
-    tp->t_maxseg = qemu_get_be16(f);
-    tp->t_force = qemu_get_sbyte(f);
-    tp->t_flags = qemu_get_be16(f);
-    tp->snd_una = qemu_get_be32(f);
-    tp->snd_nxt = qemu_get_be32(f);
-    tp->snd_up = qemu_get_be32(f);
-    tp->snd_wl1 = qemu_get_be32(f);
-    tp->snd_wl2 = qemu_get_be32(f);
-    tp->iss = qemu_get_be32(f);
-    tp->snd_wnd = qemu_get_be32(f);
-    tp->rcv_wnd = qemu_get_be32(f);
-    tp->rcv_nxt = qemu_get_be32(f);
-    tp->rcv_up = qemu_get_be32(f);
-    tp->irs = qemu_get_be32(f);
-    tp->rcv_adv = qemu_get_be32(f);
-    tp->snd_max = qemu_get_be32(f);
-    tp->snd_cwnd = qemu_get_be32(f);
-    tp->snd_ssthresh = qemu_get_be32(f);
-    tp->t_idle = qemu_get_sbe16(f);
-    tp->t_rtt = qemu_get_sbe16(f);
-    tp->t_rtseq = qemu_get_be32(f);
-    tp->t_srtt = qemu_get_sbe16(f);
-    tp->t_rttvar = qemu_get_sbe16(f);
-    tp->t_rttmin = qemu_get_be16(f);
-    tp->max_sndwnd = qemu_get_be32(f);
-    tp->t_oobflags = qemu_get_byte(f);
-    tp->t_iobc = qemu_get_byte(f);
-    tp->t_softerror = qemu_get_sbe16(f);
-    tp->snd_scale = qemu_get_byte(f);
-    tp->rcv_scale = qemu_get_byte(f);
-    tp->request_r_scale = qemu_get_byte(f);
-    tp->requested_s_scale = qemu_get_byte(f);
-    tp->ts_recent = qemu_get_be32(f);
-    tp->ts_recent_age = qemu_get_be32(f);
-    tp->last_ack_sent = qemu_get_be32(f);
-    tcp_template(tp);
-}
-
 static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
 {
     uint32_t off, sb_cc, sb_datalen;

@@ -1367,9 +1328,7 @@ static int slirp_socket_load(QEMUFile *f, struct socket *so, int version_id)
         return -ENOMEM;
     if (slirp_sbuf_load(f, &so->so_snd) < 0)
         return -ENOMEM;
-    slirp_tcp_load(f, so->so_tcpcb);
-
-    return 0;
+    return vmstate_load_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
 }
 
 static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
slirp/tcp_var.h +3 -3
index 0f8f187..895ef6d 100644
@@ -48,7 +48,7 @@ struct tcpcb {
 	short	t_rxtcur;		/* current retransmit value */
 	short	t_dupacks;		/* consecutive dup acks recd */
 	u_short	t_maxseg;		/* maximum segment size */
-	char	t_force;		/* 1 if forcing out a byte */
+	uint8_t t_force;		/* 1 if forcing out a byte */
 	u_short	t_flags;
 #define	TF_ACKNOW	0x0001		/* ack peer immediately */
 #define	TF_DELACK	0x0002		/* ack, but try to delay it */

@@ -109,8 +109,8 @@ struct tcpcb {
 	uint32_t max_sndwnd;		/* largest window peer has offered */
 
 /* out-of-band data */
-	char	t_oobflags;		/* have some */
-	char	t_iobc;			/* input character */
+	uint8_t	t_oobflags;		/* have some */
+	uint8_t	t_iobc;			/* input character */
 #define	TCPOOB_HAVEDATA	0x01
 #define	TCPOOB_HADDATA	0x02
 	short	t_softerror;		/* possible error not yet reported */
Signature · 1 line
2.9.3
author
#2 lore raw

PATCH v4 2/5 slirp: VMStatify sbuf

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Convert the sbuf structure to a VMStateDescription. Note this uses the VMSTATE_WITH_TMP mechanism to calculate and reload the offsets based on the pointers.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by David Gibson <[email protected]>
Reviewed-by Juan Quintela <[email protected]>
Acked-by Samuel Thibault <[email protected]>
---
 slirp/sbuf.h  |   4 +-
 slirp/slirp.c | 116 ++++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 78 insertions(+), 42 deletions(-)
slirp/sbuf.h +2 -2
index efcec39..a722ecb 100644
@@ -12,8 +12,8 @@
 #define sbspace(sb) ((sb)->sb_datalen - (sb)->sb_cc)
 
 struct sbuf {
-	u_int	sb_cc;		/* actual chars in buffer */
-	u_int	sb_datalen;	/* Length of data  */
+	uint32_t sb_cc;		/* actual chars in buffer */
+	uint32_t sb_datalen;	/* Length of data  */
 	char	*sb_wptr;	/* write pointer. points to where the next
 				 * bytes should be written in the sbuf */
 	char	*sb_rptr;	/* read pointer. points to where the next
slirp/slirp.c +76 -40

145 lines not on this page · raw message

Signature · 1 line
2.9.3
author
#3 lore raw

PATCH v4 3/5 slirp: Common lhost/fhost union

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

The socket structure has a pair of unions for lhost and fhost addresses; the unions are identical so split them out into a separate union declaration.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 slirp/socket.h | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
slirp/socket.h +8 -10
index 8feed2a..c1be77e 100644
@@ -15,6 +15,12 @@
  * Our socket structure
  */
 
+union slirp_sockaddr {
+    struct sockaddr_storage ss;
+    struct sockaddr_in sin;
+    struct sockaddr_in6 sin6;
+};
+
 struct socket {
   struct socket *so_next,*so_prev;      /* For a linked list of sockets */
 

@@ -31,22 +37,14 @@ struct socket {
   struct tcpiphdr *so_ti;	   /* Pointer to the original ti within
 				    * so_mconn, for non-blocking connections */
   int so_urgc;
-  union {   /* foreign host */
-      struct sockaddr_storage ss;
-      struct sockaddr_in sin;
-      struct sockaddr_in6 sin6;
-  } fhost;
+  union slirp_sockaddr fhost;      /* Foreign host */
 #define so_faddr fhost.sin.sin_addr
 #define so_fport fhost.sin.sin_port
 #define so_faddr6 fhost.sin6.sin6_addr
 #define so_fport6 fhost.sin6.sin6_port
 #define so_ffamily fhost.ss.ss_family
 
-  union {   /* local host */
-      struct sockaddr_storage ss;
-      struct sockaddr_in sin;
-      struct sockaddr_in6 sin6;
-  } lhost;
+  union slirp_sockaddr lhost;      /* Local host */
 #define so_laddr lhost.sin.sin_addr
 #define so_lport lhost.sin.sin_port
 #define so_laddr6 lhost.sin6.sin6_addr
Signature · 1 line
2.9.3
Reviewed-by on v4 3/5
#4 lore raw

Re: [Qemu-devel] [PATCH v4 3/5] slirp: Common lhost/fhost union

Quoted 7 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

The socket structure has a pair of unions for lhost and fhost addresses; the unions are identical so split them out into a separate union declaration.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Quoted 47 lines
---
 slirp/socket.h | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
slirp/socket.h +8 -10
index 8feed2a..c1be77e 100644
@@ -15,6 +15,12 @@
  * Our socket structure
  */
 
+union slirp_sockaddr {
+    struct sockaddr_storage ss;
+    struct sockaddr_in sin;
+    struct sockaddr_in6 sin6;
+};
+
 struct socket {
   struct socket *so_next,*so_prev;      /* For a linked list of sockets */
 

@@ -31,22 +37,14 @@ struct socket {
   struct tcpiphdr *so_ti;	   /* Pointer to the original ti within
 				    * so_mconn, for non-blocking connections */
   int so_urgc;
-  union {   /* foreign host */
-      struct sockaddr_storage ss;
-      struct sockaddr_in sin;
-      struct sockaddr_in6 sin6;
-  } fhost;
+  union slirp_sockaddr fhost;      /* Foreign host */
 #define so_faddr fhost.sin.sin_addr
 #define so_fport fhost.sin.sin_port
 #define so_faddr6 fhost.sin6.sin6_addr
 #define so_fport6 fhost.sin6.sin6_port
 #define so_ffamily fhost.ss.ss_family
 
-  union {   /* local host */
-      struct sockaddr_storage ss;
-      struct sockaddr_in sin;
-      struct sockaddr_in6 sin6;
-  } lhost;
+  union slirp_sockaddr lhost;      /* Local host */
 #define so_laddr lhost.sin.sin_addr
 #define so_lport lhost.sin.sin_port
 #define so_laddr6 lhost.sin6.sin6_addr
Reviewed-by on v4 3/5
#5 lore raw

Re: [Qemu-devel] [PATCH v4 3/5] slirp: Common lhost/fhost union

In reply to Dr. David Alan Gilbert (git)
Quoted 7 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

The socket structure has a pair of unions for lhost and fhost addresses; the unions are identical so split them out into a separate union declaration.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Juan Quintela <[email protected]>
author
#6 lore raw

PATCH v4 4/5 slirp: VMStatify socket level

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 slirp/slirp.c  | 170 ++++++++++++++++++++++++++++++---------------------------
 slirp/socket.h |   6 +-
 2 files changed, 93 insertions(+), 83 deletions(-)
slirp/slirp.c +90 -80
index 178c2b6..6583de8 100644
@@ -1250,40 +1250,99 @@ static const VMStateDescription vmstate_slirp_sbuf = {
     }
 };
 
+static bool slirp_older_than_v4(void *opaque, int version_id)
+{
+    return version_id < 4;
+}
 
-static void slirp_socket_save(QEMUFile *f, struct socket *so)
+static bool slirp_family_inet(void *opaque, int version_id)
 {
-    qemu_put_be32(f, so->so_urgc);
-    qemu_put_be16(f, so->so_ffamily);
-    switch (so->so_ffamily) {
-    case AF_INET:
-        qemu_put_be32(f, so->so_faddr.s_addr);
-        qemu_put_be16(f, so->so_fport);
-        break;
-    default:
-        error_report("so_ffamily unknown, unable to save so_faddr and"
-                     " so_fport");
-    }
-    qemu_put_be16(f, so->so_lfamily);
-    switch (so->so_lfamily) {
-    case AF_INET:
-        qemu_put_be32(f, so->so_laddr.s_addr);
-        qemu_put_be16(f, so->so_lport);
-        break;
-    default:
-        error_report("so_ffamily unknown, unable to save so_laddr and"
-                     " so_lport");
+    union slirp_sockaddr *ssa = (union slirp_sockaddr *)opaque;
+    return ssa->ss.ss_family == AF_INET;
+}
+
+static int slirp_socket_pre_load(void *opaque)
+{
+    struct socket *so = opaque;
+    if (tcp_attach(so) < 0) {
+        return -ENOMEM;
     }
-    qemu_put_byte(f, so->so_iptos);
-    qemu_put_byte(f, so->so_emu);
-    qemu_put_byte(f, so->so_type);
-    qemu_put_be32(f, so->so_state);
-    /* TODO: Build vmstate at this level */
-    vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_rcv, 0);
-    vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_snd, 0);
-    vmstate_save_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
+    /* Older versions don't load these fields */
+    so->so_ffamily = AF_INET;
+    so->so_lfamily = AF_INET;
+    return 0;
 }
 
+#ifndef _WIN32
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_UINT16(f, s)
+#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_UINT32_TEST(f, s, t)
+#else
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)
+/* Win uses u_long rather than uint32_t - but it's still 32bits long */
+#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_SINGLE_TEST(f, s, t, 0, \
+                                       vmstate_info_uint32, u_long)
+#endif
+
+static const VMStateDescription vmstate_slirp_socket_addr = {
+    .name = "slirp-socket-addr",
+    .version_id = 4,
+    .fields = (VMStateField[]) {
+        VMSTATE_SS_FAMILY(ss.ss_family, union slirp_sockaddr),
+        VMSTATE_SIN4_ADDR(sin.sin_addr.s_addr, union slirp_sockaddr,
+                            slirp_family_inet),
+        VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr,
+                            slirp_family_inet),
+
+#if 0
+        /* Untested: Needs checking by someone with IPv6 test */
+        VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr,
+                            slirp_family_inet6),
+#endif
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_slirp_socket = {
+    .name = "slirp-socket",
+    .version_id = 4,
+    .pre_load = slirp_socket_pre_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(so_urgc, struct socket),
+        /* Pre-v4 versions */
+        VMSTATE_SIN4_ADDR(so_faddr.s_addr, struct socket,
+                            slirp_older_than_v4),
+        VMSTATE_SIN4_ADDR(so_laddr.s_addr, struct socket,
+                            slirp_older_than_v4),
+        VMSTATE_UINT16_TEST(so_fport, struct socket, slirp_older_than_v4),
+        VMSTATE_UINT16_TEST(so_lport, struct socket, slirp_older_than_v4),
+        /* v4 and newer */
+        VMSTATE_STRUCT(fhost, struct socket, 4, vmstate_slirp_socket_addr,
+                       union slirp_sockaddr),
+        VMSTATE_STRUCT(lhost, struct socket, 4, vmstate_slirp_socket_addr,
+                       union slirp_sockaddr),
+
+        VMSTATE_UINT8(so_iptos, struct socket),
+        VMSTATE_UINT8(so_emu, struct socket),
+        VMSTATE_UINT8(so_type, struct socket),
+        VMSTATE_INT32(so_state, struct socket),
+        VMSTATE_STRUCT(so_rcv, struct socket, 0, vmstate_slirp_sbuf,
+                       struct sbuf),
+        VMSTATE_STRUCT(so_snd, struct socket, 0, vmstate_slirp_sbuf,
+                       struct sbuf),
+        VMSTATE_STRUCT_POINTER(so_tcpcb, struct socket, vmstate_slirp_tcp,
+                       struct tcpcb),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
 {
     int i;

@@ -1308,7 +1367,7 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
                 continue;
 
             qemu_put_byte(f, 42);
-            slirp_socket_save(f, so);
+            vmstate_save_state(f, &vmstate_slirp_socket, so, NULL);
         }
     qemu_put_byte(f, 0);
 

@@ -1317,55 +1376,6 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
     slirp_bootp_save(f, slirp);
 }
 
-static int slirp_socket_load(QEMUFile *f, struct socket *so, int version_id)
-{
-    int ret = 0;
-    if (tcp_attach(so) < 0)
-        return -ENOMEM;
-
-    so->so_urgc = qemu_get_be32(f);
-    if (version_id <= 3) {
-        so->so_ffamily = AF_INET;
-        so->so_faddr.s_addr = qemu_get_be32(f);
-        so->so_laddr.s_addr = qemu_get_be32(f);
-        so->so_fport = qemu_get_be16(f);
-        so->so_lport = qemu_get_be16(f);
-    } 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);
-    so->so_type = qemu_get_byte(f);
-    so->so_state = qemu_get_be32(f);
-    /* TODO: VMState at this level */
-    ret = vmstate_load_state(f, &vmstate_slirp_sbuf, &so->so_rcv, 0);
-    if (!ret) {
-        ret = vmstate_load_state(f, &vmstate_slirp_sbuf, &so->so_snd, 0);
-    }
-    if (!ret) {
-        ret = vmstate_load_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
-    }
-    return ret;
-}
 
 static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
 {

@@ -1389,7 +1399,7 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
         if (!so)
             return -ENOMEM;
 
-        ret = slirp_socket_load(f, so, version_id);
+        ret = vmstate_load_state(f, &vmstate_slirp_socket, so, version_id);
 
         if (ret < 0)
             return ret;
slirp/socket.h +3 -3
index c1be77e..2f224bc 100644
@@ -36,7 +36,7 @@ struct socket {
 				    * PING reply's */
   struct tcpiphdr *so_ti;	   /* Pointer to the original ti within
 				    * so_mconn, for non-blocking connections */
-  int so_urgc;
+  uint32_t      so_urgc;
   union slirp_sockaddr fhost;      /* Foreign host */
 #define so_faddr fhost.sin.sin_addr
 #define so_fport fhost.sin.sin_port

@@ -54,8 +54,8 @@ struct socket {
   uint8_t	so_iptos;	/* Type of service */
   uint8_t	so_emu;		/* Is the socket emulated? */
 
-  u_char	so_type;		/* Type of socket, UDP or TCP */
-  int	so_state;		/* internal state flags SS_*, below */
+  uint8_t       so_type;        /* Type of socket, UDP or TCP */
+  int32_t       so_state;       /* internal state flags SS_*, below */
 
   struct 	tcpcb *so_tcpcb;	/* pointer to TCP protocol control block */
   u_int	so_expire;		/* When the socket will expire */
Signature · 1 line
2.9.3
Reviewed-by on v4 4/5
#7 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 9 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Juan Quintela <[email protected]>

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1281

Quoted 2 lines
slirp/slirp.c +2 -0
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)

Great!
Just hope that there is no 32000 families soon :-)

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1307

Quoted 11 lines
slirp/slirp.c +11 -0
+#if 0
+        /* Untested: Needs checking by someone with IPv6 test */
+        VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr,
+                            slirp_family_inet6),
+#endif

I think this is wrong, we have different fieldbs depending in if it is ipv6 or not?

Later, Juan.

author
#8 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 21 lines · * Juan Quintela ([email protected]) wrote:
Quoted 9 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Juan Quintela <[email protected]>

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1281

Quoted 2 lines
slirp/slirp.c +2 -0
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)

Great!
Just hope that there is no 32000 families soon :-)

Quoted 14 lines

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1307

Quoted 11 lines
slirp/slirp.c +11 -0
+#if 0
+        /* Untested: Needs checking by someone with IPv6 test */
+        VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr,
+                            slirp_family_inet6),
+#endif

I think this is wrong, we have different fieldbs depending in if it is ipv6 or not?

Yes, if the family field tells us it's IPv6 then we have to send the longer and the extra IPv6 info - I don't know IPv6, I just used the fields in the sin6 branch of the address and left it in there at Samuel's suggestion for the person who actually implements IPv6 support in this part of SLIRP.

Dave

Quoted 1 line

Later, Juan.

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

author
#9 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

In reply to Juan Quintela

Samual, Jan: Can you just take 1-3 of this series; 4 has a problem;

Quoted 11 lines · * Juan Quintela ([email protected]) wrote:
Quoted 9 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Quoted 8 lines
Reviewed-by Juan Quintela <[email protected]>

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1281

Quoted 2 lines
slirp/slirp.c +2 -0
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)

Great!
Just hope that there is no 32000 families soon :-)

Actually; that's a problem; it turns out FreeBSD has a char for it's family type rahter than the uint16 that linux and windows have. I need to think hth to abstract that.

Dave

Quoted 17 lines

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1307

Quoted 13 lines
slirp/slirp.c +11 -0
+#if 0
+        /* Untested: Needs checking by someone with IPv6 test */
+        VMSTATE_BUFFER_TEST(sin6.sin6_addr, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT16_TEST(sin6.sin6_port, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_flowinfo, union slirp_sockaddr,
+                            slirp_family_inet6),
+        VMSTATE_UINT32_TEST(sin6.sin6_scope_id, union slirp_sockaddr,
+                            slirp_family_inet6),
+#endif

I think this is wrong, we have different fieldbs depending in if it is ipv6 or not?

Quoted 2 lines

Later, Juan.

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#10 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 23 lines · Dr. David Alan Gilbert

Samual, Jan: Can you just take 1-3 of this series; 4 has a problem;

Quoted 11 lines · * Juan Quintela ([email protected]) wrote:
Quoted 9 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Quoted 8 lines
Reviewed-by Juan Quintela <[email protected]>

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1281

Quoted 2 lines
slirp/slirp.c +2 -0
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)

Great!
Just hope that there is no 32000 families soon :-)

Quoted 3 lines

Actually; that's a problem; it turns out FreeBSD has a char for it's family type rahter than the uint16 that linux and windows have. I need to think hth to abstract that.

What, if any, promises have we made about the migration data format being ABI stable across platforms ?

It strikes me that even if FreeBSD & Linux had the same sized type for ss_family, the constants used to popuate it might be different on each platform. eg AF_INET6 is 10 on Linux but 23 on Mingw and probably something else again on FreeBSD.

IOW if we transmit this data on the wire, we've effectively said that our migration data format is *not* portable across different host OS platforms. At that point, sending different sized types on BSD vs Linux is no big deal since we're already incompatible semantically.

Regards,
Daniel

Signature · 3 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|
author
#11 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 24 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 23 lines · Dr. David Alan Gilbert

Samual, Jan: Can you just take 1-3 of this series; 4 has a problem;

Quoted 19 lines · * Juan Quintela ([email protected]) wrote:
Quoted 9 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Working up the stack, this replaces the slirp_socket_load/save with VMState definitions.

A place holder for IPv6 support is added as a comment; it needs testing once the rest of the IPv6 code is there.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
Reviewed-by Juan Quintela <[email protected]>

slirp/slirp.c static const VMStateDescription vmstate_slirp_sbuf = { · line 1281

Quoted 2 lines
slirp/slirp.c +2 -0
+/* Win has a signed family number */
+#define VMSTATE_SS_FAMILY(f, s) VMSTATE_INT16(f, s)

Great!
Just hope that there is no 32000 families soon :-)

Quoted 6 lines
Quoted 3 lines

Actually; that's a problem; it turns out FreeBSD has a char for it's family type rahter than the uint16 that linux and windows have. I need to think hth to abstract that.

What, if any, promises have we made about the migration data format being ABI stable across platforms ?

It should be in theory; it should just reflect the state of the guest, and the state of the guest should be independent of the platform it's running on. Now slirp is a bit of an odd case since it involves more host state.

Quoted 4 lines

It strikes me that even if FreeBSD & Linux had the same sized type for ss_family, the constants used to popuate it might be different on each platform. eg AF_INET6 is 10 on Linux but 23 on Mingw and probably something else again on FreeBSD.

Ewww I'd thought that was at least standardised. It looks like AF_INET is 2 on Linux, mingw and FreeBSD; but AF_INET6 is more dependent on what snook in before hand.

Quoted 4 lines

IOW if we transmit this data on the wire, we've effectively said that our migration data format is *not* portable across different host OS platforms. At that point, sending different sized types on BSD vs Linux is no big deal since we're already incompatible semantically.

This is a relatively recent error; it comes from eae303ff which added ss_family to allow IPv6.

I suspect the right fix here is to populate a temporary for ss_family on the wire; if we make it conveniently match Linux's AF_INET6 values it might work.

Dave

Quoted 6 lines

Regards,
Daniel

Signature · 3 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#12 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Hello,

Quoted 6 lines · Dr. David Alan Gilbert, on jeu. 23 févr. 2017 11:40:45 +0000, wrote:
Quoted 4 lines · * Daniel P. Berrange ([email protected]) wrote:

IOW if we transmit this data on the wire, we've effectively said that our migration data format is *not* portable across different host OS platforms. At that point, sending different sized types on BSD vs Linux is no big deal since we're already incompatible semantically.

Quoted 6 lines

This is a relatively recent error; it comes from eae303ff which added ss_family to allow IPv6.

I suspect the right fix here is to populate a temporary for ss_family on the wire; if we make it conveniently match Linux's AF_INET6 values it might work.

So this issue is actually not new, so I asked to pull your patch series, thanks!

Now we should indeed fix the issue. Making the values match Linux' AF_INET6, why not (better have some known values than arbitrary values), but I don't think we'd want to rely on this: we'll have to introduce a versioned difference, since we'll want to change the size of the field as well as its value on other OSes (and I don't think we want to manage different versioning on different OSes).

Samuel

#13 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 1 line · Samuel Thibault, on dim. 26 févr. 2017 21:34:27 +0100, wrote:

since we'll want to change the size of the field

Ah, no, sorry, it was forced to be 16bit, so at least the size is fine.

But I guess we don't want to change the values to have cross-OS compatibility without changing the version.

Samuel

author
#14 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

In reply to Samuel Thibault
Quoted 12 lines · * Samuel Thibault ([email protected]) wrote:

Hello,

Quoted 9 lines · Dr. David Alan Gilbert, on jeu. 23 févr. 2017 11:40:45 +0000, wrote:
Quoted 4 lines · * Daniel P. Berrange ([email protected]) wrote:

IOW if we transmit this data on the wire, we've effectively said that our migration data format is *not* portable across different host OS platforms. At that point, sending different sized types on BSD vs Linux is no big deal since we're already incompatible semantically.

This is a relatively recent error; it comes from eae303ff which added ss_family to allow IPv6.

Quoted 6 lines
Quoted 3 lines

I suspect the right fix here is to populate a temporary for ss_family on the wire; if we make it conveniently match Linux's AF_INET6 values it might work.

So this issue is actually not new, so I asked to pull your patch series, thanks!

It is because the old non-vmstate code just truncated the value read, where as the VMSTATE macros have strong type checks.

Quoted 11 lines

Now we should indeed fix the issue. Making the values match Linux' AF_INET6, why not (better have some known values than arbitrary values), but I don't think we'd want to rely on this: we'll have to introduce a versioned difference, since we'll want to change the size of the field as well as its value on other OSes (and I don't think we want to manage different versioning on different OSes).

Ah, no, sorry, it was forced to be 16bit, so at least the size is fine.

But I guess we don't want to change the values to have cross-OS compatibility without changing the version.

I'm thinking one way to do it without changing the version would be to use the existing value for IPv4, and on reading allow any other value for IPv6 (or just the ones we know about); that would make it inwards migration compatible. For writing we pick one for IPv6 and stick to it.

Dave

Quoted 2 lines

Samuel

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#15 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 4 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +0000, wrote:

I'm thinking one way to do it without changing the version would be to use the existing value for IPv4, and on reading allow any other value for IPv6 (or just the ones we know about); that would make it inwards migration compatible.

Right. I don't know if that's enough for QEMU requirements.

Samuel

author
#16 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 7 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 4 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +0000, wrote:

I'm thinking one way to do it without changing the version would be to use the existing value for IPv4, and on reading allow any other value for IPv6 (or just the ones we know about); that would make it inwards migration compatible.

Right. I don't know if that's enough for QEMU requirements.

If you change the version number you break backwards migration anyway; but doing what I suggested would keep backwards working in most cases.

Dave

Quoted 1 line

Samuel

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#17 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 11 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:00:17 +0000, wrote:
Quoted 7 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 4 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +0000, wrote:

I'm thinking one way to do it without changing the version would be to use the existing value for IPv4, and on reading allow any other value for IPv6 (or just the ones we know about); that would make it inwards migration compatible.

Right. I don't know if that's enough for QEMU requirements.

If you change the version number you break backwards migration anyway; but doing what I suggested would keep backwards working in most cases.

Sure, but in some cases we're breaking upward compatibility *silently*.

Samuel

author
#18 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 8 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 7 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:00:17 +0000, wrote:
Quoted 6 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 4 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 16:54:46 +0000, wrote:

I'm thinking one way to do it without changing the version would be to use the existing value for IPv4, and on reading allow any other value for IPv6 (or just the ones we know about); that would make it inwards migration compatible.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

Right. I don't know if that's enough for QEMU requirements.

If you change the version number you break backwards migration anyway; but doing what I suggested would keep backwards working in most cases.

Sure, but in some cases we're breaking upward compatibility *silently*.

No, we don't need to break upwards compatibility; if I understand correctly the only values that can be in that field at the moment are:
AF_INET - IPv4 which is 2 on everything we've seen
AF_INET6 - whose value is different on different implementations.

If we accept an incoming value of '2' to mean AF_INET then all existing IPv4 connections work.
If we accept any other incoming value as AF_INET6 all existing IPv6 connections work.
- so that's foward compatibility OK.

If we generate '2' as the outgoing value for AF_INET then migration to all exiting code works for IPv4; so backwards works for IPv4. If we generate Linux's AF_INET6 number then IPv6 also works backwards but only for Linux.

Dave

Quoted 1 line

Samuel

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#19 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 1 line · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:09:26 +0000, wrote:

but only for Linux.

That's what I was referring to.

Samuel

author
#20 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 4 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 1 line · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:09:26 +0000, wrote:

but only for Linux.

That's what I was referring to.

Yes I think that's OK because:
a) The alternative breaks all backwards migration
b) This doesn't casuse any problem for forward migration

There is another way, we could add a subsection that's sent whenever we use non-IPv4; that would (probably) fail noisily on a backwards migration to an old setup. But IMHO it's not worth it in this case given how rare a backwards migration of an IPv6 slirp connection on something other than Linux is.

Dave

Quoted 1 line

Samuel

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#21 lore raw

Re: [Qemu-devel] [PATCH v4 4/5] slirp: VMStatify socket level

Quoted 10 lines · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:40:19 +0000, wrote:
Quoted 4 lines · * Samuel Thibault ([email protected]) wrote:
Quoted 1 line · Dr. David Alan Gilbert, on mar. 28 févr. 2017 17:09:26 +0000, wrote:

but only for Linux.

That's what I was referring to.

Yes I think that's OK because:
a) The alternative breaks all backwards migration
b) This doesn't casuse any problem for forward migration

Quoted 5 lines

There is another way, we could add a subsection that's sent whenever we use non-IPv4; that would (probably) fail noisily on a backwards migration to an old setup. But IMHO it's not worth it in this case given how rare a backwards migration of an IPv6 slirp connection on something other than Linux is.

Alright.

Samuel

author
#22 lore raw

PATCH v4 5/5 slirp: VMStatify remaining except for loop

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

This converts the remaining components, except for the top level loop, to VMState.

Signed-off-by Dr. David Alan Gilbert <[email protected]>
---
 slirp/slirp.c | 48 +++++++++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)
slirp/slirp.c +19 -29
index 6583de8..f8ee7f9 100644
@@ -1343,15 +1343,25 @@ static const VMStateDescription vmstate_slirp_socket = {
     }
 };
 
-static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
-{
-    int i;
+static const VMStateDescription vmstate_slirp_bootp_client = {
+    .name = "slirp_bootpclient",
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(allocated, BOOTPClient),
+        VMSTATE_BUFFER(macaddr, BOOTPClient),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
-    for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-        qemu_put_be16(f, slirp->bootp_clients[i].allocated);
-        qemu_put_buffer(f, slirp->bootp_clients[i].macaddr, 6);
+static const VMStateDescription vmstate_slirp = {
+    .name = "slirp",
+    .version_id = 4,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16_V(ip_id, Slirp, 2),
+        VMSTATE_STRUCT_ARRAY(bootp_clients, Slirp, NB_BOOTP_CLIENTS, 3,
+                             vmstate_slirp_bootp_client, BOOTPClient),
+        VMSTATE_END_OF_LIST()
     }
-}
+};
 
 static void slirp_state_save(QEMUFile *f, void *opaque)
 {

@@ -1371,22 +1381,10 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
         }
     qemu_put_byte(f, 0);
 
-    qemu_put_be16(f, slirp->ip_id);
-
-    slirp_bootp_save(f, slirp);
+    vmstate_save_state(f, &vmstate_slirp, slirp, NULL);
 }
 
 
-static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
-{
-    int i;
-
-    for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-        slirp->bootp_clients[i].allocated = qemu_get_be16(f);
-        qemu_get_buffer(f, slirp->bootp_clients[i].macaddr, 6);
-    }
-}
-
 static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
 {
     Slirp *slirp = opaque;

@@ -1421,13 +1419,5 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
         so->extra = (void *)ex_ptr->ex_exec;
     }
 
-    if (version_id >= 2) {
-        slirp->ip_id = qemu_get_be16(f);
-    }
-
-    if (version_id >= 3) {
-        slirp_bootp_load(f, slirp);
-    }
-
-    return 0;
+    return vmstate_load_state(f, &vmstate_slirp, slirp, version_id);
 }
Signature · 1 line
2.9.3
bot
#24 lore raw
In reply to Dr. David Alan Gilbert (git)
This series seems to have some coding style problems. See output below for more information:

Hi,

This series seems to have some coding style problems. See output below for more information:

Type: series
Subject: [Qemu-devel] [PATCH v4 0/5] SLIRP VMStatification
Message-id: [email protected]

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do

    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))

done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu
- [tag update] patchew/[email protected] -> patchew/[email protected]
* [new tag] patchew/[email protected] -> patchew/[email protected]
Switched to a new branch 'test'
42ec6e4 slirp: VMStatify remaining except for loop 8278228 slirp: VMStatify socket level b2cd17c slirp: Common lhost/fhost union af8192f slirp: VMStatify sbuf
3bb572c slirp: VMState conversion; tcpcb

=== OUTPUT BEGIN ===
Checking PATCH 1/5: slirp: VMState conversion; tcpcb...
ERROR: code indent should never use tabs
#211: FILE: slirp/tcp_var.h:51:
+^Iuint8_t t_force;^I^I/* 1 if forcing out a byte */$

ERROR: code indent should never use tabs
#221: FILE: slirp/tcp_var.h:112:
+^Iuint8_t^It_oobflags;^I^I/* have some */$

ERROR: code indent should never use tabs
#222: FILE: slirp/tcp_var.h:113:
+^Iuint8_t^It_iobc;^I^I^I/* input character */$

total: 3 errors, 0 warnings, 195 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

Checking PATCH 2/5: slirp: VMStatify sbuf...
ERROR: code indent should never use tabs #26: FILE: slirp/sbuf.h:15:
+^Iuint32_t sb_cc;^I^I/* actual chars in buffer */$

ERROR: code indent should never use tabs
#27: FILE: slirp/sbuf.h:16:
+^Iuint32_t sb_datalen;^I/* Length of data */$

total: 2 errors, 0 warnings, 155 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

Checking PATCH 3/5: slirp: Common lhost/fhost union... Checking PATCH 4/5: slirp: VMStatify socket level...
ERROR: if this code is redundant consider removing it #96: FILE: slirp/slirp.c:1297:
+#if 0

total: 1 errors, 0 warnings, 217 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

Checking PATCH 5/5: slirp: VMStatify remaining except for loop... === OUTPUT END ===

Test command exited with code: 1

---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [email protected]