Skip to contentSkip to conversation

SLIRP VMStatification

PATCH In review Dr. David Alan Gilbert (git) 4 participants 10 messages Apr 3, 2017 – Apr 21, 2017

lore
Share & tools
lore

Dr. David Alan Gilbert (git)

author
#0 lore raw

v5 cover SLIRP VMStatification

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

Hi,
Another crack at slirp vmstatification; v5 fixes the type and
value portability issues in the ss_family field.

It now always sends this as a 16bit field and always uses Linux's
AF_INET or AF_INET6 value on writing. It'll also accept Windows and FreeBSDs values as inet6 to keep compatibility with incoming migrations from previous versions.

Dave

v5
Fix ss_family portability

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   | 494 +++++++++++++++++++++++++++++++-------------------------
 slirp/socket.h  |  24 ++-
 slirp/tcp_var.h |   6 +-
 4 files changed, 288 insertions(+), 240 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH v5 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 5a94b06..86eefc3 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 v5 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 v5 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 v5 3/5
#4 lore raw

Re: [Qemu-devel] [PATCH v5 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 v5 3/5
#5 lore raw

Re: [Qemu-devel] [PATCH v5 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 v5 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  | 219 +++++++++++++++++++++++++++++++++++++--------------------
 slirp/socket.h |   6 +-
 2 files changed, 145 insertions(+), 80 deletions(-)
slirp/slirp.c +142 -77
index 2b834af..2de97b6 100644
@@ -1250,40 +1250,154 @@ 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");
+    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_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);
+    /* Older versions don't load these fields */
+    so->so_ffamily = AF_INET;
+    so->so_lfamily = AF_INET;
+    return 0;
+}
+
+#ifndef _WIN32
+#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_UINT32_TEST(f, s, t)
+#else
+/* 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
+
+/* The OS provided ss_family field isn't that portable; it's size
+ * and type varies (16/8 bit, signed, unsigned)
+ * and the values it contains aren't fully portable.
+ */
+typedef struct SS_FamilyTmpStruct {
+    union slirp_sockaddr    *parent;
+    uint16_t                 portable_family;
+} SS_FamilyTmpStruct;
+
+#define SS_FAMILY_MIG_IPV4   2  /* Linux, BSD, Win... */
+#define SS_FAMILY_MIG_IPV6  10  /* Linux */
+#define SS_FAMILY_MIG_OTHER 0xffff
+
+static void ss_family_pre_save(void *opaque)
+{
+    SS_FamilyTmpStruct *tss = opaque;
+
+    tss->portable_family = SS_FAMILY_MIG_OTHER;
+
+    if (tss->parent->ss.ss_family == AF_INET) {
+        tss->portable_family = SS_FAMILY_MIG_IPV4;
+    } else if (tss->parent->ss.ss_family == AF_INET6) {
+        tss->portable_family = SS_FAMILY_MIG_IPV6;
+    }
+}
+
+static int ss_family_post_load(void *opaque, int version_id)
+{
+    SS_FamilyTmpStruct *tss = opaque;
+
+    switch (tss->portable_family) {
+    case SS_FAMILY_MIG_IPV4:
+        tss->parent->ss.ss_family = AF_INET;
+        break;
+    case SS_FAMILY_MIG_IPV6:
+    case 23: /* compatibility: AF_INET6 from mingw */
+    case 28: /* compatibility: AF_INET6 from FreeBSD sys/socket.h */
+        tss->parent->ss.ss_family = AF_INET6;
         break;
     default:
-        error_report("so_ffamily unknown, unable to save so_laddr and"
-                     " so_lport");
+        error_report("invalid ss_family type %x", tss->portable_family);
+        return -EINVAL;
     }
-    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);
+
+    return 0;
 }
 
+static const VMStateDescription vmstate_slirp_ss_family = {
+    .name = "slirp-socket-addr/ss_family",
+    .pre_save  = ss_family_pre_save,
+    .post_load = ss_family_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(portable_family, SS_FamilyTmpStruct),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_slirp_socket_addr = {
+    .name = "slirp-socket-addr",
+    .version_id = 4,
+    .fields = (VMStateField[]) {
+        VMSTATE_WITH_TMP(union slirp_sockaddr, SS_FamilyTmpStruct,
+                            vmstate_slirp_ss_family),
+        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 +1422,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 +1431,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 +1454,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
author
#7 lore raw

PATCH v5 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 2de97b6..701eb7e 100644
@@ -1398,15 +1398,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)
 {

@@ -1426,22 +1436,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;

@@ -1476,13 +1474,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
#9 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:

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

=== 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'
b85f0d0 slirp: VMStatify remaining except for loop 6664e5e slirp: VMStatify socket level b735a98 slirp: Common lhost/fhost union 049836c slirp: VMStatify sbuf
43cbd18 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 #148: FILE: slirp/slirp.c:1352:
+#if 0

total: 1 errors, 0 warnings, 269 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]