On Wed, 27.08.14 22:48, Thomas Bächler (tho...@archlinux.org) wrote: > Am 27.08.2014 um 20:25 schrieb Ivan Shapovalov: > > On Wednesday 27 August 2014 at 20:19:45, Lennart Poettering wrote: > >> On Wed, 27.08.14 20:26, Ivan Shapovalov (intelfx...@gmail.com) wrote: > >> > >>> This is as proposed by Thomas in review of my hibernate-resume patchset. > >>> > >>> The objective benefit of this change is that in_initrd() function is used > >>> for checking, which not only checks for /etc/initrd-release, but also > >>> verifies > >>> that the rootfs is on a virtual device. > >> > >> If we add a new condition then I want to hear a strong case for it. I > >> mean, what's wrong with checking for initrd-release? Why would that not > >> suffice? > > Whether or not we are in initrd is what we want to check. The existence > of /etc/initrd-release is an implementation detail. Having the same file > check as a condition in units duplicates the code that has been > implemented in the in_initrd() function.
Well, in_initrd() previously did only the exacty same check, as the condition... We added the tmpfs/ramfs check only as extra safety net, because dracut once upon a time ended up copying the initrd-release file into the host os and disaster ensued... Note that the ConditionXYZ stuff in most cases is just supposed to be optimization stuff, which allows us to skip execution of things if there's no point to bother... I think you do have a point though, and it would be good to use the full in_initrd() check here, but maybe the lesson to take here is to simply do that inside of the resume tool, rather than add a new condition for it... That way, the condition is just an optimization, but the safety net is inside the tool itself... I have now commited a patch that adds this. (the generator actually already had it...) > In addition, someone who writes unit files should not be forced to know > these implementation details, but should have the proper condition > documented for the purpose. No, /etc/initrd-release is really supposed to be API (we probably should put together a man page for it, to make that clear). It's something an initrd implementor has to write, and then systemd can make use of it, but where systemd might just be one consumer of... Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
