mbox

[PULL,for-2.4,0/7] update ipxe roms, fix efi support

Message ID 1437143854-2826-1-git-send-email-kraxel@redhat.com
State New
Headers show

Pull-request

git://git.kraxel.org/qemu tags/pull-ipxe-20150717-1

Message

Gerd Hoffmann July 17, 2015, 2:37 p.m. UTC
Hi,

This pull finally fixes the efi boot support.  ipxe is updated to the
latest master, two non-upstream commits needed to make efi work are
added on top, and the build process is tweaked a bit.

The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
before merging this pull request.

please pull,
  Gerd

The following changes since commit 2d5ee9e7a7dd495d233cf9613a865f63f88e3375:

  Merge remote-tracking branch 'remotes/lalrae/tags/mips-20150716' into staging (2015-07-16 10:40:23 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-ipxe-20150717-1

for you to fetch changes up to 4f0c601b71e53a7d225a1913b784242400788991:

  ipxe: update binaries (2015-07-16 17:39:12 +0200)

----------------------------------------------------------------
update ipxe roms, fix efi support

----------------------------------------------------------------
Gerd Hoffmann (7):
      ipxe: update from 35c53797 to 24112d9 (upstream/master)
      ipxe: update to 87981bb (qemu)
      ipxe: rm local config in cleanup
      ipxe: disable load file protocol
      ipxe: add qemu branding
      ipxe: don't override GITVERSION
      ipxe: update binaries

 pc-bios/efi-e1000.rom       | Bin 197120 -> 192512 bytes
 pc-bios/efi-eepro100.rom    | Bin 197632 -> 192512 bytes
 pc-bios/efi-ne2k_pci.rom    | Bin 195584 -> 190976 bytes
 pc-bios/efi-pcnet.rom       | Bin 195584 -> 190976 bytes
 pc-bios/efi-rtl8139.rom     | Bin 200192 -> 194560 bytes
 pc-bios/efi-virtio.rom      | Bin 194048 -> 188928 bytes
 roms/Makefile               |   9 +++++----
 roms/config.ipxe.branding.h |   2 ++
 roms/config.ipxe.general.h  |   1 +
 roms/ipxe                   |   2 +-
 10 files changed, 9 insertions(+), 5 deletions(-)
 create mode 100644 roms/config.ipxe.branding.h

Comments

Peter Maydell July 20, 2015, 10:04 a.m. UTC | #1
On 17 July 2015 at 15:37, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> This pull finally fixes the efi boot support.  ipxe is updated to the
> latest master, two non-upstream commits needed to make efi work are
> added on top, and the build process is tweaked a bit.
>
> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
> before merging this pull request.

Is this supposed to happen automatically, or does somebody
need to manually do something for that to happen?

thanks
-- PMM
Peter Maydell July 21, 2015, 8:21 a.m. UTC | #2
On 20 July 2015 at 11:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 July 2015 at 15:37, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>   Hi,
>>
>> This pull finally fixes the efi boot support.  ipxe is updated to the
>> latest master, two non-upstream commits needed to make efi work are
>> added on top, and the build process is tweaked a bit.
>>
>> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>> before merging this pull request.
>
> Is this supposed to happen automatically, or does somebody
> need to manually do something for that to happen?

I need an answer to this question or this pull will miss rc2 :-(

-- PMM
Paolo Bonzini July 21, 2015, 8:51 a.m. UTC | #3
On 21/07/2015 10:21, Peter Maydell wrote:
>>> >> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>>> >> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>>> >> before merging this pull request.
>> >
>> > Is this supposed to happen automatically, or does somebody
>> > need to manually do something for that to happen?
> I need an answer to this question or this pull will miss rc2 :-(

Looks like it needs to be done manually.

Paolo
Peter Maydell July 21, 2015, 1:27 p.m. UTC | #4
On 21 July 2015 at 09:51, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 21/07/2015 10:21, Peter Maydell wrote:
>>>> >> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>>>> >> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>>>> >> before merging this pull request.
>>> >
>>> > Is this supposed to happen automatically, or does somebody
>>> > need to manually do something for that to happen?
>> I need an answer to this question or this pull will miss rc2 :-(
>
> Looks like it needs to be done manually.

I think we're mirroring git://git.ipxe.org/ipxe.git, so the ipxe
changes need to be merged into that repo first.

Gerd, have you submitted those to the ipxe upstream?

thanks
-- PMM
Paolo Bonzini July 21, 2015, 1:47 p.m. UTC | #5
On 21/07/2015 15:27, Peter Maydell wrote:
>> > On 21/07/2015 10:21, Peter Maydell wrote:
>>>>>>> >>>> >> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>>>>>>> >>>> >> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>>>>>>> >>>> >> before merging this pull request.
>>>>> >>> >
>>>>> >>> > Is this supposed to happen automatically, or does somebody
>>>>> >>> > need to manually do something for that to happen?
>>> >> I need an answer to this question or this pull will miss rc2 :-(
>> >
>> > Looks like it needs to be done manually.
> I think we're mirroring git://git.ipxe.org/ipxe.git, so the ipxe
> changes need to be merged into that repo first.
> 
> Gerd, have you submitted those to the ipxe upstream?

Multiple times in the last six months.

Paolo
Peter Maydell July 21, 2015, 1:57 p.m. UTC | #6
On 21 July 2015 at 14:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/07/2015 15:27, Peter Maydell wrote:
>>> > On 21/07/2015 10:21, Peter Maydell wrote:
>>>>>>>> >>>> >> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>>>>>>>> >>>> >> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>>>>>>>> >>>> >> before merging this pull request.
>>>>>> >>> >
>>>>>> >>> > Is this supposed to happen automatically, or does somebody
>>>>>> >>> > need to manually do something for that to happen?
>>>> >> I need an answer to this question or this pull will miss rc2 :-(
>>> >
>>> > Looks like it needs to be done manually.
>> I think we're mirroring git://git.ipxe.org/ipxe.git, so the ipxe
>> changes need to be merged into that repo first.
>>
>> Gerd, have you submitted those to the ipxe upstream?
>
> Multiple times in the last six months.

So is the proposal here that we ship a non-upstream IPXE?
That was less than entirely clear from the pull request...

-- PMM
Paolo Bonzini July 21, 2015, 2:09 p.m. UTC | #7
On 21/07/2015 15:57, Peter Maydell wrote:
> On 21 July 2015 at 14:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 21/07/2015 15:27, Peter Maydell wrote:
>>>>> On 21/07/2015 10:21, Peter Maydell wrote:
>>>>>>>>>>>>>>> The ipxe changes are pushed to git://git.kraxel.org/ipxe (branch qemu,
>>>>>>>>>>>>>>> tag qemu-2.4).  They should be mirrored to git://git.qemu.org/ipxe.git
>>>>>>>>>>>>>>> before merging this pull request.
>>>>>>>>>>>
>>>>>>>>>>> Is this supposed to happen automatically, or does somebody
>>>>>>>>>>> need to manually do something for that to happen?
>>>>>>> I need an answer to this question or this pull will miss rc2 :-(
>>>>>
>>>>> Looks like it needs to be done manually.
>>> I think we're mirroring git://git.ipxe.org/ipxe.git, so the ipxe
>>> changes need to be merged into that repo first.
>>>
>>> Gerd, have you submitted those to the ipxe upstream?
>>
>> Multiple times in the last six months.
> 
> So is the proposal here that we ship a non-upstream IPXE?
> That was less than entirely clear from the pull request...

Well, it did say "This pull finally fixes the efi boot support.  ipxe is
updated to the latest master, two non-upstream commits needed to make
efi work are added on top, and the build process is tweaked a bit".

Paolo
Peter Maydell July 21, 2015, 2:25 p.m. UTC | #8
On 21 July 2015 at 15:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/07/2015 15:57, Peter Maydell wrote:
>> On 21 July 2015 at 14:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> So is the proposal here that we ship a non-upstream IPXE?
>> That was less than entirely clear from the pull request...
>
> Well, it did say "This pull finally fixes the efi boot support.  ipxe is
> updated to the latest master, two non-upstream commits needed to make
> efi work are added on top, and the build process is tweaked a bit".

Right, but if you want us to switch from "we just mirror
upstream ipxe" to "we have our own ipxe" then it's probably
better to start with that, rather than by submitting a pullreq
that can't be applied until we switch our ipxe workflow.

Do we really need this for 2.4, or can we wait and sort
this out afterwards? It feels a bit late in the release
cycle to start doing this kind of thing to me.

-- PMM
Paolo Bonzini July 21, 2015, 2:28 p.m. UTC | #9
On 21/07/2015 16:25, Peter Maydell wrote:
>> >
>> > Well, it did say "This pull finally fixes the efi boot support.  ipxe is
>> > updated to the latest master, two non-upstream commits needed to make
>> > efi work are added on top, and the build process is tweaked a bit".
> Right, but if you want us to switch from "we just mirror
> upstream ipxe" to "we have our own ipxe" then it's probably
> better to start with that, rather than by submitting a pullreq
> that can't be applied until we switch our ipxe workflow.
> 
> Do we really need this for 2.4, or can we wait and sort
> this out afterwards? It feels a bit late in the release
> cycle to start doing this kind of thing to me.

Well, it is a bugfix.  shim.efi doesn't work with upstream iPXE, and
hence doesn't work with the ROMs currently distributed by QEMU (Fedora
applies the patches already).

The patches have missed 2.3 and 2.4 because Gerd has been sending them
again upstream every month.

That said, I see your point.  It's probably not of utmost importance as
long as OVMF remains non-free and hence not shipped by most distributions.

Paolo
Stefan Hajnoczi July 21, 2015, 4:10 p.m. UTC | #10
On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/07/2015 16:25, Peter Maydell wrote:
>>> >
>>> > Well, it did say "This pull finally fixes the efi boot support.  ipxe is
>>> > updated to the latest master, two non-upstream commits needed to make
>>> > efi work are added on top, and the build process is tweaked a bit".
>> Right, but if you want us to switch from "we just mirror
>> upstream ipxe" to "we have our own ipxe" then it's probably
>> better to start with that, rather than by submitting a pullreq
>> that can't be applied until we switch our ipxe workflow.
>>
>> Do we really need this for 2.4, or can we wait and sort
>> this out afterwards? It feels a bit late in the release
>> cycle to start doing this kind of thing to me.
>
> Well, it is a bugfix.  shim.efi doesn't work with upstream iPXE, and
> hence doesn't work with the ROMs currently distributed by QEMU (Fedora
> applies the patches already).
>
> The patches have missed 2.3 and 2.4 because Gerd has been sending them
> again upstream every month.
>
> That said, I see your point.  It's probably not of utmost importance as
> long as OVMF remains non-free and hence not shipped by most distributions.

With regards to git.qemu.org mirroring:
I could update the ipxe.git mirror URL on git.qemu.org to Gerd's
public repo and change the description to indicate that this includes
out-of-tree patches.  Please let me know if you'd like to go ahead.

I'd prefer it if we don't ship a patched ipxe since Paolo mentions
Fedora already has a fix in place.  Instead of propagating that fix
into QEMU, let's focus on ipxe upstream to solve the problem.

I've reviewed the ipxe-devel archives and see that patches have been
on the list for weeks/months without replies.  I didn't see ping
emails though so maybe it just requires a bit of poking via email or
IRC.

We either need to figure out how to get attention upstream or work
with others to add upstream maintainers.  I see that Hannes Reinecke
also has patches on ipxe-devel that look ignored, so Gred and Laszlo
are not the only ones struggling to get patches upstream into ipxe.

Stefan
Peter Maydell July 21, 2015, 4:18 p.m. UTC | #11
On 21 July 2015 at 17:10, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> With regards to git.qemu.org mirroring:
> I could update the ipxe.git mirror URL on git.qemu.org to Gerd's
> public repo and change the description to indicate that this includes
> out-of-tree patches.  Please let me know if you'd like to go ahead.
>
> I'd prefer it if we don't ship a patched ipxe since Paolo mentions
> Fedora already has a fix in place.  Instead of propagating that fix
> into QEMU, let's focus on ipxe upstream to solve the problem.

I think this would be my preference too. I wouldn't absolutely
rule out shipping a patched ipxe, but I think it should be a
last resort and not something we do at this point in our
release cycle.

thanks
-- PMM
Laszlo Ersek July 21, 2015, 10:58 p.m. UTC | #12
On 07/21/15 18:10, Stefan Hajnoczi wrote:
> On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 21/07/2015 16:25, Peter Maydell wrote:
>>>>>
>>>>> Well, it did say "This pull finally fixes the efi boot support.  ipxe is
>>>>> updated to the latest master, two non-upstream commits needed to make
>>>>> efi work are added on top, and the build process is tweaked a bit".
>>> Right, but if you want us to switch from "we just mirror
>>> upstream ipxe" to "we have our own ipxe" then it's probably
>>> better to start with that, rather than by submitting a pullreq
>>> that can't be applied until we switch our ipxe workflow.
>>>
>>> Do we really need this for 2.4, or can we wait and sort
>>> this out afterwards? It feels a bit late in the release
>>> cycle to start doing this kind of thing to me.
>>
>> Well, it is a bugfix.  shim.efi doesn't work with upstream iPXE, and
>> hence doesn't work with the ROMs currently distributed by QEMU (Fedora
>> applies the patches already).
>>
>> The patches have missed 2.3 and 2.4 because Gerd has been sending them
>> again upstream every month.
>>
>> That said, I see your point.  It's probably not of utmost importance as
>> long as OVMF remains non-free and hence not shipped by most distributions.
> 
> With regards to git.qemu.org mirroring:
> I could update the ipxe.git mirror URL on git.qemu.org to Gerd's
> public repo and change the description to indicate that this includes
> out-of-tree patches.  Please let me know if you'd like to go ahead.
> 
> I'd prefer it if we don't ship a patched ipxe since Paolo mentions
> Fedora already has a fix in place.

RHEL doesn't (yet). In fact these QEMU patches are the result of a long
(and mostly fruitless) effort to get the ipxe patches upstream -- for
RHEL we prefer *something* to point at as "upstream".

> Instead of propagating that fix
> into QEMU, let's focus on ipxe upstream to solve the problem.

How's this for focus:

(1) [PATCH] efi_snp: improve compliance with the
            EFI_SIMPLE_NETWORK_PROTOCOL spec
    http://thread.gmane.org/gmane.network.ipxe.devel/3799
    Date: 2015-Jan-27
    feedback: none

(2) [PATCH] efi_snp: improve compliance with the
            EFI_SIMPLE_NETWORK_PROTOCOL spec
    http://thread.gmane.org/gmane.network.ipxe.devel/3955
    Date: 2015-Feb-10
    feedback: zero

(3) [PATCH] [efi] make load file protocol optional
    http://thread.gmane.org/gmane.network.ipxe.devel/3815
    Date: 2015-Feb-11
    feedback: the patch destroys the user's ability to use
              most features of iPXE
    our point: we don't care about most features of iPXE, we just need
               it for EFI drivers (Simple Network Protocol instances)
    result: nothing

(4) [RESENT PATCH 0/2] efi boot fixes.
    http://thread.gmane.org/gmane.network.ipxe.devel/3854
    Date: 2015-Mar-10
    feedback: zilch

(5) [RESEND PATCH] efi_snp: improve compliance with the
                   EFI_SIMPLE_NETWORK_PROTOCOL spec
    http://thread.gmane.org/gmane.network.ipxe.devel/3934
    Date: 2015-Apr-14
    feedback: nada

(6) [PATCH] efi_snp: improve compliance with the
            EFI_SIMPLE_NETWORK_PROTOCOL spec
    http://thread.gmane.org/gmane.network.ipxe.devel/4096
    Date: 2015-Jun-10
    feedback: null

So, let us *not* focus on getting these into upstream ipxe. The mailing
list is a black hole. The upstream maintainer behaves completely
unpredictably. For example, look at this one example:

  [PATCH 0/2] virtio-net shutdown fix for ExitBootServices()
  http://thread.gmane.org/gmane.network.ipxe.devel/3918
  Date: 2015-Apr-10

For this submission, the maintainer provided good feedback, then decided
to rewrite the patches (which was a good call, no doubt about it), then
went ahead and committed that patch, and even gave credit:

commit 755d2b8f6be681a2e620534b237471b75f28ed8c
Author: Michael Brown <mcb30@ipxe.org>
Date:   Mon Apr 13 12:06:59 2015 +0100

    [efi] Ensure drivers are disconnected when ExitBootServices() is
    called
    [...]
    Originally-fixed-by: Laszlo Ersek <lersek@redhat.com>
    Tested-by: Laszlo Ersek <lersek@redhat.com>
    Signed-off-by: Michael Brown <mcb30@ipxe.org>

(Therefore this patch is actually covered by the upstream rebase in
[PULL 1/7] here.)

And then, *just one day* after this commit, Gerd submits (5) -- and
complete silence.

> I've reviewed the ipxe-devel archives and see that patches have been
> on the list for weeks/months without replies.  I didn't see ping
> emails though so maybe it just requires a bit of poking via email or
> IRC.

Rather than pings, Gerd kept resending the actual patches. That's a
strictly stronger kind of "reminder". So no, pings are useless.

Regarding IRC, let me quote the following from freenode | #ipxe, date
2015-Jan-23 (ie. four days before posting (1)). The first quote is about
the patch in (3) -- the implementation looked differently at that point,
but the behavior was identical:

<mcb30>  Patches to modify our LOAD_FILE_PROTOCOL to support loading
         arbitrary files would be fine
<lersek> mcb30, thanks. :)
<mcb30>  but there's no way I'm turning UEFI iPXE back into being a dumb
         SNP driver with the appalling UEFI UX
<mcb30>  Consider what happens when a user attempts normal network boot
         and something goes wrong (e.g. the file doesn't exist)
<mcb30>  iPXE displays a UI with a meaningful error message and a shell
         allowing you to troubleshoot
<lersek> the UEFI boot option fails and the boot option processing
         continues.
<mcb30>  UEFI simply displays a dot on an otherwise blank screen and
         then returns to the boot menu
<lersek> yes
<lersek> if there are no more UEFI boot options to try
<lersek> I understand your point fully
<mcb30>  A dot on a blank screen is not a meaningful error message
<lersek> we have different goals here
<mcb30>  Understood
<lersek> the dot is not meant as an error message, it's progress info
         AFAIR
<mcb30>  In which case, a total absence of an error message is not a
         meaningful error message
<lersek> correct
<lersek> but it's consistent with the handling of other (not necessarily
         network related) boot options when they fail
<lersek> your main goal is to provide a nice iPXE experience, while we
         need some UEFI NIC drivers (for OVMF) in qemu-kvm guests
<lersek> so I thought I'd try to check with upstream developers before
         keeping these patches downstream only
<lersek> thanks!
<mcb30>  I would suggest adding code to support downloading arbitrary
         files via LOAD_FILE_PROTOCOL
<mcb30>  which would avoid a UX downgrade when network-booting a
         qemu-kvm guest
<mcb30>  (i.e. when not going through grub)

So, this is consistent with the feedback we'd actually get later for
(3). It makes no sense to struggle more with this patch.

Then, regarding the other patch (the one in (1)):

<lersek> mcb30, what about the second patch?
<mcb30>  lersek: sorry; I mssed the second patch
<mcb30>  Looking...
<lersek> mcb30, thanks -- my first msg in this channel was quite long &
         crowded
<mcb30>  lersek: looks okish.  I need to check the code again; it's been
         a long time since I looked at that.  In particular, is there
         any functional difference between the existing
         tx_count_interrupts (used I think to determine how many TX
         completions to return) and the producer/consumer counters that
         you have added?  If tx_count_interrupts is now redundant
         because the producer/consumer already include that information
         then it should probably be
<mcb30>  removed
<lersek> mcb30, that's a good question. The UEFI spec emphasizes that
         clearing / retrieving a recycled txbuf does not clear interrupt
         status if the caller doesn't ask for that too, and vice versa
         -- if the caller only asks about interrupt status, then no
         recycled txbuf is returned
<lersek> so they should remain independent
<lersek> but how exactly you calculate the interrupt status when the
         caller *does* ask for it, that's anyone's best guess
<lersek> the spec is unclear about it
<lersek> in OVMF's builtin virtio-net driver I think I just check if
         there are any pending outgoing & incoming packets, and set the
         corresponding bits
<lersek> I don't use a counter
<lersek> but it's very obscure indeed and no caller I know of seems to
         care about interrupt status
<lersek> which is why I didn't touch that part of the code
<lersek> I just kept those two things independent, because their
         independence *is* specified
<mcb30>  ok
<mcb30>  That makes sense
<lersek> (more precisely, re OVMF's builtin virtio-net driver, the RX
         intr bit is reported when more stuff is available for
         reception, and the TX intr bit is reported if we have
         transmitted at least one buffer that the caller queued with
         Transmit)
<lersek> mcb30, so can you pick up the 2nd patch from bugzilla, or
         should I send it to the list? (Actually if you deem the patch
         appropriate I can only send it to the list, because current
         master iPXE looks a bit different from our fork in RHEL, and
         the 2nd patch takes different forms accordingly.)

I got no further replies here. Based on the apparently positive feedback
on IRC, four days later I posted (1). No feedback.

> We either need to figure out how to get attention upstream

Good luck with that. In my educated experience: completely unpredictable
maintainer behavior, not a real community project.

> or work
> with others to add upstream maintainers.

When we can't get the maintainer's attention for our patches, and when
the maintainer tends to rewrite even those patches he more or less
likes, how do you propose we convince him to give *push access* to
random people?

> I see that Hannes Reinecke
> also has patches on ipxe-devel that look ignored, so Gred and Laszlo
> are not the only ones struggling to get patches upstream into ipxe.

I've said it several times (on other lists too), and I'll say it again:
ipxe is not an "open process" community project at this point. The last
half year, as Paolo indicated, and as I proved above, has been ample
experience.

--*--

On the technical level, let me summarize: we needed three patches in
total to get UEFI boot working:

#1 efi_snp: improve compliance with the EFI_SIMPLE_NETWORK_PROTOCOL spec
#2 [efi] make load file protocol optional
#3 [efi] Ensure drivers are disconnected when ExitBootServices() is
   called

Wrt. #1, the maintainer expressed agreement on IRC, but never replied to
patch emails.

Wrt. #2, the maintainer expressed strong disagreement (due to "user
interface" concerns) on both IRC and later on the mailing list.
Therefore the idea of upstreaming this patch is dead in the water. The
maintainer would accept an alternative that would take a huge
development effort, and would be useless for virtualization purposes
(ie. PXE-booting with OVMF in QEMU).

Wrt. #3, the maintainer decided to look at the patch, rewrite it, and
commit it. For some unfathomable reason. Maybe because he was Cc'd
directly on the patch email. I don't know. (The ipxe-devel list has
absolutely minimal traffic, why wouldn't he read it without explicit Cc?!)

This PULL is about getting #3 via rebase, and #1 and #2 as downstream
patches.

Thanks
Laszlo
Laszlo Ersek July 21, 2015, 11 p.m. UTC | #13
On 07/21/15 18:18, Peter Maydell wrote:
> On 21 July 2015 at 17:10, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> With regards to git.qemu.org mirroring:
>> I could update the ipxe.git mirror URL on git.qemu.org to Gerd's
>> public repo and change the description to indicate that this includes
>> out-of-tree patches.  Please let me know if you'd like to go ahead.
>>
>> I'd prefer it if we don't ship a patched ipxe since Paolo mentions
>> Fedora already has a fix in place.  Instead of propagating that fix
>> into QEMU, let's focus on ipxe upstream to solve the problem.
> 
> I think this would be my preference too. I wouldn't absolutely
> rule out shipping a patched ipxe, but I think it should be a
> last resort

Yes, it should be. And, we're already there.

> and not something we do at this point in our
> release cycle.

Not contesting that.

Laszlo
Laszlo Ersek July 21, 2015, 11:22 p.m. UTC | #14
Oops, I got carried away a little bit, and forgot about a crucial detail:

On 07/22/15 00:58, Laszlo Ersek wrote:

[snip]

> (1) [PATCH] efi_snp: improve compliance with the
>             EFI_SIMPLE_NETWORK_PROTOCOL spec
>     http://thread.gmane.org/gmane.network.ipxe.devel/3799
>     Date: 2015-Jan-27
>     feedback: none
> 
> (2) [PATCH] efi_snp: improve compliance with the
>             EFI_SIMPLE_NETWORK_PROTOCOL spec
>     http://thread.gmane.org/gmane.network.ipxe.devel/3955
>     Date: 2015-Feb-10
>     feedback: zero
> 
> (3) [PATCH] [efi] make load file protocol optional
>     http://thread.gmane.org/gmane.network.ipxe.devel/3815
>     Date: 2015-Feb-11
>     feedback: the patch destroys the user's ability to use
>               most features of iPXE
>     our point: we don't care about most features of iPXE, we just need
>                it for EFI drivers (Simple Network Protocol instances)
>     result: nothing

Mark this ^^^

> 
> (4) [RESENT PATCH 0/2] efi boot fixes.
>     http://thread.gmane.org/gmane.network.ipxe.devel/3854
>     Date: 2015-Mar-10
>     feedback: zilch
> 
> (5) [RESEND PATCH] efi_snp: improve compliance with the
>                    EFI_SIMPLE_NETWORK_PROTOCOL spec
>     http://thread.gmane.org/gmane.network.ipxe.devel/3934
>     Date: 2015-Apr-14
>     feedback: nada
> 
> (6) [PATCH] efi_snp: improve compliance with the
>             EFI_SIMPLE_NETWORK_PROTOCOL spec
>     http://thread.gmane.org/gmane.network.ipxe.devel/4096
>     Date: 2015-Jun-10
>     feedback: null

[snip]

> On the technical level, let me summarize: we needed three patches in
> total to get UEFI boot working:
> 
> #1 efi_snp: improve compliance with the EFI_SIMPLE_NETWORK_PROTOCOL spec
> #2 [efi] make load file protocol optional
> #3 [efi] Ensure drivers are disconnected when ExitBootServices() is
>    called
> 
> Wrt. #1, the maintainer expressed agreement on IRC, but never replied to
> patch emails.
> 
> Wrt. #2, the maintainer expressed strong disagreement (due to "user
> interface" concerns) on both IRC and later on the mailing list.
> Therefore the idea of upstreaming this patch is dead in the water. The
> maintainer would accept an alternative that would take a huge
> development effort, and would be useless for virtualization purposes
> (ie. PXE-booting with OVMF in QEMU).

So, the important detail that I forgot is that Gerd's patch, listed as
#2 here, and as (3) above in the list of earlier submissions, actually
does not change the behavior of ipxe *at all*. It just introduces a
config option, a knob if you like, that allows downstreams to rebuild
ipxe *that specific way* easily.

Therefore, absolutely no user interface concerns are valid in this case
-- those concerns may have been correct for my very first patch, but
Gerd reworked the patch to depend on a new config option (with unchanged
default behavior), and the maintainer refused to take even that.
(Despite the fact that upstream iPXE users would see no change in behavior.)

Laszlo

> 
> Wrt. #3, the maintainer decided to look at the patch, rewrite it, and
> commit it. For some unfathomable reason. Maybe because he was Cc'd
> directly on the patch email. I don't know. (The ipxe-devel list has
> absolutely minimal traffic, why wouldn't he read it without explicit Cc?!)
> 
> This PULL is about getting #3 via rebase, and #1 and #2 as downstream
> patches.
> 
> Thanks
> Laszlo
>
Stefan Hajnoczi July 22, 2015, 9:05 a.m. UTC | #15
On Wed, Jul 22, 2015 at 12:58:59AM +0200, Laszlo Ersek wrote:
> On 07/21/15 18:10, Stefan Hajnoczi wrote:
> > On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> On 21/07/2015 16:25, Peter Maydell wrote:
> > or work
> > with others to add upstream maintainers.
> 
> When we can't get the maintainer's attention for our patches, and when
> the maintainer tends to rewrite even those patches he more or less
> likes, how do you propose we convince him to give *push access* to
> random people?
> 
> > I see that Hannes Reinecke
> > also has patches on ipxe-devel that look ignored, so Gred and Laszlo
> > are not the only ones struggling to get patches upstream into ipxe.
> 
> I've said it several times (on other lists too), and I'll say it again:
> ipxe is not an "open process" community project at this point. The last
> half year, as Paolo indicated, and as I proved above, has been ample
> experience.

I understand the frustration with upstream.  Thanks for posting a
summary of stranded patch series, it helped explain that.

The reason I'm suggesting reaching out to Michael Brown is that the
downstream repo will only be an "open process" for us virtualization
developers.  It won't have a user community, support, or help improve
the situation for non-virtualization developers - all things which
matter for a healthy long-term open source project.

It may be simplest if Gerd maintains a QEMU downstream repository.  I'm
not against that.  But let's notify Michael Brown so he has a chance to
consider the problem.
Laszlo Ersek July 22, 2015, 10:05 a.m. UTC | #16
On 07/22/15 11:05, Stefan Hajnoczi wrote:
> On Wed, Jul 22, 2015 at 12:58:59AM +0200, Laszlo Ersek wrote:
>> On 07/21/15 18:10, Stefan Hajnoczi wrote:
>>> On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> On 21/07/2015 16:25, Peter Maydell wrote:
>>> or work
>>> with others to add upstream maintainers.
>>
>> When we can't get the maintainer's attention for our patches, and when
>> the maintainer tends to rewrite even those patches he more or less
>> likes, how do you propose we convince him to give *push access* to
>> random people?
>>
>>> I see that Hannes Reinecke
>>> also has patches on ipxe-devel that look ignored, so Gred and Laszlo
>>> are not the only ones struggling to get patches upstream into ipxe.
>>
>> I've said it several times (on other lists too), and I'll say it again:
>> ipxe is not an "open process" community project at this point. The last
>> half year, as Paolo indicated, and as I proved above, has been ample
>> experience.
> 
> I understand the frustration with upstream.  Thanks for posting a
> summary of stranded patch series, it helped explain that.
> 
> The reason I'm suggesting reaching out to Michael Brown is that the
> downstream repo will only be an "open process" for us virtualization
> developers.  It won't have a user community, support, or help improve
> the situation for non-virtualization developers - all things which
> matter for a healthy long-term open source project.

All the things upstream ipxe has been lacking for at least half a year
now, without much indication that it could improve.

> It may be simplest if Gerd maintains a QEMU downstream repository.  I'm
> not against that.  But let's notify Michael Brown so he has a chance to
> consider the problem.

If you can reach out to Michael Brown, that would be highly appreciated.
Personally I lost all hope.

Thanks
Laszlo
Stefan Hajnoczi July 22, 2015, 11:42 a.m. UTC | #17
On Wed, Jul 22, 2015 at 11:05 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/22/15 11:05, Stefan Hajnoczi wrote:
>> On Wed, Jul 22, 2015 at 12:58:59AM +0200, Laszlo Ersek wrote:
>>> On 07/21/15 18:10, Stefan Hajnoczi wrote:
>>>> On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> On 21/07/2015 16:25, Peter Maydell wrote:
>>>> or work
>>>> with others to add upstream maintainers.
>>>
>>> When we can't get the maintainer's attention for our patches, and when
>>> the maintainer tends to rewrite even those patches he more or less
>>> likes, how do you propose we convince him to give *push access* to
>>> random people?
>>>
>>>> I see that Hannes Reinecke
>>>> also has patches on ipxe-devel that look ignored, so Gred and Laszlo
>>>> are not the only ones struggling to get patches upstream into ipxe.
>>>
>>> I've said it several times (on other lists too), and I'll say it again:
>>> ipxe is not an "open process" community project at this point. The last
>>> half year, as Paolo indicated, and as I proved above, has been ample
>>> experience.
>>
>> I understand the frustration with upstream.  Thanks for posting a
>> summary of stranded patch series, it helped explain that.
>>
>> The reason I'm suggesting reaching out to Michael Brown is that the
>> downstream repo will only be an "open process" for us virtualization
>> developers.  It won't have a user community, support, or help improve
>> the situation for non-virtualization developers - all things which
>> matter for a healthy long-term open source project.
>
> All the things upstream ipxe has been lacking for at least half a year
> now, without much indication that it could improve.
>
>> It may be simplest if Gerd maintains a QEMU downstream repository.  I'm
>> not against that.  But let's notify Michael Brown so he has a chance to
>> consider the problem.
>
> If you can reach out to Michael Brown, that would be highly appreciated.
> Personally I lost all hope.

Done.

Stefan
Michael Brown July 22, 2015, 8:06 p.m. UTC | #18
On 21/07/15 23:58, Laszlo Ersek wrote:
>> Instead of propagating that fix
>> into QEMU, let's focus on ipxe upstream to solve the problem.
>
> How's this for focus:
>
> (1) [PATCH] efi_snp: improve compliance with the
>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>      http://thread.gmane.org/gmane.network.ipxe.devel/3799
>
> (2) [PATCH] efi_snp: improve compliance with the
>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>      http://thread.gmane.org/gmane.network.ipxe.devel/3955
>
> (3) [PATCH] [efi] make load file protocol optional
>      http://thread.gmane.org/gmane.network.ipxe.devel/3815
>
> (4) [RESENT PATCH 0/2] efi boot fixes.
>      http://thread.gmane.org/gmane.network.ipxe.devel/3854
>
> (5) [RESEND PATCH] efi_snp: improve compliance with the
>                     EFI_SIMPLE_NETWORK_PROTOCOL spec
>      http://thread.gmane.org/gmane.network.ipxe.devel/3934
>
> (6) [PATCH] efi_snp: improve compliance with the
>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>      http://thread.gmane.org/gmane.network.ipxe.devel/4096

Both (sic) of these patches now have upstream implementations:

   http://git.ipxe.org/ipxe.git/commitdiff/88a5f56
   http://git.ipxe.org/ipxe.git/commitdiff/a15c0d7

Unless I'm missing something, there are only two patches involved here.

I've also upstreamed a named configuration for qemu which incorporates 
your existing config changes (based on roms/config.ipxe.general.h) and 
adds the option to disable the EFI_LOAD_FILE_PROTOCOL installation:

   http://git.ipxe.org/ipxe.git/commitdiff/a200ad4

To build with this named configuration, just add "CONFIG=qemu" to the 
make command line when building iPXE.

Michael
Laszlo Ersek July 22, 2015, 8:08 p.m. UTC | #19
On 07/22/15 13:42, Stefan Hajnoczi wrote:
> On Wed, Jul 22, 2015 at 11:05 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 07/22/15 11:05, Stefan Hajnoczi wrote:
>>> On Wed, Jul 22, 2015 at 12:58:59AM +0200, Laszlo Ersek wrote:
>>>> On 07/21/15 18:10, Stefan Hajnoczi wrote:
>>>>> On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>> On 21/07/2015 16:25, Peter Maydell wrote:
>>>>> or work
>>>>> with others to add upstream maintainers.
>>>>
>>>> When we can't get the maintainer's attention for our patches, and when
>>>> the maintainer tends to rewrite even those patches he more or less
>>>> likes, how do you propose we convince him to give *push access* to
>>>> random people?
>>>>
>>>>> I see that Hannes Reinecke
>>>>> also has patches on ipxe-devel that look ignored, so Gred and Laszlo
>>>>> are not the only ones struggling to get patches upstream into ipxe.
>>>>
>>>> I've said it several times (on other lists too), and I'll say it again:
>>>> ipxe is not an "open process" community project at this point. The last
>>>> half year, as Paolo indicated, and as I proved above, has been ample
>>>> experience.
>>>
>>> I understand the frustration with upstream.  Thanks for posting a
>>> summary of stranded patch series, it helped explain that.
>>>
>>> The reason I'm suggesting reaching out to Michael Brown is that the
>>> downstream repo will only be an "open process" for us virtualization
>>> developers.  It won't have a user community, support, or help improve
>>> the situation for non-virtualization developers - all things which
>>> matter for a healthy long-term open source project.
>>
>> All the things upstream ipxe has been lacking for at least half a year
>> now, without much indication that it could improve.
>>
>>> It may be simplest if Gerd maintains a QEMU downstream repository.  I'm
>>> not against that.  But let's notify Michael Brown so he has a chance to
>>> consider the problem.
>>
>> If you can reach out to Michael Brown, that would be highly appreciated.
>> Personally I lost all hope.
> 
> Done.

Thanks. Looks like you got through. Obviously, that cannot be ascribed
to anything else than blind luck, or a personal relationship from the
past. Ie. things that should not matter in open source.

In any case, I asked Michael Brown what we should do differently next time.

Laszlo
Laszlo Ersek July 22, 2015, 8:10 p.m. UTC | #20
On 07/22/15 22:06, Michael Brown wrote:
> On 21/07/15 23:58, Laszlo Ersek wrote:
>>> Instead of propagating that fix
>>> into QEMU, let's focus on ipxe upstream to solve the problem.
>>
>> How's this for focus:
>>
>> (1) [PATCH] efi_snp: improve compliance with the
>>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>>      http://thread.gmane.org/gmane.network.ipxe.devel/3799
>>
>> (2) [PATCH] efi_snp: improve compliance with the
>>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>>      http://thread.gmane.org/gmane.network.ipxe.devel/3955
>>
>> (3) [PATCH] [efi] make load file protocol optional
>>      http://thread.gmane.org/gmane.network.ipxe.devel/3815
>>
>> (4) [RESENT PATCH 0/2] efi boot fixes.
>>      http://thread.gmane.org/gmane.network.ipxe.devel/3854
>>
>> (5) [RESEND PATCH] efi_snp: improve compliance with the
>>                     EFI_SIMPLE_NETWORK_PROTOCOL spec
>>      http://thread.gmane.org/gmane.network.ipxe.devel/3934
>>
>> (6) [PATCH] efi_snp: improve compliance with the
>>              EFI_SIMPLE_NETWORK_PROTOCOL spec
>>      http://thread.gmane.org/gmane.network.ipxe.devel/4096
> 
> Both (sic) of these patches now have upstream implementations:
> 
>   http://git.ipxe.org/ipxe.git/commitdiff/88a5f56
>   http://git.ipxe.org/ipxe.git/commitdiff/a15c0d7

Thank you.

> Unless I'm missing something, there are only two patches involved here.

No, you are right, it's about two patches.

> I've also upstreamed a named configuration for qemu which incorporates
> your existing config changes (based on roms/config.ipxe.general.h) and
> adds the option to disable the EFI_LOAD_FILE_PROTOCOL installation:
> 
>   http://git.ipxe.org/ipxe.git/commitdiff/a200ad4
> 
> To build with this named configuration, just add "CONFIG=qemu" to the
> make command line when building iPXE.

Much appreciated. Hopefully Gerd can cover this when he returns from his
vacation.

Laszlo
Stefan Hajnoczi July 23, 2015, 8:26 a.m. UTC | #21
On Wed, Jul 22, 2015 at 9:08 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/22/15 13:42, Stefan Hajnoczi wrote:
>> On Wed, Jul 22, 2015 at 11:05 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 07/22/15 11:05, Stefan Hajnoczi wrote:
>>>> On Wed, Jul 22, 2015 at 12:58:59AM +0200, Laszlo Ersek wrote:
>>>>> On 07/21/15 18:10, Stefan Hajnoczi wrote:
>>>>>> On Tue, Jul 21, 2015 at 3:28 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>>> On 21/07/2015 16:25, Peter Maydell wrote:
>>>>>> or work
>>>>>> with others to add upstream maintainers.
>>>>>
>>>>> When we can't get the maintainer's attention for our patches, and when
>>>>> the maintainer tends to rewrite even those patches he more or less
>>>>> likes, how do you propose we convince him to give *push access* to
>>>>> random people?
>>>>>
>>>>>> I see that Hannes Reinecke
>>>>>> also has patches on ipxe-devel that look ignored, so Gred and Laszlo
>>>>>> are not the only ones struggling to get patches upstream into ipxe.
>>>>>
>>>>> I've said it several times (on other lists too), and I'll say it again:
>>>>> ipxe is not an "open process" community project at this point. The last
>>>>> half year, as Paolo indicated, and as I proved above, has been ample
>>>>> experience.
>>>>
>>>> I understand the frustration with upstream.  Thanks for posting a
>>>> summary of stranded patch series, it helped explain that.
>>>>
>>>> The reason I'm suggesting reaching out to Michael Brown is that the
>>>> downstream repo will only be an "open process" for us virtualization
>>>> developers.  It won't have a user community, support, or help improve
>>>> the situation for non-virtualization developers - all things which
>>>> matter for a healthy long-term open source project.
>>>
>>> All the things upstream ipxe has been lacking for at least half a year
>>> now, without much indication that it could improve.
>>>
>>>> It may be simplest if Gerd maintains a QEMU downstream repository.  I'm
>>>> not against that.  But let's notify Michael Brown so he has a chance to
>>>> consider the problem.
>>>
>>> If you can reach out to Michael Brown, that would be highly appreciated.
>>> Personally I lost all hope.
>>
>> Done.
>
> Thanks. Looks like you got through. Obviously, that cannot be ascribed
> to anything else than blind luck, or a personal relationship from the
> past. Ie. things that should not matter in open source.

Maybe a bit of both.

I sent a link to the email thread via IRC.

Stefan