Backport "populate PCI DT in reverse order" patch

Jon,

A QEMU backport request. Please have the Server team evaluate.

Thanks.

                     Michael

On 03/06/2017 12:19 PM, Launchpad Bug Tracker wrote:
> bugproxy (bugproxy) has assigned this bug to you for Ubuntu:
>
> == Comment: #0 - MIKHAIL S. MEDVEDEV <email address hidden> - 2017-03-03 14:21:43 ==
> ---Problem Description---
> On ppc64el qemu the order of device scanning was recently changed, causing device names to be in reverse order compared to e.g. x86. This is creating a need for all sorts of work arounds to make the code that assumes certain order of devices to work on Power.
>
> The fix has been proposed and it appears it would go into Qemu 2.9, see [1]. It might be awhile until qemu 2.9 gets widely adopted as default. It would be beneficial if the fix can be backported to an earlier versions of Qemu.
>
> [1] http://patchwork.ozlabs.org/patch/731061/
>
> ---uname output---
> Linux devstack-xenial-ppc64-84615 4.8.0-39-generic #42~16.04.1-Ubuntu SMP Mon Feb 20 15:09:38 UTC 2017 ppc64le ppc64le ppc64le GNU/Linux
>
> Machine Type = 8247-21L
>
> ---Debugger---
> A debugger is not configured
>
> ---Steps to Reproduce---
>
>
> Contact Information = Mikhail Medvedev <email address hidden> / Tiago Rodrigues de Mello <email address hidden>
>
> ** Affects: ubuntu
> Importance: Undecided
> Assignee: Taco Screen team (taco-screen-team)
> Status: New
>
>
> ** Tags: architecture-ppc64le bugnameltc-152251 severity-medium targetmilestone-inin16042

--
Michael Hohnbaum
OIL Program Manager
Power (ppc64el) Development Project Manager
Canonical, Ltd.

Yeah, already saw that go through. That being said, we'd probably reject
unless accepted upstream.

On Mon, Mar 6, 2017 at 2:31 PM, Michael Hohnbaum <
<email address hidden>> wrote:

> Jon,
>
> A QEMU backport request. Please have the Server team evaluate.
>
> Thanks.
>
> Michael
>
>
> On 03/06/2017 12:19 PM, Launchpad Bug Tracker wrote:
>
>> bugproxy (bugproxy) has assigned this bug to you for Ubuntu:
>>
>> == Comment: #0 - MIKHAIL S. MEDVEDEV <email address hidden> - 2017-03-03
>> 14:21:43 ==
>> ---Problem Description---
>> On ppc64el qemu the order of device scanning was recently changed,
>> causing device names to be in reverse order compared to e.g. x86. This is
>> creating a need for all sorts of work arounds to make the code that assumes
>> certain order of devices to work on Power.
>>
>> The fix has been proposed and it appears it would go into Qemu 2.9, see
>> [1]. It might be awhile until qemu 2.9 gets widely adopted as default. It
>> would be beneficial if the fix can be backported to an earlier versions of
>> Qemu.
>> [1] http://patchwork.ozlabs.org/patch/731061/
>>
>> ---uname output---
>> Linux devstack-xenial-ppc64-84615 4.8.0-39-generic #42~16.04.1-Ubuntu SMP
>> Mon Feb 20 15:09:38 UTC 2017 ppc64le ppc64le ppc64le GNU/Linux
>> Machine Type = 8247-21L
>> ---Debugger---
>> A debugger is not configured
>> ---Steps to Reproduce---
>>
>> Contact Information = Mikhail Medvedev <email address hidden> / Tiago
>> Rodrigues de Mello <email address hidden>
>>
>> ** Affects: ubuntu
>> Importance: Undecided
>> Assignee: Taco Screen team (taco-screen-team)
>> Status: New
>>
>>
>> ** Tags: architecture-ppc64le bugnameltc-152251 severity-medium
>> targetmilestone-inin16042
>>
>
> --
> Michael Hohnbaum
> OIL Program Manager
> Power (ppc64el) Development Project Manager
> Canonical, Ltd.
>
>

--
Jon Grimm
Engineering Manager, Ubuntu Server
Canonical Ltd

------- Comment From <email address hidden> 2017-03-06 16:55 EDT-------
This is already upstream: a8eeafda1930f49e2080e66b7a5ef493564838d1 .

