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

2014-08-27 Thread Thomas Bächler
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


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

2014-08-27 Thread Andrei Borzenkov
On Wed, Aug 27, 2014 at 10:18 AM, 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.


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.

2014-08-27 Thread 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-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.

2014-08-27 Thread Thomas Bächler
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.

2014-08-27 Thread Lennart Poettering
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.

2014-08-27 Thread Lennart Poettering
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 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.
 
 
 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


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

2014-08-26 Thread 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.
---
 .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