[Qemu-devel] Large patch set advice

[Qemu-devel] Large patch set advice

[Warner Losh]

Greetings,

I’ve foolishly volunteered to rebase all the changes that the bad-user mode folks have done to a recent master rev to get these changes upstreamed. A number of people have been working on this for a long time. It’s possible now to run almost any FreeBSD binary from all the architectures. We use it to do ‘native’ builds of tens of thousands of packages in a chroot (so building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are quite large (on the order of 42k lines), so I anticipate some bumps in moving this stuff upstream.

I’ve read through the Qemu patch submission material and have been contributing to FreeBSD for the last 20 years or so, so I have a notion of proper patch size. I’m half way through rebasing and curating the branch. Before I “finish” I thought I’d ask for some advise. I’m not the primary author of most of this stuff: it’s been accumulating for the past 4 years as different people come and go on the project...

Every Open Source project has its own quirks. Nobody wants the 520 commits on the branch that I started with (too many merged rather than rebases in the last 5 years). The 320 real commits are a mess. I’ve been preening them to get rid of the silly stuff like (back this out, put it back, all the ‘spelling/typo/white space’ fixes). At the end of the rebase, I still wasn’t to ‘the same’ so I had a bunch of ‘true up’ commits to make it the same...

So as with everything that’s developed over time, the early patches no longer are bistable because there’s later patches that fix the old APIs that were current at the start of the project to use newer APIs. That’s true both on the FreeBSD side as well as the Qemu side. Add to the mix that the original code was done by user X, and the fix by user Y (and sometimes Z and W, though the paper trail is unclear and W may just be who reported it).

Oh, and the number of SoBs in the original code is somewhat lacking. A tedious detail I need to chase down independent of all the other stuff.

So, there’s a bunch of changes at first to un-if-def-ify main.c, elfload.c and others to keep the MD code separate from the MI code. There’s a bunch of changes to implement this batch of system calls, or that batch. There’s some patches to implement PowerPC and then more to finish some architectures. Plus the above mentioned API chasing...

My first question seems almost rhetorical: that’s not an acceptable state of affairs, and curation is needed to upstream. Right?

My curation plans:

(1) Find where each of the bits of my ‘true up’ commits at the end are needed and to merge those bits back to those commits (I’m guessing it’s due to conflicts, or false conflict detection in git’s merging logic, since it was 4 hours and there attepts to get through the ‘git rebase -i master’ phase). These aren’t true changes anyway: just my screw up. Most of them are easy to find where they belong.

(2) back merge the API changes as best I can to as early in the stream as possible. One wrinkle here is that there’s a lot of code motion in the early patches to get the MI/MD stuff right, but even in the original commits, it never was very pure at all. And these changes are 2-3k lines long, but completely confined to bsd-user so maybe that’s OK. (I say completely, but sometimes there’s this or that thing added to a Makefile).

(3) Keep the classes of syscall implementation commits separate, but merge back the API and/or related new syscalls that were added. It’s a tradeoff between having 200 diffs that are minnows and 20 diffs that are related schools of fish vs one huge whale that’s just too big.

(4) Deal with the cases where multiple people have worked on a patch by putting SoBs for all of them (or reworking them if I can’t get a hold of the original authors) on the commits that are merged. I didn’t see a specific policy for this, but this seems sane. The alternative seems worse (having elements that don’t compile)

(5) Send them in small batches. I’d go insane trying to manage 200 concurrent code reviews, and I can’t image that I’m unique in this. I also image that fixes in the early part of the series may have ripples to the later parts if my past experience with these things has any relevance.

Thanks for any help you can give me.

Warner Losh
imp@FreeSBD.org

P.S. Yes, I get that we should have been more engaged all along.

Re: [Qemu-devel] Large patch set advice

[Thomas Huth]

On 25.04.2018 21:57, Warner Losh wrote:
> Greetings,
> 
> I’ve foolishly volunteered to rebase all the changes that the bad-user mode folks have done to a recent master rev to get these changes upstreamed.

Great that finally someone dares to do this step! But I hope the "bad"
was just a typo ;-)