Hi,
thank you for your report - and being upstream queued for qemu 2.9 already I think we can take that as backport into zesty without too much discussion.

But this "argument" is already a thin line and it will get thinner when it would come to SRUs.
By the Argument I mean "This behaviour change is not actually a bug since no assumptions should be
made on DT ordering. [...] It is expected to make things easier when porting existing applications to power.".

I read through the thread and I totally understand the need why this got "reverted" now.
But trivializing this - either it is:
- not an important behavior change (then this fix would not exist)
- an important behavior change (then it is not really SRUable)

The latter means that everybody on Xenial/Yakkety where the current reversed order is released have already adapted their setups, guests, device names, ...
If they went with device labels and such due to the initial error they won't see a change, but e.g. naming of network devices might change if they are not running with persistent device naming (Note Ubuntu does so for net it would be e.g. enp0s1 no matter what, but I think e.g. disk device names would change order).

That said SRUing this would break all those then right?

TL;DR: "either it is not important to SRU or it is, but then it would be a non SRUable behavior change".
Open for discussion, but these are the thoughts going through my mind at the moment.

For Zesty I think we are fine, the next Qemu release would change it just the same.
But please discuss with me what and why we should exactly do with Xenial/Yakkety.

A zesty fix is currently building for testing and verification in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/2543
It would be great if you could confirm that achieves what you want with a test from your side.

Build complete testing started

------- Comment From <email address hidden> 2017-03-07 14:44 EDT-------
Thanks Christian, I agree that stability is important on Xenial/Yaketty.

Here is my perspective as a user. We are running an OpenStack CI that uses Cirros image (no systemd/udev and consistent device naming). We did spend time to debug the problem caused by the reverse order and to come up with a fix. If the qemu behavior remains, more people would need to put effort into writing their own fixes as long as Xenial/Yakkety is not EOL. And every workaround would need to be supported by them.

Either way, you either break current workarounds, or force more workarounds down the line. As a user, I would like to have less workarounds. And I believe that the change in device order is a bug, even though it is advised not to rely on the device order. I understand that this might not be a strong case for backporting on Ubuntu, but more of a case for having a backport in Qemu itself, which is not likely to happen.

I am unable to confirm the Qemu build works on Zesty for original problem we have, it would take time to create an OpenStack CI Zesty environment from scratch. I might try to install it on Xenial, but I suspect it would not like that. I'd need to compile it myself to test it.

Thanks mmedvede for your thoughts,
please get me right I totally see your "users perspective".
If Xenial would be out for a week that might still be fine, but being out so long breaking those users who spent effort to work around "again" is the worst case IMHO.

In some sense my Opinion doesn't make a decision - If you really want it to be SRUed back please follow [1] updating the bug and get an ack by the SRU Team.

OTOH for Zesty all my Tests passed and as I outlined the reverse change would come to users anyway in qemu >=2.9.
I'll push that to zesty presumably nothing else fails on the way into zesty it should auto-update this bug somewhen soon.

[1]: https://wiki.ubuntu.com/StableReleaseUpdates

This bug was fixed in the package qemu - 1:2.8+dfsg-3ubuntu2

---------------
qemu (1:2.8+dfsg-3ubuntu2) zesty; urgency=medium

  * d/p/ubuntu/spapr-pci-populate-PCI-DT-in-reverse-order.patch: backport
    "spapr/pci: populate PCI DT in reverse order" (LP: #1670481).

 -- Christian Ehrhardt <email address hidden> Tue, 07 Mar 2017 09:23:08 +0100

Hi,
I'm checking bugs dormant too long to update their tracking.
On this one I highly appreciate the report and the fixing we did in zesty (and later is done due to it being upstream) - it is ok to change that on a new release which zesty was.

But as outlined in comment #4 SRU'ing it would be as much chance for a regression to some users as it would fix others.

Those affected (Like the reporter) already likely have workarounds in place, but others could be impacted by the reverse order.

I asked to make it a more compelling case to drive the SRU in comment #7, but nothing happened.
Thereby "timing out" the Xenial task and setting incomplete->invalid to clear the view for the issues currently actionable.

If that still is a concern please provide the arguments needed for the SRU [1] and set it back to new or confirmed.

[1]: https://wiki.ubuntu.com/StableReleaseUpdates

P.S. Yakkety is EOL, so I set won't fix for that one