Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote: > > +switch (iph->ip_v) { > > +case IPVERSION: > > if (iph->ip_dst.s_addr == 0) { > > /* 0.0.0.0 can not be a destination address, something went wrong, > > * avoid making it worse */ > > That indentation looks

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote: > Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe > it would be nicer to do it in one go instead (after splitting off the > arp_requested renaming)? Please discuss about it with the people who asked for it. I won't

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 15:01, Samuel Thibault wrote: > Just to explain. > > I'm fine with revamping the patches, provided that we eventually > converge somewhere. > > What I'm afraid of is that splitting patches in yet more many pieces, > revamping the code yet more (moving code into their own functions,

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 14:38, Thomas Huth wrote: > On 11/12/15 01:15, Samuel Thibault wrote: >> From: Guillaume Subiron >> >> Basically, this patch replaces "arp" by "resolution" every time "arp" >> means "mac resolution" and not specifically ARP. >> >> Some indentation problems are

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Just to explain. I'm fine with revamping the patches, provided that we eventually converge somewhere. What I'm afraid of is that splitting patches in yet more many pieces, revamping the code yet more (moving code into their own functions, etc.) will only make the patch series look even bigger

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote: > So maybe it's better to do smaller steps instead: Would it for example > make sense to split the whole series into two parts - first a series > that does all the preparation and cleanup patches. And then once that > has been reviewed and

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 01:15, Samuel Thibault wrote: > From: Guillaume Subiron > > Basically, this patch replaces "arp" by "resolution" every time "arp" > means "mac resolution" and not specifically ARP. > > Some indentation problems are solved in functions that will be modified > in

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 14:43, Thomas Huth wrote: > On 11/12/15 14:38, Thomas Huth wrote: >> On 11/12/15 01:15, Samuel Thibault wrote: >>> From: Guillaume Subiron >>> >>> Basically, this patch replaces "arp" by "resolution" every time "arp" >>> means "mac resolution" and not specifically

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 15:55, Samuel Thibault wrote: > Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote: >> So maybe it's better to do smaller steps instead: Would it for example >> make sense to split the whole series into two parts - first a series >> that does all the preparation and cleanup patches.

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Laszlo Ersek, on Fri 11 Dec 2015 16:40:32 +0100, wrote: > > Sounds good, ... then I'd suggest to sent the preparation patches > > separately next time and get them accepted first. > > And then the next reviewer will say, "nice, but it would be even nicer > to see what *motivates* these

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

meta On 12/11/15 16:09, Thomas Huth wrote: > On 11/12/15 15:55, Samuel Thibault wrote: >> Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote: >>> So maybe it's better to do smaller steps instead: Would it for example >>> make sense to split the whole series into two parts - first a series >>>

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 12/11/2015 08:40 AM, Laszlo Ersek wrote: > meta > > On 12/11/15 16:09, Thomas Huth wrote: >> On 11/12/15 15:55, Samuel Thibault wrote: >>> Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote: So maybe it's better to do smaller steps instead: Would it for example make sense to split

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 12/11/15 17:17, Eric Blake wrote: > On 12/11/2015 08:40 AM, Laszlo Ersek wrote: >> meta >> >> On 12/11/15 16:09, Thomas Huth wrote: >>> On 11/12/15 15:55, Samuel Thibault wrote: Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote: > So maybe it's better to do smaller steps instead:

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

On 11/12/15 14:47, Samuel Thibault wrote: > Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote: >> Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe >> it would be nicer to do it in one go instead (after splitting off the >> arp_requested renaming)? > > Please discuss

Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote: > Also, I'd prefer if you could split this patch into two: One for > renaming the "arp_requested" field, and one for adding the additional > logic with the switch statement (since there are two different kind of > changes). Ok, at least it's

[Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

From: Guillaume Subiron Basically, this patch replaces "arp" by "resolution" every time "arp" means "mac resolution" and not specifically ARP. Some indentation problems are solved in functions that will be modified in the next patches (ip_input…). In if_encap, a switch is

[Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Basically, this patch replaces arp by resolution every time arp means mac resolution and not specifically ARP. Some indentation problems are solved in functions that will be modified in the next patches (ip_input…). In if_encap, a switch is added to prepare for the IPv6 case. Some code is

[Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff

Basically, this patch replaces arp by resolution every time arp means mac resolution and not specifically ARP. Some indentation problems are solved in functions that will be modified in the next patches (ip_input…). In if_encap, a switch is added to prepare for the IPv6 case. Some code is