From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46658) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afsMC-00085k-Ps for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:02:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afsM7-0002tI-Or for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:02:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34539) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afsM7-0002t0-I6 for qemu-devel@nongnu.org; Tue, 15 Mar 2016 13:02:03 -0400 Date: Tue, 15 Mar 2016 19:01:59 +0200 From: "Michael S. Tsirkin" Message-ID: <20160315185844-mutt-send-email-mst@redhat.com> References: <20160315085654-mutt-send-email-mst@redhat.com> <1458025488.13231.20.camel@redhat.com> <20160315091411-mutt-send-email-mst@redhat.com> <1458027249.13231.27.camel@redhat.com> <20160315093529-mutt-send-email-mst@redhat.com> <1458031522.13231.39.camel@redhat.com> <20160315113016-mutt-send-email-mst@redhat.com> <56E80253.3080605@acm.org> <20160315144324-mutt-send-email-mst@redhat.com> <56E83A17.1010906@mvista.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E83A17.1010906@mvista.com> Subject: Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Minyard Cc: Paolo Bonzini , Gerd Hoffmann , Corey Minyard , qemu-devel@nongnu.org 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? > >>>