[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] Use RHN to retrieve replacement packages



On Friday 14 May 2010 17:43:34 Matthew Booth wrote:
> Milan,
> 
> Some comments inline.
> 
> On 14/05/10 15:06, Milan Zazrivec wrote:
> > For guests registered with Red Hat Network, this patch allows for
> > the conversion process to use RHN to download appropriate kernel
> > package and its dependencies.
> >
> > We use yum on RHEL-5, up2date libraries on RHEL-4.
> >
> > We install matching kernel version whenever possible, latest available
> > kernel version otherwise.
> >
> > _discover_kernel routine has been extended to return version-release
> > of the default kernel found.
> > ---
> >  lib/Sys/VirtV2V/GuestOS/RedHat.pm |  166
> > ++++++++++++++++++++++++++++-------- 1 files changed, 129 insertions(+),
> > 37 deletions(-)
> >
> > diff --git a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
> > b/lib/Sys/VirtV2V/GuestOS/RedHat.pm index 77f0f3a..38a485c 100644
> > --- a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
> > +++ b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
> > @@ -475,7 +475,7 @@ sub add_kernel
> >  {
> >      my $self = shift;
> >
> > -    my ($kernel_pkg, $kernel_arch) = $self->_discover_kernel();
> > +    my ($kernel_pkg, $kernel_ver, $kernel_arch) =
> > $self->_discover_kernel();
> >
> >      # If the guest is using a Xen PV kernel, choose an appropriate
> > normal kernel # replacement
> > @@ -547,56 +547,142 @@ sub add_kernel
> >          }
> >      }
> >
> > -    my ($app, $depnames);
> > -    eval {
> > +    my $version;
> > +
> > +    # systemid exists, assume the system is registered w/ RHN
> > +    if ($self->{g}->exists('/etc/sysconfig/rhn/systemid')) {
> 
> Can we make this less of an explicit check for RHN registration that a
> check for the availability of an update mechanism? I was thinking you'd
> check to see if up2date/yum was installed, and if it was, try to run it.
> If running it failed, fall back to the config.

This could be done I think.

> >          my $desc = $self->{desc};
> >
> > -        ($app, $depnames) =
> > -            $self->{config}->match_app($desc, $kernel_pkg,
> > $kernel_arch); -    };
> > -    # Return undef if we didn't find a kernel
> > -    if ($@) {
> > -        print STDERR $@;
> > -        return undef;
> > -    }
> > +        my ($min_virtio_ver, @kern_vr, @preinst_cmd, @inst_cmd,
> > $inst_fmt);
> >
> > -    my @missing;
> > -    if (!$self->{g}->exists($self->_transfer_path($app))) {
> > -        push(@missing, $app);
> > +        # filter out xen/xenU from release field
> > +        if ($kernel_ver =~ /^(\S+)-(\S+?)(xen)?(U)?$/) {
> > +            @kern_vr = ($1, $2);
> > +            $kernel_ver = join('-', @kern_vr);
> > +        }
> > +
> > +        # RHEL-5
> > +        if ($desc->{distro} eq 'rhel' && $desc->{major_version} eq '5')
> > {
> 
> Again, with these tests, could we just check for the existence of
> /usr/bin/yum? I guess we'd need to use up2date in preference to yum if
> it was installed, as the most likely scenario here is RHEL 3/4 using yum
> for non-core repos.

Yes, sounds reasonable.

> > +            if (_rpmvercmp($kernel_ver, '2.6.18-128.el5') >= 0) {
> > +                # Install matching kernel version
> > +                @inst_cmd = ('/usr/bin/yum', '-y', 'install',
> > +                             "$kernel_pkg-$kernel_ver");
> > +            } else {
> > +                # Install latest available kernel version
> > +                @inst_cmd = ('/usr/bin/yum', '-y', 'install',
> > $kernel_pkg); +
> > +                # We need to upgrade kernel deps. first to avoid
> > possible conflicts
> 
> Why can't yum do it all in one go?

yum can do it all in one go as long as you don't have both i386 & x86_64
ecryptfs-utils installed (standard rhel-5 installation on x86_64 for example).

You run 'yum install kernel' and it will correctly try to upgrade
ecryptfs-utils.x86_64, but the new kernel will still conflict with
ecryptfs-utils.i386 and the command fails.

> > +                my @deps;
> > +                my $deps = ($self->{config}->match_app($desc,
> > $kernel_pkg, $kernel_arch))[1]; +
> > +                if ($deps) {
> > +                    @preinst_cmd = (('/usr/bin/yum', '-y', 'upgrade'),
> > @$deps); +                }
> > +            }
> > +        }
> > +        # RHEL-4
> > +        elsif ($desc->{distro} eq 'rhel' && $desc->{major_version} eq
> > '4') {
> 
> Using RHEL version to distinguish between the methods below is ok, I
> guess, as nothing except RHEL used up2date (as far as I'm aware).
> 
> > +            if (_rpmvercmp($kernel_ver, '2.6.9-89.EL') >= 0) {
> > +                # Install matching kernel version
> > +                @inst_cmd = ('/usr/bin/python', '-c',
> > +                    "import sys; sys.path.append('/usr/share/rhn');" .
> > +                    "import actions.packages;" .
> > +                    "actions.packages.cfg['forceInstall'] = 1;" .
> > +                    "actions.packages.update([['$kernel_pkg', " .
> > +                    "'$kern_vr[0]', '$kern_vr[1]', '']])");
> 
> Yuk, but ok ;)
> 
> > +            } else {
> > +                # Install latest available kernel version
> > +                @inst_cmd = ('/usr/sbin/up2date', '-fi', $kernel_pkg);
> > +
> > +                # We need to upgrade kernel deps. first to avoid
> > possible conflicts +                my $deps =
> > ($self->{config}->match_app($desc, $kernel_pkg, $kernel_arch))[1]; +
> > +                if ($deps) {
> > +                    @preinst_cmd = (('/usr/sbin/up2date', '-fu'),
> > @$deps); +                }
> > +            }
> > +        }
> > +        else {
> > +            @inst_cmd = ('/usr/sbin/up2date', '-fi', $kernel_pkg);
> > +        }
> > +
> > +        my (@k_before, @k_new);
> > +
> > +        # List of kernels before the new kernel installation
> > +        @k_before = $self->{g}->glob_expand('/boot/vmlinuz-*');
> > +
> > +        eval {
> > +            # Upgrade dependencies if needed
> > +            if (@preinst_cmd) {
> > +                $self->{g}->command(\ preinst_cmd);
> > +            }
> > +            # Install new kernel
> > +            $self->{g}->command(\ inst_cmd);
> 
> Again, why can't we do these in a single transaction?

up2date is not that flexible (as far as I know).

Anyway, we're looking at rhel-4 world, where we don't have to worry
about the problems with new kernel dependencies.

I'd vote for removing the up2date -fu part completely.

> > +        };
> > +
> > +        $self->_augeas_error($@) if ($@);
> 
> Just ditch the eval {} block here. I recently ditched these elsewhere in
> the code.

Okay.

> > +
> > +        # Figure out which kernel has just been installed
> > +        foreach my $k ($self->{g}->glob_expand('/boot/vmlinuz-*')) {
> > +            grep(/^$k$/, @k_before) or push(@k_new, $k);
> > +        }
> > +
> > +        # version-release of the new kernel package
> > +        $version = ($self->{g}->command_lines(
> > +            ['rpm', '-qf', '--qf=%{VERSION}-%{RELEASE}',
> > $k_new[0]]))[0];
> 
> As above, can we arrange this block to fall-through if the execution of
> up2date/yum failed for some reason?

Okay.

> >      } else {
> > -        return undef if ($self->_newer_installed($app));
> > -    }
> >
> > -    my $user_arch = $kernel_arch eq 'i686' ? 'i386' : $kernel_arch;
> > +        my ($app, $depnames);
> > +        eval {
> > +            my $desc = $self->{desc};
> >
> > -    my @deps = $self->_get_deppaths(\ missing, $user_arch, @$depnames);
> > +            ($app, $depnames) =
> > +                $self->{config}->match_app($desc, $kernel_pkg,
> > $kernel_arch); +        };
> > +        # Return undef if we didn't find a kernel
> > +        if ($@) {
> > +            print STDERR $@;
> > +            return undef;
> > +        }
> >
> > -    # We can't proceed if there are any files missing
> > -    _die_missing(@missing) if (@missing > 0);
> > +        my @missing;
> > +        if (!$self->{g}->exists($self->_transfer_path($app))) {
> > +            push(@missing, $app);
> > +        } else {
> > +            return undef if ($self->_newer_installed($app));
> > +        }
> >
> > -    # Install any required kernel dependencies
> > -    $self->_install_rpms(1, @deps);
> > +        my $user_arch = $kernel_arch eq 'i686' ? 'i386' : $kernel_arch;
> >
> > -    # Inspect the rpm to work out what kernel version it contains
> > -    my $version;
> > -    my $g = $self->{g};
> > -    foreach my $file ($g->command_lines
> > -        (["rpm", "-qlp", $self->_transfer_path($app)]))
> > -    {
> > -        if($file =~ m{^/boot/vmlinuz-(.*)$}) {
> > -            $version = $1;
> > -            last;
> > +        my @deps = $self->_get_deppaths(\ missing, $user_arch,
> > @$depnames); +
> > +        # We can't proceed if there are any files missing
> > +        _die_missing(@missing) if (@missing > 0);
> > +
> > +        # Install any required kernel dependencies
> > +        $self->_install_rpms(1, @deps);
> > +
> > +        # Inspect the rpm to work out what kernel version it contains
> > +        my $g = $self->{g};
> > +        foreach my $file ($g->command_lines
> > +            (["rpm", "-qlp", $self->_transfer_path($app)]))
> > +        {
> > +            if($file =~ m{^/boot/vmlinuz-(.*)$}) {
> > +                $version = $1;
> > +                last;
> > +            }
> >          }
> > -    }
> >
> > -    die(user_message(__x("{path} doesn't contain a valid kernel",
> > -                         path => $app))) if(!defined($version));
> > +        die(user_message(__x("{path} doesn't contain a valid kernel",
> > +                             path => $app))) if(!defined($version));
> >
> > -    $self->_install_rpms(0, ($app));
> > +        $self->_install_rpms(0, ($app));
> > +
> > +    }
> >
> >      # Make augeas reload so it'll find the new kernel
> >      eval {
> > -        $g->aug_load();
> > +        $self->{g}->aug_load();
> 
> Why?

Because my $g = $self->{g}; is no longer valid here, but this could
be handled more nicely, I agree.

-Milan Z√°zrivec

> >      };
> >
> >      $self->_augeas_error($@) if ($@);
> > @@ -632,6 +718,7 @@ sub _discover_kernel
> >
> >      # Get a current bootable kernel, preferrably the default
> >      my $kernel_pkg;
> > +    my $kernel_ver;
> >      my $kernel_arch;
> >
> >      foreach my $i (@configs) {
> > @@ -647,6 +734,11 @@ sub _discover_kernel
> >
> >          # Get the kernel package name
> >          $kernel_pkg = $kernel->{package};
> > +
> > +        # Get the kernel package version
> > +        $kernel_ver = $kernel->{version};
> > +
> > +        last;
> >      }
> >
> >      # Default to 'kernel' if package name wasn't discovered
> > @@ -664,7 +756,7 @@ sub _discover_kernel
> >      # a very long time.
> >      $kernel_arch = 'i686' if('i386' eq $kernel_arch);
> >
> > -    return ($kernel_pkg, $kernel_arch);
> > +    return ($kernel_pkg, $kernel_ver, $kernel_arch);
> >  }
> >
> >  =item remove_kernel(version)
> > -- 1.7.1
> 
> Thanks,
> 
> Matt
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]