On Wed, Oct 08, 2014 at 11:54:19AM +0200, Lennart Poettering wrote:
> On Tue, 07.10.14 13:35, Jan Synacek (jsyna...@redhat.com) wrote:
>
> > ---
> > src/shared/install.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/src/shared/install.c b/src/shared/install.c
> > index fa064c2..945bb27 100644
> > --- a/src/shared/install.c
> > +++ b/src/shared/install.c
> > @@ -1516,6 +1516,19 @@ int unit_file_enable(
> > return r;
> >
> > STRV_FOREACH(i, files) {
> > + UnitFileState state;
> > +
> > + state = unit_file_get_state(scope, root_dir, *i);
> > + if (state < 0) {
> > + log_error("Failed to get unit file state for %s:
> > %s", *i, strerror(-state));
> > + return state;
> > + }
> > +
> > + if (state == UNIT_FILE_MASKED || state ==
> > UNIT_FILE_MASKED_RUNTIME) {
> > + log_error("Failed to enable unit: Unit %s is
> > masked", *i);
> > + return -ENOTSUP;
> > + }
> > +
>
> Looks mostly OK. However, we should probably use a more useful error
> here. Maybe EADDRNOTAVAIL or so. Even better though would be to
> actually change the call to take an sd_bus_error argument and then
> return a proper error message that can be passed back to the bus
> clients with a real explanation.
Yes, EADDRNOTAVAIL is used in another place where the unit is masked.
I'm not sure which one is more "useful" :). But hopefully this will be
an internal-only thing once meaningful dbus error messages are provided.
The problem with passing dbus *error into the lower levels is that
the functions live in src/share/, and were so far not linked with the
bus libraries. I think that the best way to handle this would be to
use a temporary structure like
{ char *unit_name; char *error_message; int code}
and use this to pass the information about the error from the lower
to the upper levels. But maybe I'm overcomplicating things.
--
A related thing: there's a mapping bus-error <-> errno implemented,
but it only works for the errors defined in the library itself. It
would be nice to extend this mapping to the "user" defined errors,
e.g. in core/. Would you be amenable to adding a mechianism to
register additional mappings like bus-error-mapping.gperf so that they
are used by the library?
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
