[libvirt] [PATCH] spec: Only call ldconfig on RHEL7

Cole Robinson posted 1 patch 5 years, 1 month ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/a46dc99a3c38dfd1b051468e82ff25eaae83473f.1552937725.git.crobinso@redhat.com
libvirt.spec.in | 4 ++++
1 file changed, 4 insertions(+)
[libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Cole Robinson 5 years, 1 month ago
Since Fedora 28 (our minimum supported build), ldconfig is called
automatically for us:

https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets

These changes appear to be implemented for rhel > 7 as well, so only
run ldconfig on rhel7

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 libvirt.spec.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 1497cad3d2..445fddc801 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1477,12 +1477,16 @@ exit 0
 
 %post client
 
+%if 0%{?rhel} == 7
 /sbin/ldconfig
+%endif
 %systemd_post libvirt-guests.service
 
 %postun client
 
+%if 0%{?rhel} == 7
 /sbin/ldconfig
+%endif
 %systemd_postun libvirt-guests.service
 
 %triggerun client -- libvirt < 0.9.4
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Andrea Bolognani 5 years, 1 month ago
On Mon, 2019-03-18 at 15:35 -0400, Cole Robinson wrote:
> Since Fedora 28 (our minimum supported build), ldconfig is called
> automatically for us:
> 
> https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
> 
> These changes appear to be implemented for rhel > 7 as well, so only
> run ldconfig on rhel7

s/rhel/RHEL/g

Also we are allowed to say "RHEL 8" now ;)

[...]
> +++ b/libvirt.spec.in
> @@ -1477,12 +1477,16 @@ exit 0
>  
>  %post client
>  
> +%if 0%{?rhel} == 7
>  /sbin/ldconfig
> +%endif
>  %systemd_post libvirt-guests.service
>  
>  %postun client
>  
> +%if 0%{?rhel} == 7
>  /sbin/ldconfig
> +%endif
>  %systemd_postun libvirt-guests.service
>  
>  %triggerun client -- libvirt < 0.9.4

IIUC ldconfig should be called after installing the libraries, but
here we're calling it in %post(un) for the client subpackage which
I believe is incorrect. We should fix that too.

Your changes look good otherwise, and it doesn't really matter in
which order the changes are applied, so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Cole Robinson 5 years, 1 month ago
On 3/19/19 4:13 AM, Andrea Bolognani wrote:
> On Mon, 2019-03-18 at 15:35 -0400, Cole Robinson wrote:
>> Since Fedora 28 (our minimum supported build), ldconfig is called
>> automatically for us:
>>
>> https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
>>
>> These changes appear to be implemented for rhel > 7 as well, so only
>> run ldconfig on rhel7
> 
> s/rhel/RHEL/g
> 
> Also we are allowed to say "RHEL 8" now ;)
> 
> [...]
>> +++ b/libvirt.spec.in
>> @@ -1477,12 +1477,16 @@ exit 0
>>   
>>   %post client
>>   
>> +%if 0%{?rhel} == 7
>>   /sbin/ldconfig
>> +%endif
>>   %systemd_post libvirt-guests.service
>>   
>>   %postun client
>>   
>> +%if 0%{?rhel} == 7
>>   /sbin/ldconfig
>> +%endif
>>   %systemd_postun libvirt-guests.service
>>   
>>   %triggerun client -- libvirt < 0.9.4
> 
> IIUC ldconfig should be called after installing the libraries, but
> here we're calling it in %post(un) for the client subpackage which
> I believe is incorrect. We should fix that too.
> 

Calling /sbin/ldconfig from postun is recommended on fedora < 28 (and 
thus I presume rhel7), according to:

https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/

The wiki page in the git commit also indicates that previously packages 
would call /sbin/ldconfig on postun too, with the %postun -p 
/sbin/ldconfig call

> Your changes look good otherwise, and it doesn't really matter in
> which order the changes are applied, so
> 
>    Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> 

