Re: [Qemu-devel] [RFC 8/8] slirp: convert save/load function to visitor interface

[Anthony Liguori <anthony@codemonkey.ws>]

On 09/19/2011 09:41 AM, Michael Roth wrote:
> Where possible common routines are used for both input and output, thus
> save on lines of code, theoretically. The added lines here are mostly
> due to extra logic for each save/load routine to manipulate strings into
> a unique field name for each saved field, and in some cases a few extra
> Visitor calls to due list/struct i/o. With some reworking we can
> probably optimize all of these to reduce the amount of added code.
>
> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
> ---
>   slirp/slirp.c |  366 +++++++++++++++++++++++++++++++++-----------------------
>   1 files changed, 216 insertions(+), 150 deletions(-)
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 19d69eb..8783626 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -26,6 +26,9 @@
>   #include "qemu-char.h"
>   #include "slirp.h"
>   #include "hw/hw.h"
> +#include "qemu-error.h"
> +
> +#define SLIRP_DELIMITER 42 /* used to separate slirp instances in save/load */
>
>   /* host loopback address */
>   struct in_addr loopback_addr;
> @@ -871,96 +874,171 @@ 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 void slirp_tcp_visit(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
>   {
> -    int i;
> -
> -    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);
> +    char f[128];
> +    int i, l = 0;
> +    int16_t *ptr;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(pfield);
> +    }
> +
> +    visit_type_int16_t(v,&tp->t_state, strocat(f, ".t_state", l),&err);
> +    ptr = tp->t_timer;
> +    visit_start_array(v, (void **)&ptr, strocat(f, ".t_timer", l), TCPT_NTIMERS, sizeof(*ptr),&err);
> +    for (i = 0; i<  TCPT_NTIMERS; ++i) {
> +        visit_type_int16_t(v,&ptr[i], NULL,&err);
> +    }
> +    visit_end_array(v,&err);
> +    visit_type_int16_t(v,&tp->t_rxtshift, strocat(f, ".t_rxtshift", l),&err);


Hrm, you should never concat a name like this.  A better approach would be:

visit_start_struct(v, NULL, f);
visit_type_int16_t(v, &tp->t_rxtshift, "t_rxtshift", &err);
...
visit_end_struct(v, NULL, f);

structs indicate hierarchy.  They don't have to correspond to real types.

Regards,

Anthony Liguori

