This approach looks great. I think there are a couple of bits to look at (see below), but probably not much. Matt On Wed, 2013-10-02 at 23:46 -0600, Mike Latimer wrote:
This is a proposed patch which changes the RedHat.pm converter to Linux.pm, and adds support for SUSE guest conversion. This is first approach recommended by Matt Booth in:
https://www.redhat.com/archives/libguestfs/2013-September/msg00076.html
Some aspects of this patch still need additional testing, and a couple of changes are not foolproof (such as the lack of grub2 support in the menu.lst changes in _remap_block_devices). However, it is complete enough that I'd like some feedback before continuing.
Note - This patch alone is not enough to support SUSE guests, as changes to virt-v2v.db are also required. After these changes are flushed out, I'll post all the patches in one thread.
-Mike
N.B. I've cut the original below and replaced it with the output of git show, which deals with the rename sensibly. The patch is identical, though.
diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/Linux.pm similarity index 79% rename from lib/Sys/VirtConvert/Converter/RedHat.pm rename to lib/Sys/VirtConvert/Converter/Linux.pm index 612ab2e..f3d13df 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/Linux.pm
@@ -500,7 +584,7 @@ sub convert_efi eval { $g->aug_save(); }; augeas_error($g, $@) if $@;
- # Install grub2 in the BIOS boot partition. This overwrites the previous + # Install grub2 in the BIOS beot partition. This overwrites the previous
Typo
# contents of the EFI boot partition. $g->command(['grub2-install', $device]);
@@ -1002,8 +1101,9 @@ sub _configure_kernel last; }
- # 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 none of the installed kernels are appropriate, install a new one
@@ -1055,7 +1155,7 @@ sub _configure_kernel unless defined($boot_kernel); } else { v2vdie __x('Failed to find a {name} package to install', - name => "kernel_pkg.$kernel_arch"); + name => "$kernel_pkg.$kernel_arch");
Thanks :)
@@ -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. (I haven't bothered commenting on all the hunks this change would also affect below.)
@@ -2235,6 +2541,10 @@ sub _remap_block_devices # Fedora has used libata since FC7, which is long out of support. We assume # that all Fedora distributions in use use libata.
+ # Create a hash to track any hd->sd conversions, as any references to + # hd devices (in fstab, menu.lst, etc) will need to be converted later. + my %idemap; + if ($libata) { # If there are any IDE devices, the guest will have named these sdX # after any SCSI devices. i.e. If we have disks hda, hdb, sda and sdb, @@ -2247,10 +2557,15 @@ sub _remap_block_devices # this is a weird and somewhat unlikely situation I'm going with SCSI # first until we have a more comprehensive solution.
+ my $idedev; my @newdevices; my $suffix = 'a'; foreach my $device (@devices) { - $device = 'sd'.$suffix++ if ($device =~ /(?:h|s)d[a-z]+/); + if ($device =~ /(?:h|s)d[a-z]+/) { + $idedev = $device; + $device = 'sd'.$suffix++; + $idemap{$device} = $idedev if ($device ne $idedev); + } push(@newdevices, $device); } @devices = @newdevices; @@ -2286,12 +2601,21 @@ sub _remap_block_devices $map{'xvd'.$1} = $mapped; } $map{$device} = $mapped; + # Also add a mapping for previously saved hd->sd conversions + if (defined($idemap{$device})) { + my $idedevice; + $idedevice = $idemap{$device}; + $map{$idedevice} = $mapped; + } +
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.