[...]
> So as with everything that’s developed over time, the early patches no longer are bistable because there’s later patches that fix the old APIs that were current at the start of the project to use newer APIs.

bistable = bisectable? ... well, I guess that doesn't really matter here
since the upstream bsd-user code is completely broken anyway.

[...]
> Oh, and the number of SoBs in the original code is somewhat lacking. A tedious detail I need to chase down independent of all the other stuff.

Yes, please chase that down first. QEMU project is quite strict about SoBs.

[...]
> My first question seems almost rhetorical: that’s not an acceptable state of affairs, and curation is needed to upstream. Right?

I'd say this does not sound like an acceptable state, but it's finally
Peter who decides what patches get merged.

> My curation plans:
> 
> (1) Find where each of the bits of my ‘true up’ commits at the end are needed and to merge those bits back to those commits (I’m guessing it’s due to conflicts, or false conflict detection in git’s merging logic, since it was 4 hours and there attepts to get through the ‘git rebase -i master’ phase). These aren’t true changes anyway: just my screw up. Most of them are easy to find where they belong.
> 
> (2) back merge the API changes as best I can to as early in the stream as possible. One wrinkle here is that there’s a lot of code motion in the early patches to get the MI/MD stuff right, but even in the original commits, it never was very pure at all. And these changes are 2-3k lines long, but completely confined to bsd-user so maybe that’s OK. (I say completely, but sometimes there’s this or that thing added to a Makefile).
> 
> (3) Keep the classes of syscall implementation commits separate, but merge back the API and/or related new syscalls that were added. It’s a tradeoff between having 200 diffs that are minnows and 20 diffs that are related schools of fish vs one huge whale that’s just too big.
> 
> (4) Deal with the cases where multiple people have worked on a patch by putting SoBs for all of them (or reworking them if I can’t get a hold of the original authors) on the commits that are merged. I didn’t see a specific policy for this, but this seems sane. The alternative seems worse (having elements that don’t compile)

AFAIK, merging patches is ok, just mention it in the patch description.
For example if you have a patch with a description like this:

"
 Implement new feature XYZ

 This patch implements a new feature that is very cool.

 Signed-off-by: Alice
"

and then a fixup patch later:

"
 Fix XYZ

 This fixes a bug in XYZ

 Signed-off-by: Bob
"

You could merge them by supplying a note in square brackets:

"
 Implement new feature XYZ

 This patch implements a new feature that is very cool.

 Signed-off-by: Alice
 Signed-off-by: Bob
 [imp: merged fix from Bob into the patch from Alice]
 Signed-off-by: Warner Losh <imp@bsdimp.com>
"

I think it depends on the contents of Bob's patch whether you should
also include the complete patch description of Bob's patch or not.

> (5) Send them in small batches. I’d go insane trying to manage 200 concurrent code reviews, and I can’t image that I’m unique in this. I also image that fixes in the early part of the series may have ripples to the later parts if my past experience with these things has any relevance.

Yes, please keep the batch sizes small and digestible.

 Thanks,
  Thomas

Re: [Qemu-devel] Large patch set advice

[Markus Armbruster]

Warner Losh <imp@bsdimp.com> writes:

> Greetings,
>
> I’ve foolishly volunteered to rebase all the changes that the bad-user

You just won the "Typo of the Week" trophy, congratulations ;)

> mode folks have done to a recent master rev to get these changes
> upstreamed. A number of people have been working on this for a long
> time. It’s possible now to run almost any FreeBSD binary from all the
> architectures. We use it to do ‘native’ builds of tens of thousands of
> packages in a chroot (so building FreeBSD/arm packages on a FreeBSD
> amd64 box). The diffs are quite large (on the order of 42k lines), so
> I anticipate some bumps in moving this stuff upstream.
>
> I’ve read through the Qemu patch submission material and have been
> contributing to FreeBSD for the last 20 years or so, so I have a
> notion of proper patch size. I’m half way through rebasing and
> curating the branch. Before I “finish” I thought I’d ask for some
> advise. I’m not the primary author of most of this stuff: it’s been
> accumulating for the past 4 years as different people come and go on
> the project...

