Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
Hello, Doug Evans, le lun. 08 févr. 2021 10:59:01 -0800, a ecrit: > Samuel, how do qemu patches involving libslirp changes usually work? Well, we haven't had many yet actually :) > Should I have held off submitting the qemu patch until the libslirp > prerequisite has been added to qemu's tree, No, as this example shows, there are iterations that can happen on the qemu side before we have something we can commit, so better do both in parallel. I don't know what qemu people think about the slirp submodule: do qemu prefers to only track stable branchs, or would it be fine to track the master branch? I guess you'd prefer to have a slirp stable release you can depend on when releasing qemu, so perhaps better wait for a slirp release before bumping in qemu, just to be safe? Which doesn't mean avoiding submitting patchqueues that depend on it before that, better go in parallel. > or maybe I should include the libslirp patch so that people can at least apply > it (with a caveat saying the patch is already in libslirp.git) until it's > added > to the qemu tree? Not sure what is best here. We at least need something so that people are not confused by the patchqueue calling some function that doesn't exist in the submodule yet. Samuel
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
On Mon, Feb 8, 2021 at 3:09 AM Philippe Mathieu-Daudé wrote: > Hi Doug, > > On 2/3/21 10:37 PM, dje--- via wrote: > > ... in preparation for adding ipv6 host forwarding support. > > Please duplicate subject line, else this commit description as it > doesn't make sense. > Hmmm. Is this a bug in git format-patch/send-email? I agree the current behaviour is suboptimal ... Perhaps there's an option I'm not adding? Or does one manually work around this? > --- > > net/slirp.c | 200 +--- > > slirp | 2 +- > > 2 files changed, 130 insertions(+), 72 deletions(-) > > > ... > > > diff --git a/slirp b/slirp > > index 8f43a99191..358c0827d4 16 > > --- a/slirp > > +++ b/slirp > > @@ -1 +1 @@ > > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > > +Subproject commit 358c0827d49778f016312bfb4167fe639900681f > > > > When updating submodules, please describe changes (usually - > when possible - a previous commit updating the submodule is > preferred). > > I can not apply your patch using either > https://git.qemu.org/git/libslirp.git or > https://gitlab.freedesktop.org/slirp/libslirp.git: > > fatal: bad object 358c0827d49778f016312bfb4167fe639900681f > I think that's expected until the patch has been merged from libslirp into qemu 's tree. Samuel, how do qemu patches involving libslirp changes usually work? Should I have held off submitting the qemu patch until the libslirp prerequisite has been added to qemu's tree, or maybe I should include the libslirp patch so that people can at least apply it (with a caveat saying the patch is already in libslirp.git) until it's added to the qemu tree?
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
Hi Doug, On 2/3/21 10:37 PM, dje--- via wrote: > ... in preparation for adding ipv6 host forwarding support. Please duplicate subject line, else this commit description as it doesn't make sense. > --- > net/slirp.c | 200 +--- > slirp | 2 +- > 2 files changed, 130 insertions(+), 72 deletions(-) > ... > diff --git a/slirp b/slirp > index 8f43a99191..358c0827d4 16 > --- a/slirp > +++ b/slirp > @@ -1 +1 @@ > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > +Subproject commit 358c0827d49778f016312bfb4167fe639900681f > When updating submodules, please describe changes (usually - when possible - a previous commit updating the submodule is preferred). I can not apply your patch using either https://git.qemu.org/git/libslirp.git or https://gitlab.freedesktop.org/slirp/libslirp.git: fatal: bad object 358c0827d49778f016312bfb4167fe639900681f
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
On Wed, Feb 3, 2021 at 2:15 PM Samuel Thibault wrote: > Doug Evans, le mer. 03 févr. 2021 13:37:28 -0800, a ecrit: > > ... in preparation for adding ipv6 host forwarding support. > > Reviewed-by: Samuel Thibault > > except > > > diff --git a/slirp b/slirp > > index 8f43a99191..358c0827d4 16 > > --- a/slirp > > +++ b/slirp > > @@ -1 +1 @@ > > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > > +Subproject commit 358c0827d49778f016312bfb4167fe639900681f > > Which, I would say, deserves its own commit? > Yep. Still getting used to patch submission in the presence of submodules ...
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
Doug Evans, le mer. 03 févr. 2021 13:37:28 -0800, a ecrit: > ... in preparation for adding ipv6 host forwarding support. Reviewed-by: Samuel Thibault except > diff --git a/slirp b/slirp > index 8f43a99191..358c0827d4 16 > --- a/slirp > +++ b/slirp > @@ -1 +1 @@ > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > +Subproject commit 358c0827d49778f016312bfb4167fe639900681f Which, I would say, deserves its own commit?
Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing
On Wed, Feb 3, 2021 at 1:37 PM Doug Evans wrote: > ... in preparation for adding ipv6 host forwarding support. > --- > net/slirp.c | 200 +--- > slirp | 2 +- > 2 files changed, 130 insertions(+), 72 deletions(-) > > diff --git a/net/slirp.c b/net/slirp.c > index be914c0be0..a21a313302 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -631,15 +631,83 @@ static SlirpState *slirp_lookup(Monitor *mon, const > char *id) Yeah, the Signed-off-by line is missing here. Will add in next version, but will wait for further comments.
