On 25/07/09 02:14, Guido Günther wrote:
Hi Matthew,
not that I'm qualified to comment on this but it seems this is very
rpm/RedHat specific. Would it make sense to abstrǎct the package format
from the very beginning by introducing some thin wrappers:
It's absolutely Red Hat specific. That code's from RedHat.pm ;)
One of the principal reasons the code is structured this way is to make
the Debian hooks easier to write and maintain. I think I mentioned in
one of my other emails that much of RedHat.pm is actually quite generic.
In writing a Debian module, I'd expect to abstract a significant amount
of code into a common module.
Unless you can convince me otherwise I'm inclined to keep the OS level
abstraction, rather than just package management. The reason for this is
that different distros using the same package manager can be quite
different. This is especially true for rpm. Abstracting common code
should achieve everything you're looking for.
On Fri, Jul 24, 2009 at 10:34:21PM +0100, Matthew Booth wrote:
[..snip..]
> +sub add_kernel
> +{
> + my $self = shift;
> + my $kernel_arch = "i386"; # XXX: Need to get this from inspection!
> +
> + my $g = $self->{g};
> +
> + my $filename = $self->match_file('kernel', $kernel_arch);
> +
> + # Inspect the rpm to work out what kernel version it contains
> + my $version;
> + foreach my $file ($g->command_lines(["rpm", "-qlp",
$filename])) {
list_pkg
> + if($file =~ m{^/boot/vmlinuz-(.*)$}) {
> + $version = $1;
> + last;
> + }
> + }
> +
> + die(__x"{filename} doesn't contain a valid kernel\n",
> + filename => $filename) if(!defined($version));
> +
> + $self->install_rpm($filename);
> +
> + # Make augeas reload so it'll find the new kernel
> + $g->aug_load();
> +
> + return $version;
> +}
> +
> +sub remove_kernel
> +{
> + my $self = shift;
> + my $version = shift;
> +
> + my $g = $self->{g};
> + eval {
> + # Work out which rpm contains the kernel
> + my $rpm = $g->command(["rpm", "-qf",
"/boot/vmlinuz-".$version]);
grep_pkg
> +
> + $g->command(["rpm", "-e", $rpm]);
> + };
> +
> + die($@) if($@);
> +}
> +
> +sub add_application
> +{
> + my $self = shift;
> + my $label = shift;
> + my $user_arch = "i386"; # XXX: Need to get this from inspection!
> +
> + my $filename = $self->match_file($label, $user_arch);
> + $self->install_rpm($filename);
install_pkg
> +}
> +
> +sub remove_application
> +{
> + my $self = shift;
> + my $name = shift;
> +
> + my $g = $self->{g};
> + eval {
> + $g->command(["rpm", "-e", $name]);
remove_pkg
We could then hook in deb based distros more easily by looking at
$desc->{package_format}.
Have a look at how GuestOS instantiates a specific instance. It iterates
through the available backends, calling can_handle($desc) on each one
until it finds one which reports yet. RedHat.pm is only looking for
'linux' as an OS and 'rpm' as a package format. Debian.pm shouldn't be
hard to write.
There should be nothing Red Hat specific outside RedHat.pm. I'd regard
that as a bug.
Let me know what you think,
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490