I know next to nothing about bsd-user, so my advice is rather general,
and may or may not apply to your specific situation.

> Every Open Source project has its own quirks. Nobody wants the 520
> commits on the branch that I started with (too many merged rather than
> rebases in the last 5 years). The 320 real commits are a mess. I’ve
> been preening them to get rid of the silly stuff like (back this out,
> put it back, all the ‘spelling/typo/white space’ fixes). At the end of
> the rebase, I still wasn’t to ‘the same’ so I had a bunch of ‘true up’
> commits to make it the same...
>
> So as with everything that’s developed over time, the early patches no
> longer are bistable because there’s later patches that fix the old
> APIs that were current at the start of the project to use newer
> APIs. That’s true both on the FreeBSD side as well as the Qemu
> side. Add to the mix that the original code was done by user X, and
> the fix by user Y (and sometimes Z and W, though the paper trail is
> unclear and W may just be who reported it).
>
> Oh, and the number of SoBs in the original code is somewhat lacking. A
> tedious detail I need to chase down independent of all the other
> stuff.
>
> So, there’s a bunch of changes at first to un-if-def-ify main.c,
> elfload.c and others to keep the MD code separate from the MI
> code. There’s a bunch of changes to implement this batch of system
> calls, or that batch. There’s some patches to implement PowerPC and
> then more to finish some architectures. Plus the above mentioned API
> chasing...
>
> My first question seems almost rhetorical: that’s not an acceptable
> state of affairs, and curation is needed to upstream. Right?
>
> My curation plans:
>
> (1) Find where each of the bits of my ‘true up’ commits at the end are
> needed and to merge those bits back to those commits (I’m guessing
> it’s due to conflicts, or false conflict detection in git’s merging
> logic, since it was 4 hours and there attepts to get through the ‘git
> rebase -i master’ phase). These aren’t true changes anyway: just my
> screw up. Most of them are easy to find where they belong.

This is standard operating procedure for rebase.

After the cleanup, every commit along the way should compile and pass
tests.

> (2) back merge the API changes as best I can to as early in the stream
> as possible. One wrinkle here is that there’s a lot of code motion in
> the early patches to get the MI/MD stuff right, but even in the
> original commits, it never was very pure at all. And these changes are
> 2-3k lines long, but completely confined to bsd-user so maybe that’s
> OK. (I say completely, but sometimes there’s this or that thing added
> to a Makefile).

If a subsystem is broken to begin with (I'm not saying bsd-user is!),
then changing the details of its breakage within your series can be
acceptable.

If something useful stops working within your patch series, you got a
problem.

If you're "only" groping your way from flawed to sensible arrangement of
source code, and some of your intermediate steps are less than spotless,
it's a judgement call.  Explanations in commit messages can go a long
way to getting your judgement accepted.

> (3) Keep the classes of syscall implementation commits separate, but
> merge back the API and/or related new syscalls that were added. It’s a
> tradeoff between having 200 diffs that are minnows and 20 diffs that
> are related schools of fish vs one huge whale that’s just too big.

As you said, it's a tradeoff.  Use your judgement, then work with your
reviewers.

> (4) Deal with the cases where multiple people have worked on a patch
> by putting SoBs for all of them (or reworking them if I can’t get a
> hold of the original authors) on the commits that are merged. I didn’t
> see a specific policy for this, but this seems sane. The alternative
> seems worse (having elements that don’t compile)

Joint works bearing all authors' S-o-b are fine.

> (5) Send them in small batches. I’d go insane trying to manage 200
> concurrent code reviews, and I can’t image that I’m unique in this. I
> also image that fixes in the early part of the series may have ripples
> to the later parts if my past experience with these things has any
> relevance.
>
> Thanks for any help you can give me.
>
> Warner Losh imp@FreeSBD.org
>
> P.S. Yes, I get that we should have been more engaged all along.

Correct :)

Re: [Qemu-devel] Large patch set advice

[Daniel P. Berrangé]

