Bug 786770

Can you describe in more detail what functional problem you're seeing?  For sure, the messages in the log should be fixed, but you've marked this as Regression and blocker and severity high, and at best this seems like severity low to me.  Is there any functional problem here?

The spec file snippet in question is trying to determine whether to run libvirt-guests on the first shutdown after installation, but to not run it if that service has been previously explicitly disabled.  But because it is using chkconfig incorrectly, it fails to enable the service at all, so we have a worse out-of-the-box experience (if you install libvirt, start up a new guest, then restart your machine, your guest won't be saved by libvirt-guests even though that was our out-of-the-box goal).  On the other hand, in a chroot environment, where the service isn't needed, the failure to enable it is harmless and only pollutes the log; and even in the main environment, the only thing we've lost is use of the script on first shutdown after installation, since all subsequent shutdowns have the service in place from system startup.

Conditionally writing the snippet to use chkconfig for RHEL 6 and Fedora 15 but systemd for Fedora 16 would both solve the log message, as well as make the spec file a bit cleaner.  But it's not my highest priority, so I'm hoping someone else can contribute a patch if it bothers them more than it does me.

(In reply to comment #1)
> Can you describe in more detail what functional problem you're seeing?  For
> sure, the messages in the log should be fixed, but you've marked this as
> Regression and blocker and severity high, and at best this seems like severity
> low to me.  Is there any functional problem here?

I didn't check if there are any consequences caused by incorrect behaviour of postinstall script during the installation, so I don't know if there is any functional problem.
This issue is a regression (it doesn't happen with RHEL-6.2) and must be fixed in RHEL-6.3 (no regressions are allowed between releases), could you return the Regression keyword, please? I'm fine with any severity as long as this bug gets fixed in RHEL-6.3.

Looking at the code in question:

    level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2)
    if /sbin/chkconfig --levels $level libvirt-guests; then

if /sbin/runlevel fails, then $level is empty, so /sbin/chkconfig is invoked with a syntax error, and that explains why there is noise in the log.

On a bare-metal machine, /sbin/runlevel never fails, so there is no syntax error.

So the _only_ case where the noise occurs is in a chroot environment, where /sbin/runlevel fails; but that's precisely the type of environment where you _don't_ want to start the libvirt-guests script running.  So we succeeded at the goal, even if we were noisy at getting at this point.

I will review any patches to split the spec file snippet into conditional code based on whether we know we are a systemd service (F16) or chkconfig service (RHEL6, F15), but I repeat that it is not my highest priority, as no one has proven to me that a bare metal install is broken at doing the desired libvirt-guests run on first install, while suppressing the libvirt-guests script if it has previously been explicitly disabled.

Given that, I do not see any functional regression here.

*** Bug 788104 has been marked as a duplicate of this bug. ***

If I'm reading 'man chkconfig' correctly, it looks like we can consolidate:
    level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2)
    if /sbin/chkconfig --levels $level libvirt-guests; then
into the shorter:
    if /sbin/chkconfig libvirt-guests; then
and get the same behavior, but without the syntax error if /sbin/runlevel doesn't evaluate to something that populates $level.

also get the following info in the install.log:
...
Installing libvirt-0.9.9-2.el6.x86_64
cannot determine current run level
...

# /sbin/runlevel 
N 5

I need to test the exact case when /sbin/runlevel returns unknown and libvirt-guests script is added to the runlevel in which the installation is running. if in this case "/sbin/chkconfig libvirt-guests" returns with 0, then it seems like the best option to do it exactly as Eric mentioned.

Unfortunately, changing this to just "/sbin/chkconfig libvirt-guests" is not enough because this command tries to do the same as /sbin/runlevel (getting the current runlevel from /var/run/utmp which is empty), so this does not help.

However the service should be started not just when it is enabled in current runlevel but also only if libvirtd is already running. This functionality was probably added there just for the sake of upgrade from older libvirt versions that didn't have libvirt-client installed. That is in case of libvirt-client being installed while libvirtd is being updated and was running even before the update (thus running after the update as well). Otherwise everything is alright. So I see there are two options:
 - check if libvirtd is running and then start the service
 - completely remove the lines starting the service and relying on the user to start them in this particular situation

The latter make more sense because install scripts shouldn't be starting any service at all. What do you say, Eric?

The original goal is:

If this is a brand-new install of both libvirtd and libvirt-guests, and the user uses libvirt to create a new VM, all before rebooting, then we want that VM to be saved across the reboot.  Note that we turn on libvirtd by default on initial installation; therefore, we must also turn on libvirt-guests in the same conditions.  But since libvirtd (libvirt) and libvirt-guests (libvirt-client) are in different packages, and libvirt depends on libvirt-guests, we have an ordering dependency where the post-install for libvirt-guests will be run first, prior to libvirtd running.

For any other situation, we have nothing to do (if this is not the first install, then either libvirt-guests is already running or it was explicitly disabled).  Thinking about it more, I'm starting to think the real fix is to use the same logic as we use for turning on libvirtd only on the first install, where we check $1 to see if this is the initial install, rather than _always_ probing things even on upgrades.  Maybe something like:

%if %{with_systemd}
    # do we even have libvirt-guests working under systemd?
%else
/sbin/chkconfig --add libvirt-guests
if [ "$1" -ge "1" ]; then
        /sbin/service libvirt-guests condrestart > /dev/null 2>&1
fi
%endif

Check the specific script of libvirt-client-0.9.10-4.el6.x86_64, there's no action to start libvirt-guests after install the package, so this bug VERIFIED.
#rpm -q libvirt-client
libvirt-client-0.9.10-4.el6.x86_64

# rpm -q --scripts libvirt-client
postinstall scriptlet (using /bin/sh):

/sbin/ldconfig
/sbin/chkconfig --add libvirt-guests
preuninstall scriptlet (using /bin/sh):

if [ $1 = 0 ]; then
    /sbin/chkconfig --del libvirt-guests
    rm -f /var/lib/libvirt/libvirt-guests
fi
postuninstall program: /sbin/ldconfig

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
No Documentation needed

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2012-0748.html