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

[Michael Roth]

On Fri, 30 Sep 2011 08:39:49 -0500, Anthony Liguori <address@hidden> wrote:
> 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<address@hidden>
> > ---
> >   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.

Yeah, I thought a lot about using visit_start_struct purely for namespacing,
but wasn't sure if that was too hackish. I like it a whole lot better than
manipulating the field names though... I'll rework it and see.

> 
> 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;
> 

-- 
Sincerely,
Mike Roth
IBM Linux Technology Center