On Wed, Apr 25, 2018 at 01:57:01PM -0600, Warner Losh wrote:
> Every Open Source project has its own quirks. Nobody wants the 520 commits
> on the branch that I started with (too many merged rather than rebases in
> the last 5 years). The 320 real commits are a mess. I’ve been preening them
> to get rid of the silly stuff like (back this out, put it back, all the
> ‘spelling/typo/white space’ fixes). At the end of the rebase, I still
> wasn’t to ‘the same’ so I had a bunch of ‘true up’ commits to make it the
> same...

> My first question seems almost rhetorical: that’s not an acceptable state
> of affairs, and curation is needed to upstream. Right?

Yeah, no one is going to want to review 300 patches !

> (4) Deal with the cases where multiple people have worked on a patch by
> putting SoBs for all of them (or reworking them if I can’t get a hold
> of the original authors) on the commits that are merged. I didn’t see
> a specific policy for this, but this seems sane. The alternative seems
> worse (having elements that don’t compile)

The signed off by line is attesting that you agree to this:

   http://developercertificate.org/

IIUC nothing there requires you to add a SoB line for each author,
but you have to be confident that you can agree to terms a) & b),
given the the work you are building on top of.


> (5) Send them in small batches. I’d go insane trying to manage
> 200 concurrent code reviews, and I can’t image that I’m unique
> in this. I also image that fixes in the early part of the series
> may have ripples to the later parts if my past experience with
> these things has any relevance.

Can't  disagree with that !

> Thanks for any help you can give me.

IIUC, the current bsd-user code in GIT is completely broken & unusable.
After all 500 patches have been applied, I'm wondering how much of
the original code is actually left ? If it is a very small percentage,
an alternative strategy might be to send a patch deleting everything
in tree, and then treat your code as if you were adding bsd-user as a
brand new feature. This could potentially let you merge & restructure
the patches into a smaller more easily reviewable set of work. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Large patch set advice

[Peter Maydell]

On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
> I’ve foolishly volunteered to rebase all the changes that the bad-user
> mode folks have done to a recent master rev to get these changes upstreamed.
> A number of people have been working on this for a long time. It’s possible
> now to run almost any FreeBSD binary from all the architectures. We use it
> to do ‘native’ builds of tens of thousands of packages in a chroot (so
> building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are quite
> large (on the order of 42k lines), so I anticipate some bumps in moving
> this stuff upstream.

So, first up, thanks for agreeing to do this. It sounds from your
mail like you're already pretty well aware of the usual pitfalls
with this kind of work, and I don't really have much to add that's
QEMU specific that nobody else has said already.

One question I do have is about the other BSDs: bsd-user at least
in theory is supposed to support freebsd, netbsd and openbsd.
Your patchsets should fix freebsd, but do you know what the status
is of netbsd and openbsd? Upstream we do compiletest but no runtime
testing; I think last time I tried it they didn't work very well,
but it would be worth checking with the other BSDs downstream to
see if they're using bsd-user and to make sure we don't break anything
that is currently working for netbsd/openbsd...

thanks
-- PMM

Re: [Qemu-devel] Large patch set advice

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]

On 26.04.2018 10:11, Peter Maydell wrote:
> On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
>> I’ve foolishly volunteered to rebase all the changes that the bad-user
>> mode folks have done to a recent master rev to get these changes upstreamed.
>> A number of people have been working on this for a long time. It’s possible
>> now to run almost any FreeBSD binary from all the architectures. We use it
>> to do ‘native’ builds of tens of thousands of packages in a chroot (so
>> building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are quite
>> large (on the order of 42k lines), so I anticipate some bumps in moving
>> this stuff upstream.
> 
> So, first up, thanks for agreeing to do this. It sounds from your
> mail like you're already pretty well aware of the usual pitfalls
> with this kind of work, and I don't really have much to add that's
> QEMU specific that nobody else has said already.
> 
> One question I do have is about the other BSDs: bsd-user at least
> in theory is supposed to support freebsd, netbsd and openbsd.
> Your patchsets should fix freebsd, but do you know what the status
> is of netbsd and openbsd? Upstream we do compiletest but no runtime
> testing; I think last time I tried it they didn't work very well,
> but it would be worth checking with the other BSDs downstream to
> see if they're using bsd-user and to make sure we don't break anything
> that is currently working for netbsd/openbsd...
> 

I'm looking forward to see it updated for FreeBSD first. We can
reschedule NetBSD part for later.

Feel free to CC me for code review.

> thanks
> -- PMM
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 850 bytes --]

Re: [Qemu-devel] Large patch set advice

[Warner Losh]

On Thu, Apr 26, 2018 at 5:22 AM, Kamil Rytarowski <n54@gmx.com> wrote:

> On 26.04.2018 10:11, Peter Maydell wrote:
> > On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
> >> I’ve foolishly volunteered to rebase all the changes that the bad-user
> >> mode folks have done to a recent master rev to get these changes
> upstreamed.
> >> A number of people have been working on this for a long time. It’s
> possible
> >> now to run almost any FreeBSD binary from all the architectures. We use
> it
> >> to do ‘native’ builds of tens of thousands of packages in a chroot (so
> >> building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are
> quite
> >> large (on the order of 42k lines), so I anticipate some bumps in moving
> >> this stuff upstream.
> >
> > So, first up, thanks for agreeing to do this. It sounds from your
> > mail like you're already pretty well aware of the usual pitfalls
> > with this kind of work, and I don't really have much to add that's
> > QEMU specific that nobody else has said already.
> >
> > One question I do have is about the other BSDs: bsd-user at least
> > in theory is supposed to support freebsd, netbsd and openbsd.
> > Your patchsets should fix freebsd, but do you know what the status
> > is of netbsd and openbsd? Upstream we do compiletest but no runtime
> > testing; I think last time I tried it they didn't work very well,
> > but it would be worth checking with the other BSDs downstream to
> > see if they're using bsd-user and to make sure we don't break anything
> > that is currently working for netbsd/openbsd...
> >
>
> I'm looking forward to see it updated for FreeBSD first. We can
> reschedule NetBSD part for later.
>
> Feel free to CC me for code review.
>

Sure. Will do. Do you have NetBSD patches against the git repo as well?

Warner

Re: [Qemu-devel] Large patch set advice

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 1985 bytes --]

On 26.04.2018 21:53, Warner Losh wrote:
> On Thu, Apr 26, 2018 at 5:22 AM, Kamil Rytarowski <n54@gmx.com> wrote:
> 
>> On 26.04.2018 10:11, Peter Maydell wrote:
>>> On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
>>>> I’ve foolishly volunteered to rebase all the changes that the bad-user
>>>> mode folks have done to a recent master rev to get these changes
>> upstreamed.
>>>> A number of people have been working on this for a long time. It’s
>> possible
>>>> now to run almost any FreeBSD binary from all the architectures. We use
>> it
>>>> to do ‘native’ builds of tens of thousands of packages in a chroot (so
>>>> building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are
>> quite
>>>> large (on the order of 42k lines), so I anticipate some bumps in moving
>>>> this stuff upstream.
>>>
>>> So, first up, thanks for agreeing to do this. It sounds from your
>>> mail like you're already pretty well aware of the usual pitfalls
>>> with this kind of work, and I don't really have much to add that's
>>> QEMU specific that nobody else has said already.
>>>
>>> One question I do have is about the other BSDs: bsd-user at least
>>> in theory is supposed to support freebsd, netbsd and openbsd.
>>> Your patchsets should fix freebsd, but do you know what the status
>>> is of netbsd and openbsd? Upstream we do compiletest but no runtime
>>> testing; I think last time I tried it they didn't work very well,
>>> but it would be worth checking with the other BSDs downstream to
>>> see if they're using bsd-user and to make sure we don't break anything
>>> that is currently working for netbsd/openbsd...
>>>
>>
>> I'm looking forward to see it updated for FreeBSD first. We can
>> reschedule NetBSD part for later.
>>
>> Feel free to CC me for code review.
>>
> 
> Sure. Will do. Do you have NetBSD patches against the git repo as well?
> 

At the moment nothing sharable for NetBSD.

> Warner
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 850 bytes --]

