* Samuel Thibault (samuel.thiba...@gnu.org) wrote: > Hello, > > Dr. David Alan Gilbert (git), on Thu 27 Oct 2016 16:32:16 +0100, wrote: > > - case AF_INET: > > - qemu_put_be32(f, so->so_faddr.s_addr); > > - qemu_put_be16(f, so->so_fport); > > - break; > > > + if (version_id >= 4 && !is_inet) { > > + error_report("%s: so_ffamily unknown, socket not preserved", > > __func__); > > } > > Well, no, we need to settle this another way, because we want to be able > to easily add inet6 support here. At least pave the way in a way that > makes it not unnecessarily hard. The code you are adding here looks to > me like very hard to rework to make it support the various socket > families.
Well, I was just trying to match the current semantics/errors there. > > + VMSTATE_UINT16_V(so_ffamily, struct socket, 4), > > + VMSTATE_UINT32_TEST(so_faddr.s_addr, struct socket, > > + slirp_v4_or_newer_ffamily_inet), > > + VMSTATE_UINT16_TEST(so_fport, struct socket, > > + slirp_v4_or_newer_ffamily_inet), > > Does VMStat not provide a way to have differing content depending on a > field? (here, so_ffamily) It has two things: a) Versions (e.g. VMSTATE_UINT16_V) - that says only include this field if we're on version >= n b) Tests (.e.g. VMSTATE_UINT16_TEST) - that says only include the field if the test says so. The tests are a bit tedious since they're each a function, their is no generic test description scheme. (A lambda here would be lovely, but making one to work on both Gcc and Clang and anything else isn't trivial; I looked) I think the underlying macros would make it easy to do a VMSTATE_UINT16_TEST_V that only happens if both the test and the check are true which might make this easier. It's not that hard, but a bit tedious if we wanted to add another family here; we'd end up with: VMSTATE_UINT16_V(so_ffamily, struct socket, 4), VMSTATE_UINT32_TEST(so_faddr.s_addr, struct socket, slirp_v4_or_newer_ffamily_inet), VMSTATE_UINT32_TEST(so_faddr.s_addr6, struct socket, slirp_v4_or_newer_ffamily_inet6), VMSTATE_UINT16_TEST(so_fport, struct socket, slirp_v4_or_newer_ffamily_inet), and change the test to: static bool slirp_v4_or_newer_ffamily_inet(void *opaque, int version_id) { bool is_inet = ((struct socket *)opaque)->so_ffamily == AF_INET; bool is_inet6 = ((struct socket *)opaque)->so_ffamily == AF_INET6; if (version_id >= 4 && !is_inet && !is_inet6) { error_report("%s: so_ffamily unknown, socket not preserved", __func__); } return version_id >= 4 && is_inet; } static bool slirp_v4_or_newer_ffamily_inet6(void *opaque, int version_id) { bool is_inet6 = ((struct socket *)opaque)->so_ffamily == AF_INET6; return version_id >= 4 && is_inet6; } The asymmetry is purely to generate the error. Actually, I might be able to avoid some of the lfamily/ffamily duplication here - I hadn't spotted the were macros for lhost.ss.ss_family, so I think I can rework the v4 or newer by doing: a) Split the fhost and lhost union's out as a separate union and share them. b) For the v4 entries do: VMSTATE_STRUCT(fhost, struct socket, 4, vmstate_slirp_hosts); VMSTATE_STRUCT(lhost, struct socket, 4, vmstate_slirp_hosts); and define vmstate_slirp_hosts to have the logic to check the family; although I've never tried VMSTATE_STRUCT on a union I think it'll work (!). Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK