Re: [systemd-devel] Using *.path units without races?
Hi!
On Tue, 17 Mar 2020 at 23:52, Michael Chapman wrote:
>
> On Wed, 18 Mar 2020, Uwe Geuder wrote:
> > Hi!
> >
> > I have wondered for a while how I can use *.path units without (too bad)
> > races.
> >
> > Since
> > https://github.com/systemd/systemd/pull/13509/commits/06582e42de65a61d0238a18720a12b6353edb7cd
> > the behaviour has been become much clearer, but I must admit I still
> > don't get it.
>
> That commit does look incomplete to me.
>
> As a quick test, are you able to try out the patch below? This makes
> systemd always check the filesystem when the service stops, rather than
> just relying on the (as of that commit nonexistent) inotify event.
...
> diff --git a/src/core/path.c b/src/core/path.c
> index cb75d778af..a513df97b2 100644
> --- a/src/core/path.c
> +++ b/src/core/path.c
> @@ -759,11 +759,7 @@ static void path_trigger_notify(Unit *u, Unit *other)
> {
> if (p->state == PATH_RUNNING &&
> UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
> log_unit_debug(UNIT(p), "Got notified about unit
> deactivation.");
> -
> -/* Hmm, so inotify was triggered since the
> - * last activation, so I guess we need to
> - * recheck what is going on. */
> -path_enter_waiting(p, false, p->inotify_triggered);
> +path_enter_waiting(p, false, true);
> }
> }
>
Thanks a lot Michael for this quick patch. The patch seems to have
suffered a formatting accident in the mail. However, your intent to
change the 3rd agument of that function call is pretty clear.
I built that change and quickly tested it. It seems to work fine!
Please note that I have not yet done any wider (regression) testing and
my understanding of the code base is not good.
I can share my test code
- test.path -
[Unit]
Description=%n testing racyness of path
[Path]
DirectoryNotEmpty=/tmp/systemd-path-test/
MakeDirectory=true
- test.service -
[Unit]
Description=%n testing racyness of path
[Service]
ExecStart=%h/try/systemd/test.sh
- test.sh -
#!/bin/sh -eu
# Usage: Expects to find .sleep files in ${testdir}
echo "${0} starting"
testdir=/tmp/systemd-path-test/
exit_handler() {
echo "${0} exiting"
}
trap exit_handler EXIT
# assume good faith, don't worry about dotfiles or
# pathologic filenames...
files=$(ls "${testdir}/")
if [ -z "${files}" ] ; then
echo "No files found"
exit
else
echo "Found ${files}"
fi
for f in ${files} ; do
if [ -e "${testdir}/${f}" ] ; then
rm -f "${testdir}/${f}"
else
echo "Race: ${f} has disappeared"
fi
case "${f}" in
*.sleep) # don't worry about non-numeric...
s=$(basename "${f}" .sleep)
echo "sleeping ${s}"
sleep "${s}"
;;
esac
done
--
Test case:
touch /tmp/systemd-path-test/5.sleep ; sleep 2 ; \
touch /tmp/systemd-path-test/6.sleep ; sleep 5 ; \
touch /tmp/systemd-path-test/0.5.sleep
(all 5 commands in one go)
As expected the service gets now invoked 3 times. Without your patch the
second touch command/file is missed and only "handled" together with the
third touch command/file.
Regards,
Uwe
Uwe Geuder
Neuro Event Labs Oy
Tampere, Finland
uwe.gxu...@neuroeventlabs.com (bot check: fix 1 obvious typo)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Reasoning behind sd_bus_error argument to sd_bus_call?
I completely agree that for errors returned by the service, a D-Bus error
is a lot better. However, from what I understand of sd-bus, any errors
returned by the service are encoded in the reply returned by sd_bus_call
and you use sd_bus_message_is_method_error and sd_bus_message_get_error on
the reply to get the actual service error. Where does that leave the
sd_bus_error argument of sd_bus_call? Is it simply another way to get the
error? It seems to be always be set when a local or remote error occurs,
but it can only contain information that I can get by checking the return
value of the function or by checking whether the reply object passed to
sd_bus_call contains an error.
How I would imagine using sd_bus_call:
r = sd_bus_call(..., reply, ...);
if (r < 0) {
// Local error
}
if (sd_bus_message_is_method_error(reply)) {
const sd_bus_error *error = sd_bus_message_get_error(reply);
// Service error
}
But if this is the intended usage, what's the use of the sd_bus_error
argument of sd_bus_call since the above code already handles both the local
error and the remote service error failure paths?
Daan
On Wed, 18 Mar 2020 at 11:57, Simon McVittie wrote:
> On Tue, 17 Mar 2020 at 20:17:05 +0100, Daan De Meyer wrote:
> > I'm documenting sd_bus_call and its async variant and I was wondering
> about the
> > sd_bus_error output parameter that's passed to it. [...] I don't
> > see immediately see the benefit of the sd_bus_error parameter in a D-Bus
> client
> > since I can simply check the return value instead which seems to contain
> the
> > same information looking at the implementation.
>
> The return value is a single int, which according to systemd conventions
> is probably a negative errno value. That's a lot less information than
> a D-Bus error (systemd sd_bus_error, libdbus DBusError or equivalent):
> D-Bus errors consist of a machine-readable name (namespaced by a reversed
> domain name) and a human-readable message.
>
> For the information about *whether* an error occurred, sure, you get the
> same information, but for information about *which* error occurred and why,
> a sd_bus_error is a lot better.
>
> Let's pretend your D-Bus client is interacting with a D-Bus service that
> resembles systemd-timedated. An errno value can give you, at best,
> something like this (where *** marks the part that came from the service's
> reply):
>
> my-client: Error: Unable to set time zone to America/Gotham:
> ***No such file or directory (errno 2)***
>
> whereas a D-Bus error (sd_bus_error) from a well-implemented service can
> give you something a lot more detailed. For example, after you ispect
> the sd_bus_error, you might find that the error above was either of these:
>
> my-client: Error: Unable to set time zone to America/Gotham:
> ***No time zone file for "America/Gotham" found (tried
> "/usr/share/zoneinfo/America/Gotham",
> "/usr/local/share/zoneinfo/America/Gotham")
> (error code com.example.NotTimedated.Error.NoSuchTimezone)***
>
> my-client: Error: Unable to set time zone to America/Gotham:
> ***No time zone data installed (tried "/usr/share/zoneinfo",
> "/usr/local/share/zoneinfo")
> (error code com.example.NotTimedated.Error.TzdataNotInstalled)***
>
> In this example a programmatic client would also be able
> to respond differently to the distinct machine-readable
> errors com.example.NotTimedated.Error.NoSuchTimezone and
> com.example.NotTimedated.Error.TzdataNotInstalled if it wanted to;
> for example it could respond to the second error by trying to use
> PackageKit to install tzdata, which obviously wouldn't be appropriate
> for the first error.
>
> D-Bus errors were inspired by GLib's GError, which is basically a triple
> { domain: interned string, code: int, message: string }, where the domain
> provides extensible uniqueness, and the code is a member of an enum
> determined by the domain.
>
> smcv
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Reasoning behind sd_bus_error argument to sd_bus_call?
On Tue, 17 Mar 2020 at 20:17:05 +0100, Daan De Meyer wrote:
> I'm documenting sd_bus_call and its async variant and I was wondering about
> the
> sd_bus_error output parameter that's passed to it. [...] I don't
> see immediately see the benefit of the sd_bus_error parameter in a D-Bus
> client
> since I can simply check the return value instead which seems to contain the
> same information looking at the implementation.
The return value is a single int, which according to systemd conventions
is probably a negative errno value. That's a lot less information than
a D-Bus error (systemd sd_bus_error, libdbus DBusError or equivalent):
D-Bus errors consist of a machine-readable name (namespaced by a reversed
domain name) and a human-readable message.
For the information about *whether* an error occurred, sure, you get the
same information, but for information about *which* error occurred and why,
a sd_bus_error is a lot better.
Let's pretend your D-Bus client is interacting with a D-Bus service that
resembles systemd-timedated. An errno value can give you, at best,
something like this (where *** marks the part that came from the service's
reply):
my-client: Error: Unable to set time zone to America/Gotham:
***No such file or directory (errno 2)***
whereas a D-Bus error (sd_bus_error) from a well-implemented service can
give you something a lot more detailed. For example, after you ispect
the sd_bus_error, you might find that the error above was either of these:
my-client: Error: Unable to set time zone to America/Gotham:
***No time zone file for "America/Gotham" found (tried
"/usr/share/zoneinfo/America/Gotham",
"/usr/local/share/zoneinfo/America/Gotham")
(error code com.example.NotTimedated.Error.NoSuchTimezone)***
my-client: Error: Unable to set time zone to America/Gotham:
***No time zone data installed (tried "/usr/share/zoneinfo",
"/usr/local/share/zoneinfo")
(error code com.example.NotTimedated.Error.TzdataNotInstalled)***
In this example a programmatic client would also be able
to respond differently to the distinct machine-readable
errors com.example.NotTimedated.Error.NoSuchTimezone and
com.example.NotTimedated.Error.TzdataNotInstalled if it wanted to;
for example it could respond to the second error by trying to use
PackageKit to install tzdata, which obviously wouldn't be appropriate
for the first error.
D-Bus errors were inspired by GLib's GError, which is basically a triple
{ domain: interned string, code: int, message: string }, where the domain
provides extensible uniqueness, and the code is a member of an enum
determined by the domain.
smcv
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
