Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 546168 - app-emulation/libvirt calls systemd_install_serviced incorrectly
Summary: app-emulation/libvirt calls systemd_install_serviced incorrectly
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Matthias Maier
URL:
Whiteboard:
Keywords:
: 545828 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-04-10 11:20 UTC by Geaaru
Modified: 2015-04-17 16:13 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Geaaru 2015-04-10 11:20:22 UTC
On systemd_install_serviced funtion insinto command contains this command:

insinto /etc/systemd/system/"${service}".d

that for example on libvirt create /etc/systemd/system/libvirtd.d directory.

This is wrong, systemd require a path like this:

${service}.service.d/

So, correct command to fix on systemd_install_serviced must be:

insinto /etc/systemd/system/"${service}".service.d

Bye
Comment 1 Mike Gilbert gentoo-dev 2015-04-17 13:20:05 UTC
You are supposed to call it with the ".service" included.

systemd_install_serviced foo.conf libvirtd.service
Comment 2 Geaaru 2015-04-17 13:46:42 UTC
Hi,

with a fast grep under portage directory all ebuild present use only one parameter on call systemd_install_serviced function.
So systemd.eclass must be change to fix systemd_install_serviced in a way that install right directory or change requirement of second parameter as mandatary.

I fix this on my layman as describe on previous message and all works fine.

Bye
Comment 3 Matthias Maier gentoo-dev 2015-04-17 14:31:48 UTC
*libvirt-1.2.13-r1 (17 Apr 2015)
*libvirt-1.2.14-r1 (17 Apr 2015)
*libvirt-1.2.12-r1 (17 Apr 2015)
*libvirt-1.2.11-r4 (17 Apr 2015)
*libvirt-1.2.10-r5 (17 Apr 2015)

  17 Apr 2015; Matthias Maier <tamiko@gentoo.org> +libvirt-1.2.10-r5.ebuild,
  +libvirt-1.2.11-r4.ebuild, +libvirt-1.2.12-r1.ebuild,
  +libvirt-1.2.13-r1.ebuild, +libvirt-1.2.14-r1.ebuild,
  -libvirt-1.2.11-r3.ebuild, -libvirt-1.2.13.ebuild, -libvirt-1.2.14.ebuild,
  libvirt-9999.ebuild:
  Fix install location for libvirtd.service.d/00gentoo file; bug #545828, bug
  #546168
Comment 4 Matthias Maier gentoo-dev 2015-04-17 14:32:13 UTC
*** Bug 545828 has been marked as a duplicate of this bug. ***
Comment 5 Mike Gilbert gentoo-dev 2015-04-17 15:19:04 UTC
(In reply to Geaaru from comment #2)

I think you are failing to read the ebuild, the eclass, or both. The second parameter is optional.
Comment 6 Geaaru 2015-04-17 15:58:24 UTC
Hi,

I just reply to your comment where you suggest to use of second parameter with libvirtd.service as second parameter instead of fix function in systemd.eclass file.

To fix this bug there are at least two solutions:

1. fix systemd.eclass and add .service on insinto command of the systemd_install_serviced function

2. fix libvirt ebuild and all others ebuild that use same function without second parameter without .service

In my layman I choice solution 1 instead of solution 2 to avoid use of second parameter because FILESDIR file it was libvirtd.conf without .service.
While now, FILESDIR file contains also ".service" in filename so could be avoid use of second parameter.

Probably could be useful add a check that if "src" is without ".service" string add .service or generate an error.

A patch like this:

--- /usr/portage/eclass/systemd.eclass	2014-05-31 12:31:17.000000000 +0200
+++ systemd_new.eclass	2015-04-17 17:56:37.706730207 +0200
@@ -177,9 +177,8 @@
 	[[ ${src} ]] || die "No file specified"
 
 	if [[ ! ${service} ]]; then
-		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
+		[[ ${src} == *.service.conf ]] || die "Source file needs .service.conf suffix"
 		service=${src##*/}
-		service=${service%.conf}
 	fi
 	# avoid potentially common mistake
 	[[ ${service} == *.d ]] && die "Service must not have .d suffix"



I will change my layman to revert my changes to insinto and I add check of src content.

Thank you very much at all for fast bug fix.

Bye
Comment 7 Mike Gilbert gentoo-dev 2015-04-17 16:06:24 UTC
(In reply to Geaaru from comment #6)

There's nothing left to fix here, except your overlay. Most (all?) existing ebuilds in the tree call the function with valid parameters. If you find ebuild that do it incorrectly please file bugs.

I suppose we could add some sanity checking, but that doesn't mean the function is broken.
Comment 8 Geaaru 2015-04-17 16:13:53 UTC
Yes, I think that could be helpful add sanity check. No bug are present in current function of systemd.eclass. Bug it was in ebuild of libvirtd.

And sorry I post again correct patch for sanity check (previous it was wrong):

diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
index 687a16a..8870969 100644
--- a/eclass/systemd.eclass
+++ b/eclass/systemd.eclass
@@ -177,7 +177,7 @@ systemd_install_serviced() {
        [[ ${src} ]] || die "No file specified"
 
        if [[ ! ${service} ]]; then
-               [[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
+               [[ ${src} == *.service.conf ]] || die "Source file needs .service.conf suffix"
                service=${src##*/}
                service=${service%.conf}
        fi