[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/8] slirp: VMStatify socket level
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 7/8] slirp: VMStatify socket level |
Date: |
Mon, 7 Nov 2016 19:55:32 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Samuel Thibault (address@hidden) 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 / address@hidden / Manchester, UK
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 7/8] slirp: VMStatify socket level,
Dr. David Alan Gilbert <=