[systemd-devel] Using `systemctl edit` on "invalid" unit names

[Zbigniew Jędrzejewski-Szmek]

On Wed, Dec 17, 2014 at 11:29:38AM +0300, Ivan Shapovalov wrote:
> On Tuesday, December 16, 2014 at 06:35:09 AM, Zbigniew Jędrzejewski-Szmek wrote:	
> > On Sun, Dec 14, 2014 at 04:21:32PM +0300, Ivan Shapovalov wrote:
> > > On Saturday 13 December 2014 at 15:34:01, Ronny Chevalier wrote:	
> > > > 2014-12-13 11:33 GMT+01:00 Ivan Shapovalov <intelfx100 at gmail.com>:
> > > > > Hello all,
> > > > 
> > > > Hi,
> > > > 
> > > > >
> > > > > it seems that the newly added `systemctl edit` command requires its arguments
> > > > > to be valid unit names.
> > > > >
> > > > > This causes `edit` operation to fail in apparently valid use-cases like
> > > > >
> > > > >     systemctl edit getty at .service
> > > > 
> > > > This is fixed in git now, thanks!
> > > > 
> > > > > or
> > > > >     systemctl edit autovt at tty1.service
> > > > >
> > > > > In second case, the error message is especially cryptic:
> > > > > "autovt at tty1.service ignored: not found".
> > > > 
> > > > It worked before and it still works for me.
> > > 
> > > Do you have "getty at tty1.service" explicity enabled? I do have.
> > > 
> > > > > Actually I understand what it does mean: systemctl asks the manager to show
> > > > > unit's FragmentPath -> the manager tries to load the unit -> loading fails with
> > > > > "File exists" because getty at tty1.service is already instantiated.
> > > > 
> > > > I don't see why it should fail for this reason ?
> > > > 
> > > > >
> > > > > (BTW, it's a separate question: is this failure valid or is it a bug?)
> > > > >
> > > > 
> > > > systemctl edit getty at .service, should have worked before so yes this was a bug.
> > > 
> > > Now both `edit getty@` and `edit getty at tty1` work, but I'd expect the latter
> > > to honor the template parameter; i. e. create a drop-in for getty at tty1.service...
> > > Is this possible?
> > I made various unifications to the code to make it more maintainable. This
> > case should be fixed too. Please test it... it's easy to miss the corner cases.
> 
> Yes, that works now, thanks! Also the error messages are now a lot cleaner
> and more "in line" with the rest of systemctl.
> 
> I still can't edit autovt at tty1.service (other instances work correctly), but
> I guess that's another problem. However, the error message could be improved
> by querying the manager for LoadFailed and displaying that message instead
> of just saying "does not have any files on disk". I'll try to come up with a patch
> for that.
Right now, I made the path for dropins shared with the code paths in
core/, but not the path for loading the main fragment. So when aliases
are involved, systemctl might do things differently than systemd.
src/core/load-fragmnet.c does something like walk the list of
directories, look for symlinks, and remember all the names. Than it
loads configuration (and dropins) based on the full set of name.

> BTW, would it be good to have a LoadFailedErrno property or something in that line
> to get an errno instead of a string? Or is it better to convert string to errno
> using sd_bus_error_*() methods?
One errno would not be enough, because potentially you could have many dropins
which failed to load for some strange reason. I'd ignore that, you can always
run 'SYSTEMD_LOG_LEVEL=debug systemctl cat --root=/ ...' if you want to have
full logs of what is loaded or not loaded.

> I'm sorry for too many questions :P
Fine with me :)

Zbyszek