[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list |
Date: |
Tue, 15 Mar 2016 19:01:59 +0200 |
On Tue, Mar 15, 2016 at 11:36:39AM -0500, Corey Minyard wrote:
> 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
> >>>proposal.
> >>>
> >>>
> >>>
> >>>>New machine types initialize smbios late (so guests see the new
> >>>>records).
> >>>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
> issues:
>
> * 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?
>
> Thanks,
>
> -corey
I think so. By the time this becomes an issue we might decide to
disable migration from 0.11
> >>>where:
> >>>
> >>>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);
> >>>}
> >>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
> >>way.
> >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.
> >>
> >>-corey
> >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.
> >>>>
> >>>>cheers,
> >>>> Gerd
> >>>What exactly do you mean by directory entries here?
> >>>
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, (continued)
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Gerd Hoffmann, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Gerd Hoffmann, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Corey Minyard, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Corey Minyard, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Corey Minyard, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Corey Minyard, 2016/03/16
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Paolo Bonzini, 2016/03/16
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/16
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Gerd Hoffmann, 2016/03/15
- Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list, Michael S. Tsirkin, 2016/03/15