> +    visit_type_int16_t(v,&tp->t_rxtcur, strocat(f, ".t_rxtcur", l),&err);
> +    visit_type_int16_t(v,&tp->t_dupacks, strocat(f, ".t_dupacks", l),&err);
> +    visit_type_uint16_t(v,&tp->t_maxseg, strocat(f, ".t_maxseg", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_force, strocat(f, ".t_force", l),&err);
> +    visit_type_uint16_t(v,&tp->t_flags, strocat(f, ".t_flags", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_una, strocat(f, ".snd_una", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_nxt, strocat(f, ".snd_nxt", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_up, strocat(f, ".snd_up", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wl1, strocat(f, ".snd_wl1", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wl2, strocat(f, ".snd_wl2", l),&err);
> +    visit_type_uint32_t(v,&tp->iss, strocat(f, ".iss", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wnd, strocat(f, ".snd_wnd", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_wnd, strocat(f, ".rcv_wnd", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_nxt, strocat(f, ".rcv_nxt", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_up, strocat(f, ".rcv_up", l),&err);
> +    visit_type_uint32_t(v,&tp->irs, strocat(f, ".irs", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_adv, strocat(f, ".rcv_adv", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_max, strocat(f, ".snd_max", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_cwnd, strocat(f, ".snd_cwnd", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_ssthresh, strocat(f, ".snd_ssthresh", l),&err);
> +    visit_type_int16_t(v,&tp->t_idle, strocat(f, ".t_idle", l),&err);
> +    visit_type_int16_t(v,&tp->t_rtt, strocat(f, ".t_rtt", l),&err);
> +    visit_type_uint32_t(v,&tp->t_rtseq, strocat(f, ".t_rtseq", l),&err);
> +    visit_type_int16_t(v,&tp->t_srtt, strocat(f, ".t_srtt", l),&err);
> +    visit_type_int16_t(v,&tp->t_rttvar, strocat(f, ".t_rttvar", l),&err);
> +    visit_type_uint16_t(v,&tp->t_rttmin, strocat(f, ".t_rttmin", l),&err);
> +    visit_type_uint32_t(v,&tp->max_sndwnd, strocat(f, ".max_sndwnd", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_oobflags, strocat(f, ".t_oobflags", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_iobc, strocat(f, ".t_iobc", l),&err);
> +    visit_type_int16_t(v,&tp->t_softerror, strocat(f, ".t_softerror", l),&err);
> +    visit_type_uint8_t(v,&tp->snd_scale, strocat(f, ".snd_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->rcv_scale, strocat(f, ".rcv_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->request_r_scale, strocat(f, ".request_r_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->requested_s_scale, strocat(f, ".requested_s_scale", l),&err);
> +    visit_type_uint32_t(v,&tp->ts_recent, strocat(f, ".ts_recent", l),&err);
> +    visit_type_uint32_t(v,&tp->ts_recent_age, strocat(f, ".ts_recent_age", l),&err);
> +    visit_type_uint32_t(v,&tp->last_ack_sent, strocat(f, ".last_ack_sent", l),&err);
>   }
>
> -static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
> +static void slirp_tcp_save(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
>   {
> -    uint32_t off;
> +    slirp_tcp_visit(v, tp, pfield, err);
> +}
>
> -    qemu_put_be32(f, sbuf->sb_cc);
> -    qemu_put_be32(f, sbuf->sb_datalen);
> +static void slirp_sbuf_save(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
> +{
> +    int32_t off;
> +    char f[128];
> +    int i, l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    visit_type_uint32_t(v,&sbuf->sb_cc, strocat(f, ".sb_cc", l),&err);
> +    visit_type_uint32_t(v,&sbuf->sb_datalen, strocat(f, ".sb_datalen", l),&err);
>       off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
> -    qemu_put_sbe32(f, off);
> +    visit_type_int32_t(v,&off, strocat(f, ".sb_wptr_off", l),&err);
>       off = (uint32_t)(sbuf->sb_rptr - sbuf->sb_data);
> -    qemu_put_sbe32(f, off);
> -    qemu_put_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
> +    visit_type_int32_t(v,&off, strocat(f, ".sb_rptr_off", l),&err);
> +
> +    visit_start_array(v, (void **)&sbuf->sb_data, strocat(f, ".sb_data", l),
> +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> +        visit_type_uint8_t(v, (uint8_t *)&sbuf->sb_data[i], NULL,&err);
> +    }
> +    visit_end_array(v,&err);
> +}
> +
> +static void slirp_socket_visit(Visitor *v, struct socket *so, const char *pfield, Error *err)
> +{
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +    visit_type_int32_t(v,&so->so_urgc, strocat(f, ".so_urgc", l),&err);
> +    visit_type_uint32_t(v,&so->so_faddr.s_addr, strocat(f, ".so_faddr.s_addr", l),&err);
> +    visit_type_uint32_t(v,&so->so_laddr.s_addr, strocat(f, ".so_urgc", l),&err);
> +    visit_type_uint16_t(v,&so->so_fport, strocat(f, ".so_fport", l),&err);
> +    visit_type_uint16_t(v,&so->so_lport, strocat(f, ".so_lport", l),&err);
> +    visit_type_uint8_t(v,&so->so_iptos, strocat(f, ".so_iptos", l),&err);
> +    visit_type_uint8_t(v,&so->so_emu, strocat(f, ".so_emu", l),&err);
> +    visit_type_uint8_t(v,&so->so_type, strocat(f, ".so_type", l),&err);
> +    visit_type_int32_t(v,&so->so_state, strocat(f, ".so_state", l),&err);
>   }
>
> -static void slirp_socket_save(QEMUFile *f, struct socket *so)
> +static void slirp_socket_save(Visitor *v, struct socket *so, const char *pfield, Error *err)
>   {
> -    qemu_put_be32(f, so->so_urgc);
> -    qemu_put_be32(f, so->so_faddr.s_addr);
> -    qemu_put_be32(f, so->so_laddr.s_addr);
> -    qemu_put_be16(f, so->so_fport);
> -    qemu_put_be16(f, so->so_lport);
> -    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);
> -    slirp_sbuf_save(f,&so->so_rcv);
> -    slirp_sbuf_save(f,&so->so_snd);
> -    slirp_tcp_save(f, so->so_tcpcb);
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    slirp_socket_visit(v, so, f, err);
> +
> +    slirp_sbuf_save(v,&so->so_rcv, strocat(f, ".so_rcv", l), err);
> +    slirp_sbuf_save(v,&so->so_snd, strocat(f, ".so_snd", l), err);
> +    slirp_tcp_save(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
>   }
>
> -static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
> +static void slirp_bootp_visit(Visitor *v, Slirp *slirp, const char *pfield, Error *err)
>   {
> -    int i;
> +    int i, j, l;
> +    void *ptr;
> +    char f[64];
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
>
> +    ptr = slirp->bootp_clients;
> +    visit_start_array(v,&ptr, strocat(f, ".bootp_clients", l), NB_BOOTP_CLIENTS, 8,&err);
>       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);
> +        visit_type_uint16_t(v,&slirp->bootp_clients[i].allocated, "allocated",&err);
> +        ptr = slirp->bootp_clients[i].macaddr;
> +        visit_start_array(v,&ptr, "macaddr", 6, 1,&err);
> +        for (j = 0; j<  6; j++) {
> +            visit_type_uint8_t(v, (uint8_t *)&slirp->bootp_clients[j], NULL,&err);
> +        }
> +        visit_end_array(v,&err);
>       }
> +    visit_end_array(v,&err);
>   }
>
>   static void slirp_state_save(QEMUFile *f, void *opaque)
>   {
>       Slirp *slirp = opaque;
>       struct ex_list *ex_ptr;
> +    int i = 0;
> +    uint8_t padding;
> +    Visitor *v = qemu_file_get_output_visitor(f);
> +    Error *err = NULL;
> +    char id[32];
>
>       for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next)
>           if (ex_ptr->ex_pty == 3) {
> @@ -970,70 +1048,44 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
>               if (!so)
>                   continue;
>
> -            qemu_put_byte(f, 42);
> -            slirp_socket_save(f, so);
> +            padding = SLIRP_DELIMITER;
> +            visit_type_uint8_t(v,&padding, "padding",&err);
> +            slirp_socket_save(v, so, strocat(id, "so", strlen(id)), err);
>           }
> -    qemu_put_byte(f, 0);
> +    padding = 0;
> +    visit_type_uint8_t(v,&padding, "padding",&err);
> +
> +    visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
>
> -    qemu_put_be16(f, slirp->ip_id);
> +    slirp_bootp_visit(v, slirp, "slirp", err);
>
> -    slirp_bootp_save(f, slirp);
> +    if (err) {
> +        error_report("error saving slirp state: %s", error_get_pretty(err));
> +        error_free(err);
> +    }
>   }
>
> -static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp)
> +static void slirp_tcp_load(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
>   {
> -    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);
> +    slirp_tcp_visit(v, tp, pfield, err);
> +
>       tcp_template(tp);
>   }
>
> -static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
> +static int slirp_sbuf_load(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
>   {
>       uint32_t off, sb_cc, sb_datalen;
> +    int l = 0, i;
> +    char f[64];
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
>
> -    sb_cc = qemu_get_be32(f);
> -    sb_datalen = qemu_get_be32(f);
> +    visit_type_uint32_t(v,&sb_cc, strocat(f, ".sb_cc", l),&err);
> +    visit_type_uint32_t(v,&sb_datalen, strocat(f, ".sb_datalen", l),&err);
>
>       sbreserve(sbuf, sb_datalen);
>
> @@ -1042,61 +1094,68 @@ static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
>
>       sbuf->sb_cc = sb_cc;
>
> -    off = qemu_get_sbe32(f);
> +    visit_type_uint32_t(v,&off, strocat(f, ".sb_wptr_off", l),&err);
>       sbuf->sb_wptr = sbuf->sb_data + off;
> -    off = qemu_get_sbe32(f);
> +    visit_type_uint32_t(v,&off, strocat(f, ".sb_rptr_off", l),&err);
>       sbuf->sb_rptr = sbuf->sb_data + off;
> -    qemu_get_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
> +
> +    visit_start_array(v, (void **)&sbuf->sb_data, strocat(f, ".sb_data", l),
> +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> +        visit_type_uint8_t(v, (uint8_t *)&sbuf->sb_data[i], NULL,&err);
> +    }
> +    visit_end_array(v,&err);
>
>       return 0;
>   }
>
> -static int slirp_socket_load(QEMUFile *f, struct socket *so)
> +static int slirp_socket_load(Visitor *v, struct socket *so, const char *pfield, Error *err)
>   {
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    assert(v);
>       if (tcp_attach(so)<  0)
>           return -ENOMEM;
>
> -    so->so_urgc = qemu_get_be32(f);
> -    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);
> -    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);
> -    if (slirp_sbuf_load(f,&so->so_rcv)<  0)
> +    slirp_socket_visit(v, so, pfield, err);
> +
> +    if (slirp_sbuf_load(v,&so->so_rcv, strocat(f, ".so_rcv", l), err)<  0)
>           return -ENOMEM;
> -    if (slirp_sbuf_load(f,&so->so_snd)<  0)
> +    if (slirp_sbuf_load(v,&so->so_snd, strocat(f, ".so_snd", l), err)<  0)
>           return -ENOMEM;
> -    slirp_tcp_load(f, so->so_tcpcb);
> +    slirp_tcp_load(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
>
>       return 0;
>   }
>
> -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;
>       struct ex_list *ex_ptr;
> +    Visitor *v = qemu_file_get_input_visitor(f);
> +    Error *err = NULL;
> +    char id[32];
> +    uint8_t padding, i = 0;
>
> -    while (qemu_get_byte(f)) {
> +    visit_type_uint8_t(v,&padding, "padding",&err);
> +
> +    while (padding == SLIRP_DELIMITER) {
>           int ret;
>           struct socket *so = socreate(slirp);
>
>           if (!so)
>               return -ENOMEM;
>
> -        ret = slirp_socket_load(f, so);
> +        sprintf(id, "slirp[%d]", i++);
> +
> +        ret = slirp_socket_load(v, so, strocat(id, "so", strlen(id)), err);
>
>           if (ret<  0)
>               return ret;
> @@ -1118,12 +1177,19 @@ 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);
> +        visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
>       }
>
>       if (version_id>= 3) {
> -        slirp_bootp_load(f, slirp);
> +        slirp_bootp_visit(v, slirp, "slirp", err);
> +    }
> +
> +    if (err) {
> +        error_report("error loading slirp state: %s", error_get_pretty(err));
> +        error_free(err);
> +        return -EINVAL;
>       }
>
>       return 0;