Hi Glenn, On Mon, 2023-08-21 at 14:52 -0500, Glenn Washburn wrote: > On Sun, 20 Aug 2023 12:28:57 +0000 > Vitaly Kuzmichev <vitaly.kuzmic...@rtsoft.de> wrote: > > > Hello Daniel, Hello Glenn, > > Hi Vitaly, > > > > > I'm now finally completing my patch for search command to add > > support > > to search partition devices by PARTUUID, which I submitted in Apr > > 2021 [1]. > > I also integrated support to search by partition label submitted by > > Daniel > > Wagenknecht in Sep 2022 [2]. > > Awesome! I've had on my list to do this also, but haven't gotten to > it. > So thank you! > > > I (hopefully) addressed all the review points raised for both > > patches. > > I split all the changes into a set of 16 patches to ease your > > review. > > 16 patches sounds like a lot. Does it make sense to break it into so > many? I have found that larger patch sets tend to move more slowly, > or > not at all on this list. I'd suggest you break it up to at least two > patch series, one for PARTUUID and one for PARTLABEL. >
Ok, I can merge some patches together. Most of them are just adding some helper functions to be used in a later patch. I think it is ok to fold them into one. I do not like to split into two patch series because I now have single patch for all documentation update. If I split it, the changes would intersect and may cause some conflicts if you change the order of applying them. Also I have minor update for 4 filesystem modules to replace some common code with a helper function introduced in one of the patches. I will fold them too and use "fs:" prefix for subject, if it is ok for you. This should reduce patch series to 7 or so. > > My question is whould you like to review them now and include in > > upcoming > > Grub 2.12 release, or should I submit the patchset after release? > > I would have liked this functionality to be in the upcoming release, > especially considering that it will likely be a couple years until > the > next release. I think there is likely less risk in these changes > because they add compartmentalized new features that should not > effect > current features. So if its broken, then GRUB won't lose any features > it didn't already have. On the other hand if its broken in an > exploitable way, that would be worse than not including it. I'd > suggest > sending the patches now and I'll take a look at them. I suspect the > chances of Daniel wanting to include them are not great, but in that > case it can be picked backup after the release with the series > already > in progress. Ok I will send the patches within a day. > > > There were some other few attempts to add this functionality to > > Grub from > > different people since 2021, I think this is worth to be added in > > 2.12. > > I think the point here, if I understand you correctly, is that since > this has been submitted before the feature freeze, that it might be > eligible as part of the exception to the feature freeze. I think it > does add a little weight, but may be not enough and really depends on > the nature of the changes (how risky are they?). I tested the changes in a virtual machine and they work ok. PARTUUID related code is running on our x86_64 embedded devices for years already. I saw in the mailing list a problem with GUID unaligned access on Ia64. Perhaps this could affect my code too, as I do read of 'struct grub_guid_t' members from on-stack 'struct grub_gpt_partentry'. (Should I use grub_get_unaligned16/-32 in this case?) Apart from this the risk of including these patches is low. You can drop the changes related to FS modules if you find them too risky. Thanks. Best Regards, Vitaly. > > Glenn > > > > > [1] > > https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00055.html > > [2] > > https://lists.gnu.org/archive/html/grub-devel/2022-09/msg00064.html > > > > With Best Regards, > > Vitaly. > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