Re: [Qemu-devel] Large patch set advice

[Warner Losh]

On Thu, Apr 26, 2018 at 2:11 AM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
> > I’ve foolishly volunteered to rebase all the changes that the bad-user
> > mode folks have done to a recent master rev to get these changes
> upstreamed.
> > A number of people have been working on this for a long time. It’s
> possible
> > now to run almost any FreeBSD binary from all the architectures. We use
> it
> > to do ‘native’ builds of tens of thousands of packages in a chroot (so
> > building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are
> quite
> > large (on the order of 42k lines), so I anticipate some bumps in moving
> > this stuff upstream.
>
> So, first up, thanks for agreeing to do this. It sounds from your
> mail like you're already pretty well aware of the usual pitfalls
> with this kind of work, and I don't really have much to add that's
> QEMU specific that nobody else has said already.
>

Yea, this isn't my first rodeo :)


> One question I do have is about the other BSDs: bsd-user at least
> in theory is supposed to support freebsd, netbsd and openbsd.
> Your patchsets should fix freebsd, but do you know what the status
> is of netbsd and openbsd? Upstream we do compiletest but no runtime
> testing; I think last time I tried it they didn't work very well,
> but it would be worth checking with the other BSDs downstream to
> see if they're using bsd-user and to make sure we don't break anything
> that is currently working for netbsd/openbsd...
>

I thought that was the empty set: I was under the impression that bsd-user
in upstream was basically totally broken and nothing non-trivial worked.
Since nothing is working, I'd posit that it follows that nobody could be
currently using it. I've certainly heard of no parallel efforts to make
similar changes for NetBSD/OpenBSD.

I have no plans to do any testing of NetBSD or OpenBSD binaries. The vast
majority of the patches are FreeBSD oriented, but the split is done such
that NetBSD and OpenBSD specific stuff can be augmented. Although
NetBSD/OpenBSD share a history with FreeBSD, only the first 150 syscalls or
so are the same. Around that time, the syscall numbers were assigned
independently, and the API/ABIs started to diverge, sometimes
significantly. The people that did the early work made sure that all the
BSDs could be supported, even if most of the focus has been on making
FreeBSD binaries work well.

Now, having said that, if somebody pops up and wants to do the work and
needs changes to make things work, I'm happy to work with them. I think
this is a reasonable approach, and matches what other projects I've been
involved with have expected.

Warner

Re: [Qemu-devel] Large patch set advice

[Peter Maydell]

On 26 April 2018 at 20:51, Warner Losh <imp@bsdimp.com> wrote:
> On Thu, Apr 26, 2018 at 2:11 AM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>> One question I do have is about the other BSDs: bsd-user at least
>> in theory is supposed to support freebsd, netbsd and openbsd.
>> Your patchsets should fix freebsd, but do you know what the status
>> is of netbsd and openbsd? Upstream we do compiletest but no runtime
>> testing; I think last time I tried it they didn't work very well,
>> but it would be worth checking with the other BSDs downstream to
>> see if they're using bsd-user and to make sure we don't break anything
>> that is currently working for netbsd/openbsd...
>
>
> I thought that was the empty set: I was under the impression that bsd-user
> in upstream was basically totally broken and nothing non-trivial worked.
> Since nothing is working, I'd posit that it follows that nobody could be
> currently using it. I've certainly heard of no parallel efforts to make
> similar changes for NetBSD/OpenBSD.

Well, I'm basically just asking you to reach out and check with
the other *BSD folks that they agree with that "it's totally broken"
analysis (which I guess they would). I see Kamil has given the
answer for NetBSD already.

thanks
-- PMM

Re: [Qemu-devel] Large patch set advice

[Warner Losh]

On Thu, Apr 26, 2018 at 2:11 AM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 25 April 2018 at 20:57, Warner Losh <imp@bsdimp.com> wrote:
> > I’ve foolishly volunteered to rebase all the changes that the bad-user
> > mode folks have done to a recent master rev to get these changes
> upstreamed.
> > A number of people have been working on this for a long time. It’s
> possible
> > now to run almost any FreeBSD binary from all the architectures. We use
> it
> > to do ‘native’ builds of tens of thousands of packages in a chroot (so
> > building FreeBSD/arm packages on a FreeBSD amd64 box). The diffs are
> quite
> > large (on the order of 42k lines), so I anticipate some bumps in moving
> > this stuff upstream.
>
> So, first up, thanks for agreeing to do this. It sounds from your
> mail like you're already pretty well aware of the usual pitfalls
> with this kind of work, and I don't really have much to add that's
> QEMU specific that nobody else has said already.
>
> One question I do have is about the other BSDs: bsd-user at least
> in theory is supposed to support freebsd, netbsd and openbsd.
> Your patchsets should fix freebsd, but do you know what the status
> is of netbsd and openbsd? Upstream we do compiletest but no runtime
> testing; I think last time I tried it they didn't work very well,
> but it would be worth checking with the other BSDs downstream to
> see if they're using bsd-user and to make sure we don't break anything
> that is currently working for netbsd/openbsd...
>

I've made inquiries. People are aware of the sbruno branch I'm working to
merge. The consensus is that for NetBSD and OpenBSD both the current
upstream code as well as the current sbruno branch code's status is totally
broken. The sbruno branch people have said looks interesting, but doesn't
materially change what's working.

The testing aspect has me intrigued. How hard would it be to add testing
done for bsd-user to your upstream tests? Before this project, I've only
ever been a qemu user, and even then only around the edges so I'm not
familiar with what's available. For the sake of argument, you can assume
FreeBSD has its own testbed we use, some useful subset of which could be
leveraged into an extended unit test.

Finally, another poster suggested that delete and repopulate would be an
option. While I haven't made any final decisions on whether I want to go
that way, I'd like to know from Peter if he would accept things that way. I
think it may be easier for me to submit something like that, but on the
other hand the boiling the patches down even from 300 to 190 (where the
work stands at the moment) has identified some interesting issues I need to
chase down (mostly relating to either inappropriate for upstreaming patches
mixed in, or relating to what looks like a mismerge a few months ago due to
git + long-lived-branch + merges having... I'll be nice and say 'challenges
when changes collide').

Thanks for all the advice so far and not giving me too much grief over my
epic typos :).

Warner

Re: [Qemu-devel] Large patch set advice

[Peter Maydell]

On 30 April 2018 at 15:44, Warner Losh <imp@bsdimp.com> wrote:
> The testing aspect has me intrigued. How hard would it be to add testing
> done for bsd-user to your upstream tests? Before this project, I've only
> ever been a qemu user, and even then only around the edges so I'm not
> familiar with what's available. For the sake of argument, you can assume
> FreeBSD has its own testbed we use, some useful subset of which could be
> leveraged into an extended unit test.

The problem we've traditionally had is that when you're building
and testing qemu on host architecture A, you can't guarantee to
have a suitable cross building environment for architecture B
so that you can build the test guest executables you want to run
under QEMU to test whether qemu-B works. (For linux-user we're
currently looking at using docker to provide consistent working cross
environments, but that is work-in-progress.)

> Finally, another poster suggested that delete and repopulate would be an
> option. While I haven't made any final decisions on whether I want to go
> that way, I'd like to know from Peter if he would accept things that way. I
> think it may be easier for me to submit something like that, but on the
> other hand the boiling the patches down even from 300 to 190 (where the work
> stands at the moment) has identified some interesting issues I need to chase
> down (mostly relating to either inappropriate for upstreaming patches mixed
> in, or relating to what looks like a mismerge a few months ago due to git +
> long-lived-branch + merges having... I'll be nice and say 'challenges when
> changes collide').

If we definitely don't care about the current functionality of upstream
for any of the BSDs (and it sounds like we don't -- thanks for looking
into that), then I think the main concern is "what is easiest to code
review". Which approach gives the most digestible patches for code
review probably depends on how much of the existing code and structure
remains after all the changes; that's a judgement call you're probably
in a better position to make. Either way I would still like to see
patches that are each of a reasonable size and provide coherent chunks of
functionality.

thanks
-- PMM