[systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.

[Ivan Shapovalov]

On Wednesday 27 August 2014 at 08:18:38, Thomas Bächler wrote:	
> Am 26.08.2014 um 22:17 schrieb Ivan Shapovalov:
> > This can be used to initiate a resume from hibernation by path to a swap
> > device containing the hibernation image.
> > 
> > The respective templated unit is also added. It is instantiated using
> > path to the desired resume device.
> 
> Really great stuff, this was really missing from systemd initrd. I only
> saw this because of your posting to the arch-projects list, so I am late
> to the party. Anyway, although this is commited to systemd.git, there's
> no reason it can't still be improved.
> 
> > diff --git a/units/systemd-hibernate-resume at .service.in b/units/systemd-hibernate-resume at .service.in
> > new file mode 100644
> > index 0000000..6db584d
> > --- /dev/null
> > +++ b/units/systemd-hibernate-resume at .service.in
> > @@ -0,0 +1,20 @@
> > +#  This file is part of systemd.
> > +#
> > +#  systemd is free software; you can redistribute it and/or modify it
> > +#  under the terms of the GNU Lesser General Public License as published by
> > +#  the Free Software Foundation; either version 2.1 of the License, or
> > +#  (at your option) any later version.
> > +
> > +[Unit]
> > +Description=Resume from hibernation using device %f
> > +Documentation=man:systemd-hibernate-resume at .service(8)
> > +DefaultDependencies=no
> > +BindsTo=%i.device
> 
> What's the purpose of BindsTo= as opposed to Requires= here. They are
> both the same for a oneshot service, but the former is more confusing.

This is just because systemd-fsck at .service does the same. Seems like it's
the "established usage", as Andrei says.

> 
> > +Wants=local-fs-pre.target
> > +After=%i.device
> > +Before=local-fs-pre.target systemd-remount-fs.service systemd-fsck-root.service
> 
> The part of ordering this Before=local-fs-pre.target is so crucial, it
> can't be stressed enough. If _anything_ writes to _any_ file system
> before this service runs, your system is broken and your data is lost.
> That said, are you sure that all services are properly ordered against
> the target?

I've spent quite some time verifying this. The only thing not covered
is usr.mount (not sysroot-usr.mount), but Lennart says any configuration
with initramfs's /usr split off is broken.

(Yes, I assume that lvm2, mdadm/mdmon, dm-event and so on don't write
to filesystems. If I'm wrong -- this needs to be fixed...)

> 
> What's the purpose of ordering this against systemd-fsck-root.service?
> This service is not run in initrd ever, because it checks
> 'ConditionPathIsReadWrite=!/', which always fails in initrd.

Just a leftover, indeed. These services do not exist in initramfs.
They probably should be removed in a separate commit.

> 
> > +ConditionPathExists=/etc/initrd-release
> 
> We should have and use ConditionInitrd=. I am surprised that this
> doesn't exist, but it really should.

Would you accept a patch adding that (using in_initrd()) and converting
all uses of ConditionPathExists=/etc/initrd-release to this new
condition statement?

Thanks for review!

-- 
Ivan Shapovalov / intelfx /
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20140827/25db3817/attachment.sig>