On 12-07-18 06:24 PM, Michael Biebl wrote: > Hi, Hi Michael,
> I need to be honest and say that I don't like the approach to do this > via a sysvinit or upstart script which does the copy on each and every > boot, even when > a/ the kernel doesn't actually change during runtime because it isn't > upddated > b/ the user doesn't actually use hibernation OK. That's a fair assessment. TBH, I was not entirely happy with the approach but wanted to put something out there to try to solicit comment and more importantly, ideas. > Also, running grub-mkconfig on every boot is is a very costly operation > which can take several seconds. Right. Agreed. And now that I have had some time to read your entire critique and reflect, probably that belongs in the actual resuming-from-hibernate code path. > Running the grub update on *every* hibernation is wrong, too. It can probably gated with some logic to see if the grub config is newer than stashed kernel to avoid it every time. I will look into that. > What happens to those grub entries after a reboot? It seems to me they > are left there as stale hibernate entries. No. They are cleaned out by the "resume|thaw" code path in /etc/pm/sleep.d/20_update-grub. > Stale entries are cleared > during boot, No, during thaw (i.e. return from hibernation). > so we will have those entries at least for one boot cycle, > right? No, per above. > I also don't see any error handling anywhere, in case /boot was not > writable/big enough to create the stashed kernel. This can happen if you > have a separate /boot partition which is small or mounted ro. Yes. This was just proof-of-concept to solicit critique on the approach. Of course a final implementation would have to have error handling. > 08_linux_thaw looks like it could be painful to maintain, it looks > rather complex and contains stuff like kexec. Is everything there really > necessary? Is that a copy of /etc/grub.d/10_linux which has been > extended by you? Yes, it is a copy of /etc/grub.d/10_linux modified to handle the "Resume from Hibernation" grub entry. I debated simply adding that handling to the 10_linux hook or maintaining the spirit of grub.d's modularity. Obviously I chose the latter. I'm certainly not against the former, it just means more red-tape in the form of trying to get a modification into grub as well as pm-utils, co-ordinated. TBH, I'm not entirely optimistic that grub will take such a modification since it has reliance on the bits also being in pm-utils, tying the dependency of the two packages more tightly together. > It has stuff like /etc/linuxmint/info in there which make me curious > where it comes from as it generates warnings on my Debian system. > I'm not a grub expert, so I'm a bit wary about having to maintain such a > file. Yeah, that is just an artifact of the fact that my system is a Linux Mint system. Of course, if the work were applied to Debian, such a thing would not be in that file. Additionally, the duplication of "linux_entry()" in that file begs for some abstraction and/or sharing of that function amongst hooks. But again, my submission was just a PoC for comment. > To sum up: I'm not happy about the general approach to do this kernel > stashing during boot and running grub update at boot and before > hibernation and the implementation has its flaws, too. Well, I can agree with getting that stuff out of the boot path given your idea below, but if it's not in the boot path, it pretty much has to be in the hibernate path. Not having it in both the boot path and not in the hibernate path is pretty much like wanting your cake and eating it too. > You might look into the /etc/kernel/ hooks, which are run whenever a > kernel is upgraded. Maybe it allows to do the kernel stashing in a much > cleaner way. Yes, this is a good idea. I tend to forget that there is a hook system called during kernel upgrades and indeed, it would be a better place to stash the running kernel. > I would also very much prefer, if the grub specific hook is moved into > grub proper. grub already contains the hooks for xen kernels or the > recovery mode. Yes. The recovery mode is handled by 10_linux for the kernels that are handled by 10_linux but xen kernels are handled by their own hook in 20_linux_xen, just like my own 08_linux_thaw, and is very much a duplication+modification of 10_linux, again just like my own 08_linux_thaw. IOW, if xen kernel handling is the model to follow, then my 08_linux_thaw very much follows the model. Let me take a moment now to thank you very much for the critique. It was exactly what I was looking for. Thank-you. With any luck I will find the time to try to rework my implementation. Cheers, b.
signature.asc
Description: OpenPGP digital signature
