[virt-tools-list] [PATCH virt-manager]: virtinst: correctly find the sles s390x kernel and initrd

[Cole Robinson]

On 09/11/2015 12:28 PM, Charles Arnold wrote:
>>>> On 9/11/2015 at 09:59 AM, Cole Robinson <crobinso at redhat.com> wrote: 
>> On 09/11/2015 11:41 AM, Charles Arnold wrote:
>>> Allow virt-install to correctly find the SLES kernel/initrd on the media
>>> when installing an s390x VM.
>>>
>>> Signed-off-by: Charles Arnold <carnold at suse.com>
>>>
>>
>> Thanks for the patch. Is there a public URL/media to test this against?
> 
> I don't think so or at least I'm not aware of a public URL.  s390 being the big
> expensive piece of hardware that it is I doubt anyone makes it public.
> 
>>
>> There's some minor pep8 errors:
>>
>> virtinst/urlfetcher.py:940: [E201] whitespace after '['
>> virtinst/urlfetcher.py:941: [E202] whitespace before ']'
>> virtinst/urlfetcher.py:986: [E201] whitespace after '['
>> virtinst/urlfetcher.py:987: [E202] whitespace before ']'
> 
> I'll clean these up and resubmit unless you have more comments
> for my comments below :)
> 
>>
>> One comment below:
>>
>>> diff --git a/virtinst/urlfetcher.py b/virtinst/urlfetcher.py
>>> index c48e9d5..3e62c2b 100644
>>> --- a/virtinst/urlfetcher.py
>>> +++ b/virtinst/urlfetcher.py
>>> @@ -369,6 +369,8 @@ def _distroFromSUSEContent(fetcher, arch, vmtype=None):
>>>              arch = "x86_64"
>>>          elif cbuf.find("i586") != -1:
>>>              arch = "i586"
>>> +        elif cbuf.find("s390x") != -1:
>>> +            arch = "s390x"
>>>  
>>>      dclass = GenericDistro
>>>      if distribution:
>>> @@ -934,16 +936,22 @@ class SuseDistro(Distro):
>>>              oldkern += "64"
>>>              oldinit += "64"
>>>  
>>> -        # Tested with Opensuse >= 10.2, 11, and sles 10
>>> -        self._hvm_kernel_paths = [("boot/%s/loader/linux" % self.arch,
>>> -                                    "boot/%s/loader/initrd" % self.arch)]
>>> -        # Tested with Opensuse 10.0
>>> -        self._hvm_kernel_paths.append(("boot/loader/%s" % oldkern,
>>> -                                       "boot/loader/%s" % oldinit))
>>> +        if self.arch == "s390x":
>>> +            self._hvm_kernel_paths = [ ("boot/%s/linux" % self.arch,
>>> +                                        "boot/%s/initrd" % self.arch) ]
>>> +            # No Xen on s390x
>>> +            self._xen_kernel_paths = []
>>> +        else:
>>> +            # Tested with Opensuse >= 10.2, 11, and sles 10
>>> +            self._hvm_kernel_paths = [("boot/%s/loader/linux" % self.arch,
>>> +                                        "boot/%s/loader/initrd" % 
>> self.arch)]
>>> +            # Tested with Opensuse 10.0
>>> +            self._hvm_kernel_paths.append(("boot/loader/%s" % oldkern,
>>> +                                           "boot/loader/%s" % oldinit))
>>>  
>>> -        # Matches Opensuse > 10.2 and sles 10
>>> -        self._xen_kernel_paths = [("boot/%s/vmlinuz-xen" % self.arch,
>>> -                                    "boot/%s/initrd-xen" % self.arch)]
>>> +            # Matches Opensuse > 10.2 and sles 10
>>> +            self._xen_kernel_paths = [("boot/%s/vmlinuz-xen" % self.arch,
>>> +                                        "boot/%s/initrd-xen" % self.arch)]
>>>  
>>>      def _variantFromVersion(self):
>>>          distro_version = self.version_from_content[1].strip()
>>> @@ -971,6 +979,13 @@ class SuseDistro(Distro):
>>>          self._variantFromVersion()
>>>  
>>>          self.os_variant = self._detect_osdict_from_url()
>>> +
>>> +        # Reset kernel name for sle11 source on s390x
>>> +        if self.arch == "s390x":
>>> +            if self.os_variant == "sles11" or self.os_variant == "sled11":
>>> +                self._hvm_kernel_paths = [ ("boot/%s/vmrdr.ikr" % 
>> self.arch,
>>> +                                            "boot/%s/initrd" % self.arch) ]
>>> +
>>
>> From my reading of _detect_osdict_from_url it will only return opensuse* OS
>> names, not sles* or sled*, so I'm not sure how this path is triggered with
>> virt-manager.git
> 
> Not exactly. It will only return an 'opensuse' variant if the url has an opensuse in
> the path which it won't find for sles and so instead returns what is already in
> self.os_variant. In this case, self.os_variant was previously set to some sles version
> in the call to self._variantFromVersion.
> 

Whoops yeah I missed that. I fixed the pep8 errors and pushed your patch now,
no need to resend

Thanks,
Cole