Re: [Qemu-devel] [PATCHv9 0/10] slirp: Adding IPv6 support to Qemu -net user mode

[Jason Wang <jasowang@redhat.com>]

On 03/07/2016 07:19 PM, Samuel Thibault wrote:
> Jason Wang, on Mon 07 Mar 2016 14:48:16 +0800, wrote:
>> - Lots of checkpatch warnings, let's try to silent it.
> They are indentation issues, yes. They are already existing in slirp/
> . Whenever new code was added we sticked with the qemu style, but for
> patched code we prefered to stick to the existing indentation, since
> otherwise it'd mean reindenting it all to keep the functions readable at
> all. Do you what to see the whole slirp directory be reindented once for
> good before this gets applied? Personally I just don't care which way or
> the other.

Not only for indentation issue, e.g:

./scripts/checkpatch.pl
0001-slirp-Adding-IPv6-ICMPv6-Echo-and-NDP-autoconfigurat.patch
ERROR: return is not a function, parentheses are not required
#177: FILE: slirp/ip6.h:65:
+    return (a->s6_addr[prefix_len / 8] & ((1U << (8 - (prefix_len %
8))) - 1))

WARNING: line over 80 characters
#270: FILE: slirp/ip6_icmp.c:14:
+#define NDP_Interval g_rand_int_range(slirp->grand,
NDP_MinRtrAdvInterval, NDP_MaxRtrAdvInterval)

WARNING: if this code is redundant consider removing it
#870: FILE: slirp/ip6_input.c:52:
+#if 0

total: 1 errors, 2 warnings, 1121 lines checked

0001-slirp-Adding-IPv6-ICMPv6-Echo-and-NDP-autoconfigurat.patch has
style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.


>
>> - The patches do not apply to master cleanly.
> It did at the time I sent them...

Right, but not now, so please rebase.

>
>> - I expects a unit-test for this. You may want to have a look at the
>> pxe-test in tests/, I think it could be extended to test ipv6 slirp somehow.
> It doesn't seem so simple to me. In the case of PXE, you have a guest
> implementation of PXE inside the BIOS. In the case of IPv6, I don't
> think you have a guest implementation of IPv6 in the BIOS... So we'd
> need to embed some guest that would do the IPv6 stuff. At best we can
> make a qtest_start(), and that's about it.

Haven't think about this deeply, but at least from the source code, ipxe
did support ipv6. But I agree this could be done in the future.

>
> I'm awfully tired of all of this. This work has been done 3 years ago,
> has already seen 9 iterations. Had it not been something important to
> my eyes (being able to easily test ipv6), I would have abandoned a long
> time ago.

Sorry for this, I know how tired of this, and thanks a lot for not
giving up. I think the patch is pretty ready to be merged after one more
iteration.

>
> At the university of Bordeaux, we are planning to add a FOSS course with
> students participating to existing FOSS projects. I'm afraid we will
> just not be able to make them work on qemu at all.
>
> Samuel

I see, we still have time before hard freeze. So let's try to make it
for 2.6.