[OE-core] [PATCH 3/3] pulseaudio: fix to manage user services corretly

[Pau Espin Pedrol]

Hi,

I think I didn't express myself correctly. Please note I did the
longer investigations quite a while ago and I'm mainly talking from
memory, so I may be wrong in some of the assumptions.

>From systemd.bbclass, you can see this:
            if service.find('.socket') != -1:
                # for *.socket add *.service and *@.service
                service_base = service.replace('.socket', '')
                systemd_add_files_and_parse(pkg_systemd, path,
service_base + '.service', keys)
                systemd_add_files_and_parse(pkg_systemd, path,
service_base + '@.service', keys)


So, for installation purposes, no .service is required in
SYSTEMD_USER_SERVICE. Setting it .socket should also install (but not
enable) the .service file together with the .socket one into the
image, as actually the .socket one usually depends on the .service
file at runtime.

That being said, of course it's not the same behavior if you add only
one of them, the other or both:
1- Adding only .service -> It will install + enable the .service
2- Adding only the .socket -> It will install both. It will enable .socket
3- Adding .service + .socket -> It will install both. It will enable both.

Usually, you want either only the .service to be enabled (1, which
will start it automatically at startup of user session), or only the
.socket enabled (2, which will start only the socket at startup of
user session, and only start pulseaudio process when required by some
pulseaudio client). The third case, that is, installing + enabling
both is usually not a good idea, at least it makes no sense to me.

So, we should consider only "1" or "2". I would personally go for 2nd
option, and probably disable pulseaudio own autospawn system as
explained by Tanu. This way we don't start pulseaudio unless when it's
actually needed.

That is:
+SYSTEMD_PACKAGES = "${PN}-server"
+SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.socket"

And by the way, you  should be able to remove
"${systemd_user_unitdir}/*" from FILES_${PN}-server as it should be
handled automatically by systemd.bbclass. If that's not the case, then
probably there's some error in systemd.bbclass you should look into.

Hope I explained myself better now. Regards!


2016-09-08 13:22 GMT+02:00 Tanu Kaskinen <tanuk at iki.fi>:
> On Thu, 2016-09-08 at 14:34 +0800, ChenQi wrote:
>> On 09/07/2016 06:29 PM, Pau Espin Pedrol wrote:
>> >
>> >
>> >
>> > Pau Espin Pedrol
>> >
>> > 2016-09-07 11:22 GMT+02:00 Chen Qi <Qi.Chen at windriver.com
>> > <mailto:Qi.Chen at windriver.com>>:
>> >
>> >     Make use of the new SYSTEMD_USER_SERVICE variable added in
>> >     systemd.bbclass
>> >     to manage user services in pulseaudio-server package.
>> >
>> >     Signed-off-by: Chen Qi <Qi.Chen at windriver.com
>> >     <mailto:Qi.Chen at windriver.com>>
>> >     ---
>> >      meta/recipes-multimedia/pulseaudio/pulseaudio.inc | 4 ++--
>> >      1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> >     diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
>> >     b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
>> >     index 6ed79ef..f3754d7 100644
>> >     --- a/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
>> >     +++ b/meta/recipes-multimedia/pulseaudio/pulseaudio.inc
>> >     @@ -124,8 +124,8 @@ FILES_${PN}-conf = "${sysconfdir}"
>> >      FILES_${PN}-bin +=
>> >     "${sysconfdir}/default/volatiles/volatiles.04_pulse"
>> >      FILES_${PN}-server = "${bindir}/pulseaudio ${bindir}/start-*
>> >     ${sysconfdir} ${bindir}/pactl */udev/rules.d/*.rules
>> >     */*/udev/rules.d/*.rules ${systemd_user_unitdir}/*"
>> >
>> >     -#SYSTEMD_PACKAGES = "${PN}-server"
>> >     -SYSTEMD_SERVICE_${PN}-server = "pulseaudio.service"
>> >     +SYSTEMD_PACKAGES = "${PN}-server"
>> >     +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.service
>> >     pulseaudio.socket"
>> >
>> > I think specifying "pulseaudio.socket" for
>> > SYSTEMD_USER_SERVICE_${PN}-server should be enough, systemd.bbclass is
>> > going to add the .service file afair.
>>
>> Add both:
>> chenqi at pek-hostel-deb01:~/poky/build-systemd [1] $ ls
>> tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user
>> default.target.wants  sockets.target.wants
>>
>> Add pulseaudio.socket:
>> chenqi at pek-hostel-deb01:~/poky/build-systemd [1] $ ls
>> tmp/work/qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/etc/systemd/user
>> sockets.target.wants
>>
>> They have different results.
>> I don't know a lot about pulseaudio. Can you confirm that
>> pulseaudio.socket is enough?
>
> If I understood correctly, Pau's argument was that there would be no
> difference, which doesn't seem to be the case. However, if
> pulseaudio.service isn't included in default.target.wants, that
> shouldn't be a big problem, because socket activation will anyway start
> the service when needed. On the other hand, I don't
> think SYSTEMD_USER_SERVICE is intended to be used for controlling which
> units to enable - it looks more like a variable that should contain all
> units contained in the package. If my understanding is correct, you
> should list both units in the variable.
>
> The distributions that I've seen to use systemd to manage pulseaudio
> only enable pulseaudio.socket, so that socket activation is used rather
> than starting pulseaudio automatically as part of the user session. I
> don't know if OE supports that - it looks like there's only a toggle to
> enable all units or none. Other distributions also patch client.conf to
> disable pulseaudio's own autospawning mechanism, since it's redundant
> when using socket activation. Leaving autospawning enabled shouldn't
> break anything in normal use, but it can cause some confusing behaviour
> if the user tries to manually disable automatic pulseaudio starting by
> disabling the systemd units.
>
> --
> Tanu
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core at lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core