Reply to some of the points raised:
- Doesn't daemonize as usual -- is this intentional to ease upstart
tracking the service?
It's not required becaues the service is only ever started as a result
of D-Bus activation. D-Bus is okay with that.
- running_in_chroot() is liable to not function reliably in new namespaces;
expect this to work only for traditional chroot environments
- detect_container() may give silly results if kernel command line includes
"container=..." string or if the userns namespace started init with funny
environment variables
This code is copied straight out of upstream systemd sources -- I'm
trying to emulate what they do here.
- _set_using_ntpdate() uses '/bin/mv -f' rather than rename(2)
- _set_using_ntpdate() does not stop ntpd before starting ntpdate
- _set_using_ntpdate() returns TRUE if ntpdate is not available
- _set_using_ntpd() returns TRUE if ntpd is not available
- _set_using_ntp_debian() returns TRUE if:
(a) ntpd doesn't exist and ntpdate doesn't exist
(b) ntpd doesn't exist and ntpdate started once
(c) ntpd doesn't exist and if-up.d/ntpdate and if-up.d/ntpdate.disabled
don't exist
(d) ntpd was (re)started or stopped and ntpdate doesn't exist
(e) ntpd was (re)started or stopped and ntpdate started once
(f) ntpd was (re)started or stopped and if-up.d/ntpdate and
if-up.d/ntpdate.disabled don't exist
returns FALSE if errors returned while executing:
(a) g_strconcat ("/usr/sbin/update-rc.d ntp ", using_ntp ? "enable" :
"disable", NULL);
(b) g_strconcat ("/usr/sbin/service ntp ", using_ntp ? "restart" : "stop",
NULL);;
(c) "/bin/mv -f "NTPDATE_DISABLED" "NTPDATE_ENABLED;
(d) "/bin/mv -f "NTPDATE_ENABLED" "NTPDATE_DISABLED;
(e) "/etc/network/if-up.d/ntpdate"
The NTP code is gross but we already have it in Ubuntu as part of the
gnome-settings-daemon datetime mechanism. It's true that this could use
some cleaning up, but I wanted to go with 'tried and tested' rather than
rewriting it too much.
- return value of detect_virtualization() is ignored
This is a good catch. I ignored the return value because in the case of
failure I was just going to return the empty string, but on closer
reading of the code I now see that detect_virtualization() doesn't
actually set the return value in the failure case. I'll fix this by
initialising to "".
- shim_get_property allocates memory with g_variant_new(), appears leaked.
Is this the intentional way to use DBus?
>From the GDBus documentation for the callback type of this function:
Returns :
A GVariant with the value for property_name or NULL if error is set. If
the returned GVariant is floating, it is consumed - otherwise its
reference count is decreased by one.
I'll fix up the virtualisation thing, but I think we should avoid
refactoring the NTP code for now and clean it up after raring.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1153633
Title:
[MIR] systemd-shim
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/systemd-shim/+bug/1153633/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
