On Wed, 08.10.14 15:29, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 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. Hmm, yeah, and if we did it would be cyclic and stuff... > 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. Hmm, maybe a simply solution would be to convert EADDRNOTAVAIL into a proper sd_bus_error on the calling side, that shouldn't be too difficult. > 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? Maybe have internal versions of the conversion calls that allow passing in an additional table? Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel