core: add ExecXYZEx= bus hook ups to all exec command properties #13369

anitazha · 2019-08-21T01:16:47Z

The "Ex" variant was originally only added for ExecStartXYZ= but it makes
sense to have feature parity for the rest of the exec command properties
as well (e.g. ExecReload=, ExecStop=, etc).

keszybz · 2019-08-22T08:10:35Z

Hmm, could we have some test that does what it is supposed to do? Maybe somewhere in the test/TEST-* ? It's easy to make a typo in stuff like this, so we should have at least a smoke test that makes a pass through the code.

mrc0mmand

I agree with @keszybz that having a test for this would be great :-)

mrc0mmand · 2019-08-22T08:22:49Z

src/systemctl/systemctl.c

@@ -5534,16 +5534,22 @@ static int show_one(
                { "IPEgressBytes",                  "t",               NULL,           offsetof(UnitStatusInfo, ip_egress_bytes)                   },
                { "IOReadBytes",                    "t",               NULL,           offsetof(UnitStatusInfo, io_read_bytes)                     },
                { "IOWriteBytes",                   "t",               NULL,           offsetof(UnitStatusInfo, io_write_bytes)                    },
+                { "ExecCondition",                  "a(sasbttttuii)",  map_exec,       0                                                           },
+                { "ExecCondition",                  "a(sasasttttuii)", map_exec,       0                                                           },


Shouldn't this be ExecConditionEx?

Herp derp. Yes.

anitazha · 2019-09-16T14:55:38Z

I've added new tests and fixed the copy pasta. PTAL.

keszybz · 2019-09-17T08:48:10Z

semaphoreci simply timed out during build. I'll restart it.

bionic-arm64 fails:

ExecReload={ path=/bin/echo ; argv[]=/bin/echo ${5_five_ex} ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }
Sent message type=method_return sender=org.freedesktop.systemd1 destination=n/a path=n/a interface=n/a member=n/a cookie=1 reply_cookie=1 signature=a{sv} error-name=n/a error-message=n/a
Bus private-bus-connection: changing state RUNNING → CLOSING
Bus private-bus-connection: changing state CLOSING → CLOSED
Got disconnect on private connection.
+ grep -F 'path=/bin/echo ; argv[]=/bin/echo ${5_five_ex} ; flags=no-env-expand'
+ systemctl show -p ExecReloadEx 5_five_ex
Bus bus-api-system: changing state AUTHENTICATING → HELLO
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello cookie=1 reply_cookie=0 signature=n/a error-name=n/a erro>
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=AddMatch cookie=2 reply_cookie=0 signature=s error-name=n/a err>
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=GetNameOwner cookie=3 reply_cookie=0 signature=s error-name=n/a>
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=AddMatch cookie=4 reply_cookie=0 signature=s error-name=n/a err>
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=RequestName cookie=5 reply_cookie=0 signature=su error-name=n/a>
Got message type=method_return sender=org.freedesktop.DBus destination=:1.1 path=n/a interface=n/a member=n/a cookie=1 reply_cookie=1 signature=s error-name=n/a error-message=n/a
Bus bus-api-system: changing state HELLO → RUNNING
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=ListNames cookie=6 reply_cookie=0 signature=n/a error-name=n/a >
Got message type=method_return sender=org.freedesktop.DBus destination=:1.1 path=n/a interface=n/a member=n/a cookie=8 reply_cookie=6 signature=as error-name=n/a error-message=n/a
Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=GetNameOwner cookie=7 reply_cookie=0 signature=s error-name=n/a>
Got message type=method_return sender=org.freedesktop.DBus destination=:1.1 path=n/a interface=n/a member=n/a cookie=9 reply_cookie=7 signature=s error-name=n/a error-message=n/a
systemd-logind.service: D-Bus name org.freedesktop.login1 now registered by :1.0
systemd-logind.service: Changed start -> running
systemd-logind.service: Job 58 systemd-logind.service/start finished, result=done
[  OK  ] Started Login Service.
systemd-journald.service: Got notification message from PID 16 (FDSTORE=1)
systemd-journald.service: Added fd 18 (n/a) to fd store.
Bus private-bus-connection: changing state UNSET → OPENING
Bus private-bus-connection: changing state OPENING → AUTHENTICATING
Accepted new private connection.
Bus private-bus-connection: changing state AUTHENTICATING → RUNNING
Got message type=method_call sender=n/a destination=org.freedesktop.systemd1 path=/org/freedesktop/systemd1/unit/_35_5ffive_5fex_2eservice interface=org.freedesktop.DBus.Properties member=GetAll cookie=>
Sent message type=method_return sender=org.freedesktop.systemd1 destination=n/a path=n/a interface=n/a member=n/a cookie=1 reply_cookie=1 signature=a{sv} error-name=n/a error-message=n/a
Received SIGCHLD from PID 28 (testsuite.sh).
Child 28 (testsuite.sh) died (code=exited, status=2/INVALIDARGUMENT)
testsuite.service: Failed to read oom_kill field of memory.events cgroup attribute: No such file or directory
testsuite.service: Child 28 belongs to testsuite.service.
testsuite.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
testsuite.service: Failed with result 'exit-code'.

It seems that ExecReloadEx is somehow wrong...

keszybz · 2019-09-17T08:58:45Z

This needs a rebase because TEST-37 is already taken :(

mrc0mmand · 2019-09-17T09:06:17Z

Even my PR (#13300) needs a post-merge fix as it collides with the other TEST-37 test case. However, I'll wait with the fix until this PR is resolved, as the number here should be bumped to 38, or even 39, due to TEST-37-FREEZER from #13512, which needs to be bumped as well :-)

keszybz · 2019-09-17T09:09:50Z

Oh. I think we need some better way of assigning those numbers.

keszybz · 2019-09-17T09:24:29Z

I run the test on arm64 for a while in a loop, and it succeeds without issue. Dunno, maybe let's treat this is a fluke. I don't see why arm64 would behave any different here. So please rename the test to TEST-40-… and we can merge.

The "Ex" variant was originally only added for ExecStartXYZ= but it makes sense to have feature parity for the rest of the exec command properties as well (e.g. ExecReload=, ExecStop=, etc).

anitazha · 2019-09-17T11:04:25Z

Bumped test number to 40

Summary: New upstream release, plus a backport of systemd/systemd#13369 Reviewed By: anitazha Differential Revision: D17631247 fbshipit-source-id: e21450480a66af1a2e28dbe2a5612f3ddf62d4d8

mrc0mmand reviewed Aug 22, 2019

View reviewed changes

yuwata added the pid1 label Sep 8, 2019

anitazha force-pushed the more_ex branch from d35d5ea to 3c65905 Compare September 16, 2019 14:52

anitazha force-pushed the more_ex branch from 3c65905 to c1384d5 Compare September 16, 2019 16:49

keszybz added the needs-rebase label Sep 17, 2019

mrc0mmand mentioned this pull request Sep 17, 2019

test: rename TEST-EXECRELOAD to avoid name conflict #13575

Merged

core: add ExecXYZEx= bus hook ups to all exec command properties

9ecc722

The "Ex" variant was originally only added for ExecStartXYZ= but it makes sense to have feature parity for the rest of the exec command properties as well (e.g. ExecReload=, ExecStop=, etc).

anitazha force-pushed the more_ex branch from c1384d5 to 9ecc722 Compare September 17, 2019 11:04

keszybz added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed needs-rebase labels Sep 17, 2019

mrc0mmand merged commit 898fc00 into systemd:master Sep 17, 2019

evverx mentioned this pull request Sep 30, 2019

systemd-sleep: prefer resume device or file #13682

Merged

anitazha deleted the more_ex branch October 4, 2019 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: add ExecXYZEx= bus hook ups to all exec command properties #13369

core: add ExecXYZEx= bus hook ups to all exec command properties #13369

anitazha commented Aug 21, 2019

keszybz commented Aug 22, 2019

mrc0mmand left a comment

mrc0mmand Aug 22, 2019

anitazha Aug 22, 2019

anitazha commented Sep 16, 2019

keszybz commented Sep 17, 2019

keszybz commented Sep 17, 2019

mrc0mmand commented Sep 17, 2019

keszybz commented Sep 17, 2019

keszybz commented Sep 17, 2019

anitazha commented Sep 17, 2019

core: add ExecXYZEx= bus hook ups to all exec command properties #13369

core: add ExecXYZEx= bus hook ups to all exec command properties #13369

Conversation

anitazha commented Aug 21, 2019

keszybz commented Aug 22, 2019

mrc0mmand left a comment

Choose a reason for hiding this comment

mrc0mmand Aug 22, 2019

Choose a reason for hiding this comment

anitazha Aug 22, 2019

Choose a reason for hiding this comment

anitazha commented Sep 16, 2019

keszybz commented Sep 17, 2019

keszybz commented Sep 17, 2019

mrc0mmand commented Sep 17, 2019

keszybz commented Sep 17, 2019

keszybz commented Sep 17, 2019

anitazha commented Sep 17, 2019