[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Corey Minyard
Subject: Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list
Date: Tue, 15 Mar 2016 11:36:39 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 03/15/2016 07:45 AM, Michael S. Tsirkin wrote:
On Tue, Mar 15, 2016 at 07:38:43AM -0500, Corey Minyard wrote:
On 03/15/2016 04:37 AM, Michael S. Tsirkin wrote:
On Tue, Mar 15, 2016 at 09:45:22AM +0100, Gerd Hoffmann wrote:
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.
Not a new fw_cfg file.

It's existing smbios file which gets new records added by a new device.
So when initializing it early (old order) it doesn't (yet) contain the
new records.  When initializing it late it has them, but also has a
different place in the fw_cfg directory.

So old machine types initialize smbios early (for compatibility).
I see. So in this model, we'd have to somehow keep track of
the old initialization order forever, and
add hacks whenever we change it.
IMHO That would just be too hard to maintain. I have an alternative

New machine types initialize smbios late (so guests see the new
So here is what I propose instead:

- always initialize it late
- sort late, a machine done, not when inserting entries
- figure out what the order of existing entries is currently,
   and fill an array listing them in this order.
   for old machine types, insert the existing entries
   in this specific order by using a sorting function:

qsort(....., fw_cfg_cmp);

I've hit a bit of a snag here.  For 0.11 and before, PCI option ROMs
were loaded via fw_cfg, not in the PCI ROM BAR.  This causes two

* The order depends on the device initialization order,
which I'm not sure is quantifiable.  I believe it depends on
how they are listed on the command line.

* Users can load their own romfile with their own name, which
means it can't be in the list.

Also, for the ISA VGA ROMs, their order will also depend on the
device list order.

Outside of that, I have an order of file names.

I think if I treat the device ROMs separately and handle them
in init order, and then stick that device list in the proper location,
that will work.  Does that sound reasonable?




fw_cfg_find(a) {
     for (index = 0; index < fw_cfg_legacy_array_size; ++index)
         if (!strcmp(a, ...))
     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);
Last night I had an idea something like this.  Sorting by filename
may not work because the user may pass in the file from the
command line and you wouldn't be able to track the file name that
command line files must all have a consistent prefix,
so we can skip sorting them.
I'll need to look at the code - don't they already?
If not we IMHO absolutely must fix that before release
and give them consistent prefixes.

Instead, you could add a "legacy_order" parameter to the fw_cfg_add
functions.  Then figure out the current order add the numeric
order to each call.  Then sort by the numeric order.  As long as you
don't reorder things with the same numeric value I think that
would work and be fairly simple to implement.  New calls could
pass in NO_FW_CFG_LEGACY_ORDER or something like that and
be pasted onto the end in legacy mode.

OK but it's a much larger change and less well contained.

While mucking with the file ordering anyway:  Good opportunity to make
new machine types also sort the fw_cfg directory entries, so they get a
fixed order independent from the order they are created, and we will not
face this problem again.

What exactly do you mean by directory entries here?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]