From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ad7hb-0006wN-0t for qemu-devel@nongnu.org; Mon, 07 Mar 2016 21:48:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ad7hX-00089H-1u for qemu-devel@nongnu.org; Mon, 07 Mar 2016 21:48:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43933) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ad7hW-000895-QZ for qemu-devel@nongnu.org; Mon, 07 Mar 2016 21:48:46 -0500 References: <56DD2430.2090509@redhat.com> <20160307111952.GB5169@var.bordeaux.inria.fr> From: Jason Wang Message-ID: <56DE3D7B.8050902@redhat.com> Date: Tue, 8 Mar 2016 10:48:27 +0800 MIME-Version: 1.0 In-Reply-To: <20160307111952.GB5169@var.bordeaux.inria.fr> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv9 0/10] slirp: Adding IPv6 support to Qemu -net user mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Thibault Cc: Thomas Huth , zhanghailiang , Li Zhijian , Stefan Hajnoczi , Dave Gilbert , Vasiliy Tolstov , qemu-devel@nongnu.org, Gonglei , Jan Kiszka , Huangpeng , Guillaume Subiron 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.