On 09/11/2015 12:28 PM, Charles Arnold wrote: >>>> On 9/11/2015 at 09:59 AM, Cole Robinson <crobinso@xxxxxxxxxx> 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@xxxxxxxx> >>> >> >> 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 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list