I'll wait until friday to push either incase anyone else has comments

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Andrea Bolognani 5 years, 1 month ago
On Tue, 2019-03-19 at 10:49 -0400, Cole Robinson wrote:
> On 3/19/19 4:13 AM, Andrea Bolognani wrote:
> > On Mon, 2019-03-18 at 15:35 -0400, Cole Robinson wrote:
> > >   %post client
> > >   
> > > +%if 0%{?rhel} == 7
> > >   /sbin/ldconfig
> > > +%endif
> > >   %systemd_post libvirt-guests.service
> > >   
> > >   %postun client
> > >   
> > > +%if 0%{?rhel} == 7
> > >   /sbin/ldconfig
> > > +%endif
> > >   %systemd_postun libvirt-guests.service
> > 
> > IIUC ldconfig should be called after installing the libraries, but
> > here we're calling it in %post(un) for the client subpackage which
> > I believe is incorrect. We should fix that too.
> 
> Calling /sbin/ldconfig from postun is recommended on fedora < 28 (and 
> thus I presume rhel7), according to:
> 
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/
> 
> The wiki page in the git commit also indicates that previously packages 
> would call /sbin/ldconfig on postun too, with the %postun -p 
> /sbin/ldconfig call

What I was pointing out is that you're supposed to run ldconfig
after (un)installing libraries: our libraries are shipped in the
libvirt-libs package, but according to the spec file we're running
ldconfig after (un)installing the client binaries instead, and that
seems like a bug to me.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Cole Robinson 5 years, 1 month ago
On 3/19/19 11:59 AM, Andrea Bolognani wrote:
> On Tue, 2019-03-19 at 10:49 -0400, Cole Robinson wrote:
>> On 3/19/19 4:13 AM, Andrea Bolognani wrote:
>>> On Mon, 2019-03-18 at 15:35 -0400, Cole Robinson wrote:
>>>>    %post client
>>>>    
>>>> +%if 0%{?rhel} == 7
>>>>    /sbin/ldconfig
>>>> +%endif
>>>>    %systemd_post libvirt-guests.service
>>>>    
>>>>    %postun client
>>>>    
>>>> +%if 0%{?rhel} == 7
>>>>    /sbin/ldconfig
>>>> +%endif
>>>>    %systemd_postun libvirt-guests.service
>>>
>>> IIUC ldconfig should be called after installing the libraries, but
>>> here we're calling it in %post(un) for the client subpackage which
>>> I believe is incorrect. We should fix that too.
>>
>> Calling /sbin/ldconfig from postun is recommended on fedora < 28 (and
>> thus I presume rhel7), according to:
>>
>> https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/
>>
>> The wiki page in the git commit also indicates that previously packages
>> would call /sbin/ldconfig on postun too, with the %postun -p
>> /sbin/ldconfig call
> 
> What I was pointing out is that you're supposed to run ldconfig
> after (un)installing libraries: our libraries are shipped in the
> libvirt-libs package, but according to the spec file we're running
> ldconfig after (un)installing the client binaries instead, and that
> seems like a bug to me.
> 

Ah yes. Probably missed when libvirt-libs was split out of 
libvirt-client, commit 70b4f0e719cd

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only call ldconfig on RHEL7
Posted by Andrea Bolognani 5 years, 1 month ago
On Tue, 2019-03-19 at 12:10 -0400, Cole Robinson wrote:
> On 3/19/19 11:59 AM, Andrea Bolognani wrote:
> > What I was pointing out is that you're supposed to run ldconfig
> > after (un)installing libraries: our libraries are shipped in the
> > libvirt-libs package, but according to the spec file we're running
> > ldconfig after (un)installing the client binaries instead, and that
> > seems like a bug to me.
> 
> Ah yes. Probably missed when libvirt-libs was split out of
> libvirt-client, commit 70b4f0e719cd

It would certainly look that way.

I guess we didn't get bug reports about this because most people
install libraries, daemon and clients all at the same time.

-- 
Andrea Bolognani / Red Hat / Virtualization

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