Re: [Libguestfs] [PATCH] virt-v2v: Convert RedHat.pm to Linux.pm - for SUSE support

On Friday, October 04, 2013 09:38:58 AM Matthew Booth wrote: > 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. Ok. I've added a 'kernel' dependency to the virtio capability for SUSE, and this has resolved the above problem. It did bring up a versioning convention difference between RedHat and SUSE, but I've worked around the problem. (SUSE kernels include a build number that is not seen in the 'Linux kernel' string, or in the name of the kernel. However, this complete version string is required for kernel package installations. I now get this full version in _inspect_linux_kernel, and use it later in a couple of places.) Speaking of _inspect_linux_kernel, in my testing (under SUSE), $g->file('somefile') does not include the expected 'somefile:' at the beginning of the return string. Because of this, the following code isn't correct: if($filedesc =~ /^$path: Linux kernel .*\bversion\s+(\S+)\b/) { This works though: if($filedesc =~ /^Linux kernel .*\bversion\s+(\S+)\b/) { The above "Linux kernel" string doesn't match the SUSE version output. However, it's better if I don't modify the pattern to catch both version strings, as the next 'else' case will determine the correct version from the vmlinuz file. As the above pattern change doesn't really effect SUSE, I was hoping you could check the output under RedHat and let me know if you'd like '$path: ' removed from my version.

One other item I was hoping you could doublecheck is in the filtering out of xen/xenU in _install_capability. Shouldn't the xen/xenU portion of the release string actually be '-xen/xenU'? In other words, would the inclusion of the hypen in the following code also be applicable for RedHat? if (defined($kernel_release) && $kernel_release =~ /^(\S+?)(-xen)?(U)?$/) { $kernel_release = $1; }

> 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. Rather than doing that, I've added the -base kernel as an app dependency for the kernel. (It seems appropriate anyway.) At the end of _install_config, I catch this condition, and install the packages together. This approach shouldn't impact any non-SUSE environments.

> 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. Agreed, but I still think there could be environments where hdX devices exist (from the guest perspective) in a libata environment. If that ever is the case, I think any such devices should be included in the rename map. I've set all SUSE environments to $libata=0 for now, but I'm still doing research to see if there is ever a condition where we want $prefix set to 'sd' (which would mean that setting will need to be changed).