1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] spec: don't package product dirs

Nikolay Shirokovskiy posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1506580545-781427-1-git-send-email-nshirokovskiy@virtuozzo.com
libvirt.spec.in | 2 --
1 file changed, 2 deletions(-)
[libvirt] [PATCH] spec: don't package product dirs
Posted by Nikolay Shirokovskiy 6 years, 6 months ago 
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
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: don't package product dirs
Posted by Nikolay Shirokovskiy 6 years, 5 months ago 
ping

On 28.09.2017 09:35, 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
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: don't package product dirs
Posted by Martin Kletzander 6 years, 5 months ago 
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.

>--
>1.8.3.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: don't package product dirs
Posted by Nikolay Shirokovskiy 6 years, 5 months ago 

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.

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:

[localhost]# ls dir
boot  dnsmasq  filesystems  images  libxl  lockd  lxc  network  qemu  sanlock  uml  xen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: don't package product dirs
Posted by Martin Kletzander 6 years, 5 months ago 
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:
>
>[localhost]# ls dir
>boot  dnsmasq  filesystems  images  libxl  lockd  lxc  network  qemu  sanlock  uml  xen

Honestly, I'm not sure how those details work in the specfile, maybe Jirka
(Cc'd) could share some more thoughts.

Martin--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: don't package product dirs
Posted by Jiri Denemark 6 years, 4 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.