Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
On Wed, 27.08.14 10:59, Andrei Borzenkov (arvidj...@gmail.com) wrote: > On Wed, Aug 27, 2014 at 10:18 AM, Thomas Bächler wrote: > >> +[Unit] > >> +Description=Resume from hibernation using device %f > >> +Documentation=man:systemd-hibernate-resume@.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. > > > > Semantic of Requires is simply wrong for device - you cannot "start" > device, you can only passively wait for it. Requisite is more > appropriate to express "cannot start until device is available". But I > think, BindsTo is established usage for devices and is quite clear > here. Well, "starting" a device means waitinf for it. Requires and Requisite really have the same effect here... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
On Wed, 27.08.14 08:18, Thomas Bächler (tho...@archlinux.org) wrote: > > +[Unit] > > +Description=Resume from hibernation using device %f > > +Documentation=man:systemd-hibernate-resume@.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. Yeah, I figure Requires= might be slightly more appropriate for this, given that it fails on its own if the device ends up not being available anymore... > 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. I think for most purposes we should consider the initrd mostly read-only, hence I wouldn't rely on this check, even though it might effectively make the dep unnecessary... > > +ConditionPathExists=/etc/initrd-release > > We should have and use ConditionInitrd=. I am surprised that this > doesn't exist, but it really should. Why? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
Am 27.08.2014 um 09:22 schrieb Ivan Shapovalov: >>> +[Unit] >>> +Description=Resume from hibernation using device %f >>> +Documentation=man:systemd-hibernate-resume@.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@.service does the same. Seems like it's > the "established usage", as Andrei says. BindsTo= Configures requirement dependencies, very similar in style to Requires=, however in addition to this behavior, it also declares that this unit is stopped when any of the units listed suddenly disappears. "Stopping" a oneshot unit makes no sense, that's why I find BindsTo confusing. If systemd-fsck@.service does the same, then we should do the same thing here. >> 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. I've never heard of such a configuration. > (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...) They really shouldn't. And they may be required for resuming (you can resume from swap on lvm on an encrypted container, which is a rather common setup). >>> +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? I am not the one to accept patches here, but I'd love to see this implemented. signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
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-res...@.service.in > > b/units/systemd-hibernate-res...@.service.in > > new file mode 100644 > > index 000..6db584d > > --- /dev/null > > +++ b/units/systemd-hibernate-res...@.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@.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@.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 / signature.asc Description: This is a digitally signed message part. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
On Wed, Aug 27, 2014 at 10:18 AM, Thomas Bächler wrote: >> +[Unit] >> +Description=Resume from hibernation using device %f >> +Documentation=man:systemd-hibernate-resume@.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. > Semantic of Requires is simply wrong for device - you cannot "start" device, you can only passively wait for it. Requisite is more appropriate to express "cannot start until device is available". But I think, BindsTo is established usage for devices and is quite clear here. >> +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? > > What's the purpose of ordering this against systemd-fsck-root.service? I suppose it is leftover from the first version which was intended to support non-initrd case as well. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
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-res...@.service.in > b/units/systemd-hibernate-res...@.service.in > new file mode 100644 > index 000..6db584d > --- /dev/null > +++ b/units/systemd-hibernate-res...@.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@.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. > +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? 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. > +ConditionPathExists=/etc/initrd-release We should have and use ConditionInitrd=. I am surprised that this doesn't exist, but it really should. signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCHv6 2/3] hibernate-resume: add a tool to write a device node's major:minor to /sys/power/resume.
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. --- .gitignore | 1 + Makefile-man.am| 7 +++ Makefile.am| 17 +-- man/systemd-hibernate-res...@.service.xml | 81 ++ src/hibernate-resume/Makefile | 1 + src/hibernate-resume/hibernate-resume.c| 81 ++ units/.gitignore | 1 + units/systemd-hibernate-res...@.service.in | 20 8 files changed, 206 insertions(+), 3 deletions(-) create mode 100644 man/systemd-hibernate-res...@.service.xml create mode 12 src/hibernate-resume/Makefile create mode 100644 src/hibernate-resume/hibernate-resume.c create mode 100644 units/systemd-hibernate-res...@.service.in diff --git a/.gitignore b/.gitignore index 8189da7..0b5608c 100644 --- a/.gitignore +++ b/.gitignore @@ -75,6 +75,7 @@ /systemd-getty-generator /systemd-gnome-ask-password-agent /systemd-gpt-auto-generator +/systemd-hibernate-resume /systemd-hostnamed /systemd-inhibit /systemd-initctl diff --git a/Makefile-man.am b/Makefile-man.am index 562ecba..09a1038 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -70,6 +70,7 @@ MANPAGES += \ man/systemd-getty-generator.8 \ man/systemd-gpt-auto-generator.8 \ man/systemd-halt.service.8 \ + man/systemd-hibernate-resume@.service.8 \ man/systemd-inhibit.1 \ man/systemd-initctl.service.8 \ man/systemd-journald.service.8 \ @@ -199,6 +200,7 @@ MANPAGES_ALIAS += \ man/systemd-firstboot.service.1 \ man/systemd-fsck-root.service.8 \ man/systemd-fsck.8 \ + man/systemd-hibernate-resume.8 \ man/systemd-hibernate.service.8 \ man/systemd-hybrid-sleep.service.8 \ man/systemd-initctl.8 \ @@ -305,6 +307,7 @@ man/systemd-ask-password-wall.service.8: man/systemd-ask-password-console.servic man/systemd-firstboot.service.1: man/systemd-firstboot.1 man/systemd-fsck-root.service.8: man/systemd-fsck@.service.8 man/systemd-fsck.8: man/systemd-fsck@.service.8 +man/systemd-hibernate-resume.8: man/systemd-hibernate-resume@.service.8 man/systemd-hibernate.service.8: man/systemd-suspend.service.8 man/systemd-hybrid-sleep.service.8: man/systemd-suspend.service.8 man/systemd-initctl.8: man/systemd-initctl.service.8 @@ -567,6 +570,9 @@ man/systemd-fsck-root.service.html: man/systemd-f...@.service.html man/systemd-fsck.html: man/systemd-f...@.service.html $(html-alias) +man/systemd-hibernate-resume.html: man/systemd-hibernate-res...@.service.html + $(html-alias) + man/systemd-hibernate.service.html: man/systemd-suspend.service.html $(html-alias) @@ -1619,6 +1625,7 @@ EXTRA_DIST += \ man/systemd-getty-generator.xml \ man/systemd-gpt-auto-generator.xml \ man/systemd-halt.service.xml \ + man/systemd-hibernate-res...@.service.xml \ man/systemd-hostnamed.service.xml \ man/systemd-inhibit.xml \ man/systemd-initctl.service.xml \ diff --git a/Makefile.am b/Makefile.am index cbf98bd..a487caa 100644 --- a/Makefile.am +++ b/Makefile.am @@ -378,7 +378,8 @@ rootlibexec_PROGRAMS = \ systemd-sleep \ systemd-bus-proxyd \ systemd-socket-proxyd \ - systemd-update-done + systemd-update-done \ + systemd-hibernate-resume systemgenerator_PROGRAMS = \ systemd-getty-generator \ @@ -528,7 +529,8 @@ nodist_systemunit_DATA = \ units/initrd-udevadm-cleanup-db.service \ units/initrd-switch-root.service \ units/systemd-nspawn@.service \ - units/systemd-update-done.service + units/systemd-update-done.service \ + units/systemd-hibernate-resume@.service dist_userunit_DATA = \ units/user/basic.target \ @@ -575,7 +577,8 @@ EXTRA_DIST += \ units/initrd-udevadm-cleanup-db.service.in \ units/initrd-switch-root.service.in \ units/systemd-nsp...@.service.in \ - units/systemd-update-done.service.in + units/systemd-update-done.service.in \ + units/systemd-hibernate-res...@.service.in CLEANFILES += \ units/console-shell.service.m4 \ @@ -2103,6 +2106,14 @@ systemd_delta_LDADD = \ libsystemd-shared.la # -- +systemd_hibernate_resume_SOURCES = \ + src/hibernate-resume/hibernate-resume.c + +systemd_hibernate_resume_LDADD = \ + libsystemd-internal.la \ + libsystemd-shared.la + +# -- systemd_getty_generator_SOURCES = \ src/getty-generator/getty-generator.c diff --git a/man/systemd-hibernate-res...@.service.xml b/man/systemd-hi