On Wed, Nov 15, 2017 at 12:39:27 +0100, Martin Kletzander wrote:
On Tue, Nov 14, 2017 at 06:12:56PM +0300, Nikolay Shirokovskiy
wrote:
>
>
>On 13.11.2017 11:35, Martin Kletzander wrote:
>> On Thu, Sep 28, 2017 at 09:35:45AM +0300, Nikolay Shirokovskiy wrote:
>>> Directories /var/{lib,cache}/libvirt/qemu/ are created by libvirtd on
>>> start and their owner:group is changed according to the config. Thus
>>> no need to include them in libvirt-daemon-driver-qemu package. Otherwise
>>> we see noisy "directory changed" on rpm -V for the package.
>>> ---
>>> libvirt.spec.in | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>>> index a3bd77f..e20f65c 100644
>>> --- a/libvirt.spec.in
>>> +++ b/libvirt.spec.in
>>> @@ -1911,8 +1911,6 @@ exit 0
>>> %config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf
>>> %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu
>>> %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
>>> -%dir %attr(0751, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/
>>> -%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/cache/libvirt/qemu/
>>> %{_datadir}/augeas/lenses/libvirtd_qemu.aug
>>> %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
>>> %{_libdir}/%{name}/connection-driver/libvirt_driver_qemu.so
>>
>> I agree, however I think this should be done on all subdirectories of
>> %{_localstatedir}/{cache,lib}/libvirt/. That directories are owned by
>> libvirt-daemon and libvirt-libs respectively, so it should be OK.
>
>I don't think we can generalize this patch for other hypervisors. For example
>lxc don't create "%{_localstatedir}/lib/libvirt/lxc/" in runtime thus
this
>directory need to be installed.
>
We created the qemu/ dir and changed the permissions because domains needed to
access stuff inside it. Nowadays we actually create per-domain subdirectories
so to fix this we could just remove the chmod() call from qemu driver.
>I also give this patch more thought and think we can fix issue differently.
>
>- use %ghost directive just like for run dir so that we cleanup on uninstall
additionally
> or
>- install dir as root:root and don't change owner at runtime. Why do we need
>to make lib and cache group:owner qemu:qemu in the first place? Looks like
>we only store qemu caps cache and domain screenshots in cache and both files
>are created by daemon itself. And lib dir only have directories which
>in turn created by daemon too:
I think the second option, i.e., installing the directories as root:root
and dropping chown() from qemu driver is the best solution here. We have
per-domain directories so there's no reason to keep the parent owned by
qemu_group.
Jirka