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

[Dr. David Alan Gilbert]

* Samuel Thibault (address@hidden) wrote:
> Samuel Thibault, on Sun 27 Nov 2016 16:13:46 +0100, wrote:
> > Dr. David Alan Gilbert (git), on Wed 23 Nov 2016 18:52:57 +0000, wrote:
> > > +static const VMStateDescription vmstate_slirp_socket_addr = {
> > > +    .name = "slirp-socket-addr",
> > > +    .version_id = 4,
> > > +    .fields = (VMStateField[]) {
> > > +        VMSTATE_UINT16(ss.ss_family, union slirp_sockaddr),
> > > +        VMSTATE_UINT32_TEST(sin.sin_addr.s_addr, union slirp_sockaddr,
> > > +                            slirp_family_inet),
> > > +        VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr,
> > > +                            slirp_family_inet),
> > > +        VMSTATE_END_OF_LIST()
> > > +    }
> > > +};
> > 
> > How will we be able to add the IPv6 case here?
> 
> Reading again your previous post, it seemed it'd be in
> slirp_family_inet, but I don't immediately see how.
> 
> Applying your patch for 2.9 would thus make porting the code to IPv6
> more difficult than how it is now, so I'm quite reluctant :)
> 
> Could you perhaps simply add the IPv6 case in your patch series already?
> It shouldn't be much work for you who actually know how the VMSTATE
> machinery is supposed to work (I guess the amount of people who care
> about slirp *and* know about VMSTATE is extremely small), and a proof of
> concept for the portability to non-ipv4 addresse spaces.

The number of people who care about slirp, IPv6, VMState is even smaller :-)
Hmm, I don't really know IPv6 but I'm thinking this code will become something 
like
the following (says he not knowing whether a scope-id or a flowinfo
is something that needs migrating) (untested):


static const VMStateDescription vmstate_slirp_socket_addr = {
    .name = "slirp-socket-addr",
    .version_id = 4,
    .fields = (VMStateField[]) {
        VMSTATE_UINT16(ss.ss_family, union slirp_sockaddr),
        VMSTATE_UINT32_TEST(sin.sin_addr.s_addr, union slirp_sockaddr,
                            slirp_family_inet),
        VMSTATE_UINT16_TEST(sin.sin_port, union slirp_sockaddr,
                            slirp_family_inet),

        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),
                            
        VMSTATE_END_OF_LIST()
    }
};

So to me that looks pretty clean, we need to add another slirp_family_inet6
test function, but then we just add the extra fields for the IPv6 stuff.

Can you suggest an IPv6 command line for testing that ?

Dave
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK