Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Mar 15, 2016 at 08:34:09AM +0100, Gerd Hoffmann wrote:
> On Di, 2016-03-15 at 09:17 +0200, Michael S. Tsirkin wrote:
> > On Tue, Mar 15, 2016 at 08:04:48AM +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > > Don't add a new machine type in this version, just use the 2.6 one.
> > > > 
> > > > Unfortunately this patch won't help any, as your next patch reorders fw
> > > > cfg files which will affect the old machine types.
> > > 
> > > We can make that depend on dont_sort_fw_cfgs too, to keep things as-is
> > > on old machine types.
> > 
> > Yes, except the name is a bit off then. It's more "legacy_fw_cfg_order"
> > than "dont_sort_fw_cfgs".
> > 
> > > Which probably implies only new machine types
> > > will see the new smbios entries.
> > > 
> > > cheers,
> > >   Gerd
> > 
> > Not really I think.
> > If there's a new device triggering a new smbios entry, we don't care how
> > it's sorted since by then there's no question of cross-version migration
> > with old QEMU.
> 
> As I understand things the smbios fw_cfg file must be created later to
> make new entries (created by devices) are actually visible.  So we can
> make that initialization order change depend on dont_sort_fw_cfgs (or
> legacy_fw_cfg_order) too.  That way we maintain the ordering for old
> machine types. 

Yes.

> But old machine types then will also not see the entries
> added by devices.  Which isn't a problem I think, old qemu versions
> don't have support for that device in the first place ...

Depends on how you code it up.  We have a list, we look each file
there and sort accordingly.  Fine.
New devices will not be on this list, I guess you can just ignore them
and guests will not see them. OK but I think it is better to make old
machine types see them.  It's trivially easy and more user-friendly.
Simply sort anything that is not on the list alphabetically and
add it at the end.

> Corey, can you please send all this in a patch series?  This should make
> those dependencies more clear ...
> 
> thanks,
>   Gerd
> 
> > 

So like this:

fw_cfg_find(a) {
    for (index = 0; index < fw_cfg_legacy_array_size; ++index)
        if (!strcmp(a, ...))
            break;
    return index;
}

fw_cfg_cmp(a, b) {
    in cmp;
    if (legacy_fw_cfg_order) {
        int list1 = find(a);
        int list2 = find(b);

        if (list1 < list2)
            return -1;
        if (list1 > list2)
            return 1;
    }

    return strcmp(a, b);
}


This way anything not on list is sorted,
with !legacy_fw_cfg_order list is assumed empty.

-- 
MST