On Thu, 2013-10-03 at 11:48 -0600, Mike Latimer wrote:
Thanks for the comments, Matt.
On Thursday, October 03, 2013 05:34:27 PM Matthew Booth wrote:
> > - # There should be an installed virtio capable kernel if virtio was
> > installed - die("virtio configured, but no virtio kernel found")
> > + # Warn if there is no installed virtio capable kernel. It is safe to
> > + # continue and attempt to install a virtio kernel next.
> > + logmsg NOTICE, __x('virtio capable guest, but no virtio kernel
> > found.')>
> > if ($virtio && !defined($boot_kernel));
>
> No. This indicates an error in the program rather than something the
> user can fix. This is also why it's a plain die with no translation.
If a virtio kernel must be installed prior to _configure_kernel, why is there
the ability to install a replacement kernel immediately after this message? It
doesn't really matter to SUSE though - adding 'kernel' to the deps for virtio
in virt-v2v.db will trigger the installation of the correct kernel in the
install_capabilities step.
It's specifically an error if we're attempting to configure virtio, and
there's no detected virtio kernel. It shouldn't have been possible to
get here in that state, hence it's a programmer error. The code below
attempts to install *any* kernel in the case that we aren't configuring
virtio.
> > @@ -1631,14 +1752,38 @@ sub _install_any
> >
> > # If we're installing a kernel, check which kernels are there first
> > my @k_before = $g->glob_expand('/boot/vmlinuz-*') if
> > defined($kernel);
> >
> > + # If a SLES11 kernel is being added, add -base kernel
> > + if (defined($kernel)) {
> > + if (_is_suse_family($g, $root) &&
> > + ($g->inspect_get_major_version($root) == 11)) {
> > + my $tmp;
> > + push(@$tmp, $kernel);
> > + $tmp->[1][0] = $tmp->[0][0].'-base';
> > + for my $count (1..4) {
> > + if (defined($tmp->[0][$count])) {
> > + $tmp->[1][$count] = $tmp->[0][$count];
> > + }
> > + }
> > + $kernel = $tmp;
> > + }
> > + }
>
> For sanity, I think we need $kernel to be the same data structure in
> all cases. Lets either unconditionally convert $kernel into a list (of
> lists), or see of -base can be added to the @install list or something
> like that. Bear in mind, though, that in the former case you'd also
> have to update _install_yum and _install_up2date.
The problem with @install is that it is processed separately from $kernel in
_install_config. This causes the two packages to be installed by separate rpm
commands, and the dependencies cannot be satisfied. This is why I had to add
the -base package to $kernel somehow... (Note - This doesn't effect zypper
installs at all as it will satisfy dependencies automatically.)
I'll think about it some more...
You can still make $kernel a list, you just need to ensure it's always a
list. The updates to the other installation methods should be trivial.
> > @@ -2235,6 +2541,10 @@ sub _remap_block_devices
...
> I've had a change to think about this change properly, and I'm pretty
> sure it's not required. The reason is that the only place %idemap is
> modified is in the section guarded:
>
> if ($libata) {
> ...
> }
>
> If the guest is using libata it will not have any references to
> /dev/hdX devices, as IDE devices are presented as SCSI devices. That's
> a NAK to this bit of the patch. Instead, you need to work out when SLES
> switched to libata and set $libata accordingly above.
If there are never any hdX devices in libata machines, why are they renamed in
the first place? The code I proposed just tracks any such changes and adds them
to the list of remaps to perform. (i.e. if hda --> sda, and sda --> vda, then
hda --> vda as well.)
This is because there are 2 names here: the 'libvirt' name, which calls
ide devices hdX and is independent of the guest OS because it's at the
level of the hypervisor, and the name used by the guest os, which may
will be different if the guest uses libata.
The only direct information we have about the guest comes from the
hypervisor, so the only names we have to start off with are the names
the hypervisor uses. Everything else comes from subsequent inspection.
If the guest uses something different we need to handle that, hence this
whole complicated and fragile dance.
However, SLES seems to be similar to RHEL5, where libata is in use,
but udev
calls them hdX anyway (for hvm guests). I can add SLES to the list of
$libata=0 environments, but wasn't sure if that was the best long term
approach.
Looks good to me.
Matt