qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-fo


From: Doug Evans
Subject: Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support
Date: Wed, 12 May 2021 12:50:56 -0700

On Wed, May 12, 2021 at 10:18 AM Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
Hi

On Wed, May 12, 2021 at 8:43 PM Doug Evans <dje@google.com> wrote:
Ping.

On Fri, May 7, 2021 at 8:46 AM Doug Evans <dje@google.com> wrote:
On Fri, May 7, 2021 at 8:23 AM Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
Hi

[...]


Changes from v5:

1/4 slirp: Advance libslirp submodule to current master
NOTE TO REVIEWERS: It may be a better use of everyone's time if a
maintainer takes on advancing QEMU's libslirp to libslirp's master.
Beyond that, I really don't know what to do except submit this patch as
is currently provided.


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

It can do, but it should rather be a diff of the commits that are new, those that were not in the stable branch.


Can you elaborate?
E.g., Should a new libslirp release be made first, and then update qemu-master to that new release?


Hey Marc-André and Samuel,
What's the next step here (if any)?

There isn't much point in bumping qemu dependency if it doesn't make use of the new API. Thus first step is to get the rest of the series reviewed/approved imho.


I'm not suggesting the dependency be bumped prior to the entire series being approved.
By "next step" I meant whether this patch (1/4) in the series is done.
The question I asked: "Should a new libslirp release be made and then have qemu-master use that (*1)?" was outstanding as it wasn't(isn't) clear whether the plan was indeed to first make a new libslirp release (even taking into account the comments on patch 3/4).

(*1): When using QEMU-provided libslirp. When not using QEMU-provided slirp, of course, be compatible with the libslirp that is being used.
Thanks for bringing that to my attention btw, it's easy to forget given that QEMU provides its own copy of libslirp and I have completely left that out of v1 to v5.



Would it be preferable to make a new libslirp release instead?

yes, as I said in some other path review, we would need a libslirp release AND compatiblity with older slirp releases.


Indeed! I was, perhaps errantly, treating them as orthogonal discussions.
[On the theory that:
- if we resolve all issues for each piece of the current iteration then that will reduce the number of iterations,
and I'm not sure patch 1/4 is complete and what happens next for it (given that a separate repo is involved)
- discussions of making a new libslirp release can proceed independent of updating patch 3/4
That was the theory anyway.]



[I also don't understand the comment "it should rather be a diff of the commits that are new, ...".
I haven't seen this request before (apologies if I missed it), but more importantly
isn't it trivial to generate such diffs, without adding them to the email?
I could be missing something of course.]

At some point a new libslirp release needs to be made, and at some point QEMU needs to be updated to use it.
Seems like now is a good time, but maybe there are reasons to prefer not doing so now.
I can imagine QEMU wanting to always use a stable branch of libslirp,
so I just want to be absolutely clear that switching QEMU to use libslirp's master branch is desired,
and if anything more is needed from me.
I'm happy with whatever you decide, but I don't want to waste your time guessing and then having to iterate when I guess wrong.

You need to rework the series to be compatible with current libslirp. That may be one of the reason why you don't get more reviews, Jason, the maintainer, may expect you to do that based on feedback received earlier.


I'm indeed updating v7 in this series to be compatible with current libslirp, but let's get the issue of a new libslirp release resolved too.

Btw, can you elaborate on "should rather be a diff of the commits that are new"?
Up until now I've been told to provide "git shortlog old..new" output.
The patch itself is just a one-liner to update the subproject sha1.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]