Code review comment for ~paelzer/ubuntu/+source/qemu:ubuntu-eoan-4.0

Yes reviewing that was exactly what I needed to clean up all the little mistakes that creeped in over the last years.

#1 yes - done

#2 the helper script is still installed by this, but the rest is doen via debhelper automatically - done

#3 that is not a commented line, it really is a comment what the next line will do. Never the less I adapted the cahngelog - done

#4 yes the infra is not upstream (dropped) and the type is all that is left is merged - done
   Also fixed a double mentionign of bug nummbers.

#5 no need to mention details here - I think it is fine as-is

#6 was an old ref, since I want to not update it all the time.
   Since we switch to git ubuntu lets use these and no more change it ever again

#7 in fact s390x = 64bit and s390 = 31 bit, so s390x feels correct
   But then boot code is 31bit (or even 24) and the file name is therefore without x
   fixed changelog

#8 yes it needs to be mentioned, while we rely on systemd/udev a long time (ack) the code was
   still there all the time
   On the next merge it will be in Debian and it will one last time be mentioned as "Dropped"

#9 I had those patches split and maintaining it split was far more error prone (search replaces hit only a few of them) and caused extra work (changing one made the other misapply). So I made them one patch intentionally and plan to keep it that way.
This is a lesson I had to learn the hard way, this particular Delta really is better a single patch :-/

The added changes that you were missing are all mentioned under "Added Changes".
Quote:
"
 100 - added eoan types
 101 - fixed s390x issue of upstream types having a "v" prefix
 102 - add back dropped machine types to avoid more issues like LP: 1802944
 103 - fix kvm split irqchip default in ubuntu q35 machine type
 104 - drop no more needed spapr_machine_2_11_sxxm_instance_options and
 105 adapt updated CamelCase
 106 - -hpb types now need to use GlobalProperties
 107 - pc_compat_2_0 got a _fn suffix and slight changes
"

#10 - yeah the first is the change that remains (slof plus all the reasons and details) and in the "Added changes" section it is only referred to as "updated to 4.0".
Like we usually mention there "updated patch foo for context change"

For your qeustions - about SLOF.
Slof itself is an extra source package "slof" and maintained there.
For a IBM feature request (and since Debian can#t agree where to maintain the s390x bits) we need to add some (only some) of the code back (which is updated every time qemu changes). Out of that we build the s390x roms until Debian finally adds them.

We have regresson tests for slof itself (the package) and for the ccw image (used on normal guests). The only one missing is the s390x netboot which is a known todo at https://trello.com/c/7clwfTYC

#11 again some changes are mentioned as kept (for the function) and as added (for the patch update that was required to match the new version)

#12 done

#13 no really i386 build was broken - the unistd change broke it when building qemu-system-mips on i386 (it doesn't only break mips itself). Hence the statement is correct.

Thank you so much for this review.
We catched plenty of little itches which makes me feel much better!
Going forward we will review every added change for each other which will help to not need to clean them up later.