Re: [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order

[Michael S. Tsirkin]

On Fri, Aug 23, 2013 at 02:31:49PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
> 
> > On Fri, Aug 16, 2013 at 01:13:44PM +0200, address@hidden wrote:
> >> From: Markus Armbruster <address@hidden>
> >> 
> >> The first five patches are admittedly related to the stated purpose of
> >> this series pretty much only by "I can't stand perpetuating this
> >> stupid crap".  Max Filippov and Peter Maydell already cleaned up
> >> Xtensa and ARM the same way.
> >
> > I picked up patches 3,4 and 5 on my tree.
> > 1 and 2 were rebased by Eduardo, I'm taking them
> > from his patchset.
> > 6 needs to be rebased and comments addressed.
> 
> Applies fine with "git-am -3".  Pushed to
> http://repo.or.cz/w/qemu/armbru.git/shortlog/refs/heads/boot-order
> for your convenience.
> 
> We discussed the patch at some length, but it's not 100% clear to me
> what exactly you'd like me to address and how.  So let's recap briefly.
> 
> I think your main point was that PC machine type declarations are a bit
> repetitive.  They all share two lines:
> 
>     .max_cpus = 255,
>     DEFAULT_MACHINE_OPTIONS,
> 
> where DEFAULT_MACHINE_OPTIONS is defined as
> 
>     #define DEFAULT_MACHINE_OPTIONS \
>         .boot_order = "cad"
> 
> Many of them also share one of these lines:
> 
>     .desc = "Standard PC (Q35 + ICH9, 2009)",
>     .desc = "Standard PC (i440FX + PIIX, 1996)",
>     .desc = "Standard PC",
> 
> My patch touches only the shared DEFAULT_MACHINE_OPTIONS line.  It
> becomes
> 
>    .boot_order = "cad"
> 
> Commit message explains why:
> 
>     We set default boot order "cad" in every single machine definition
>     except "pseries" and "moxiesim", even though very few boards actually
>     care for boot order, and "cad" makes sense for even fewer.
> 
>     Machines that care:
>     
>     * pc and its variants
>     
>       Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>       'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> 
>     [...]
> 
>     Strip characters these machines ignore from their default boot order.
>     
>     For all other machines, remove the unused default boot order
>     alltogether.
> 
> The change is systematic: if the machine uses .boot_order, strip the
> characters it ignores from its initial value, else drop the initializer,
> so .boot_order remains null.
> 
> I don't want to squash further cleanup into this one, because it's hard
> enough to review as it is (and it already got competent review).  I
> could be persuaded to do further cleanup on top, but you need to tell me
> what cleanup you want.  Probably faster if you do it yourself :)

Responded in the relevant thread.
Hope this helps.

-- 
MST