On Wed, Dec 03, 2008 at 08:31:28PM -0800, john.levon@xxxxxxx wrote:
> # HG changeset patch
> # User john.levon@xxxxxxx
> # Date 1228365031 28800
> # Node ID 35baacfe79834400949ebdba69f952ed873ae442
> # Parent 7dd32b4d7915937025f42c3519711db50e5a7ce3
> Add support for Solaris PV
>
> Solaris PV comes in two flavours: Nevada and OpenSolaris. In order to
> correctly build network installs for Nevada, we need to pass down guest
> options into OSDistro.
>
> os_type was horribly overloaded between the Installer and Guest classes,
> meaning two different things ('solaris' or 'hvm'). Clean this up by
> naming the latter 'virt_type'.
The new Installer class for Solaris looks fine to me, but I don't really
like the renaming of os_type to virt_type, since its conflicting naming
to the libvirt usage.
'virt type' is usually used to refer to hypervisor type, eg this bit
of the XML
<domain type='xen'>
<domain type='kvm'>
<domain type='qemu'>
While 'os type' is used to refer to the Operating System guest ABI
<os>
<type>linux</type> (badly named, should really be 'xen' but
this is more compatible with older libvirt)
<type>hvm</type> (native fully virt)
<type>uml</type> (User mode linux)
Unfortunaltey the virt-install CLI flag is --os-type - should really
have been --os-distro-type. So I think I'd rather we changed the
insternal variables for this to be os_distro_type and os_distro_variant
> @@ -239,26 +249,29 @@ class DistroInstaller(Guest.Installer):
> arch = os.uname()[4]
> if hasattr(guest, "arch"):
> arch = guest.arch
> - (kernelfn, initrdfn, args) = acquireKernel(self.location,
> - meter,
> - scratchdir = self.scratchdir,
> - type = self.os_type,
> - distro = distro,
> - arch=arch)
> +
> + (kernelfn, initrdfn, args), os_type = \
> + self._acquire_kernel(guest, meter, distro = distro, arch = arch)
> +
> + guest.os_type = os_type
> self.install["kernel"] = kernelfn
> self.install["initrd"] = initrdfn
> - if not self.extraargs is None:
> - self.install["extraargs"] = self.extraargs + " " + args
> - else:
> - self.install["extraargs"] = args
> + self.install["extraargs"] = args
This chunk looks a little questionable to me - IIRC this is reverting a fix
we previously made, so perhaps this is just a merge error.
Baiscally the existing "extraargs" data will contain params from --extra-args
command line flag, and the 'self.extraargs' needs to append more data provided
by the DistroInstaller class, such as kickstart URL. This change means we loose
the former.
> @@ -172,18 +174,21 @@ class Distro:
>
> return False
>
> - def _kernelFetchHelper(self, fetcher, progresscb, kernelpath, initrdpath):
> + def _kernelFetchHelper(self, fetcher, guest, progresscb, kernelpath, initrdpath):
> # Simple helper for fetching kernel + initrd and performing
> # cleanup if neccessary
> kernel = fetcher.acquireFile(kernelpath, progresscb)
> + args = ''
> +
> + if not fetcher.location.startswith("/"):
> + args += "method=" + fetcher.location
> +
> + if guest.extraargs:
> + args += guest.extraargs
> +
> try:
> initrd = fetcher.acquireFile(initrdpath, progresscb)
> - if fetcher.location.startswith("/"):
> - # Local host path, so can't pass a location to guest
> - #for install method
> - return (kernel, initrd, "")
> - else:
> - return (kernel, initrd, "method=" + fetcher.location)
> + return kernel, initrd, args
Hmm, this existing code was dubious even before this change. method= is
the Fedora / RHEL anaconda syntax for specifying install location. It
should never havebeen in Distro class in the first place - instead it
belongs in the RedHatDistro subclass.
> @@ -692,3 +707,194 @@ class MandrivaDistro(Distro):
>
> return False
>
> +# Solaris and OpenSolaris distros
> +class SunDistro(Distro):
> +
> + name = "Solaris"
> + os_type = "solaris"
> +
> + def isValidStore(self, fetcher, progresscb):
> + """Determine if uri points to a tree of the store's distro"""
> + raise NotImplementedError
> +
> + def acquireBootDisk(self, fetcher, progresscb):
> + return fetcher.acquireFile("images/solarisdvd.iso", progresscb)
> +
> + def process_extra_args(self, argstr):
> + """Collect additional arguments."""
> + if not argstr:
> + return (None, None, None, None)
> +
> + kopts = ''
> + kargs = ''
> + smfargs = ''
> + Bargs = ''
> +
> + args = argstr.split()
> + i = 0
> + while i < len(args):
> + exarg = args[i]
> + if exarg == '-B':
> + i += 1
> + if i == len(args):
> + continue
> +
> + if not Bargs:
> + Bargs = args[i]
> + else:
> + Bargs = ','.join([Bargs, args[i]])
> +
> + elif exarg == '-m':
> + i += 1
> + if i == len(args):
> + continue
> + smfargs = args[i]
> + elif exarg.startswith('-'):
> + if kopts is None:
> + kopts = exarg[1:]
> + else:
> + kopts = kopts + exarg[1:]
> + else:
> + if kargs is None:
> + kargs = exarg
> + else:
> + kargs = kargs + ' ' + exarg
> + i += 1
> +
> + return kopts, kargs, smfargs, Bargs
> +
> +class SolarisDistro(SunDistro):
> + kernelpath = "boot/platform/i86xpv/kernel/unix"
> + initrdpath = "boot/x86.miniroot"
> +
> + def isValidStore(self, fetcher, progresscb):
> + if fetcher.hasFile(self.kernelpath):
> + logging.debug("Detected Solaris")
> + return True
> + return False
> +
> + def install_args(self, guest):
> + """Construct kernel cmdline args for the installer, consisting of:
> + the pathname of the kernel (32/64) to load, kernel options
> + and args, and '-B' boot properties."""
> +
> + # XXX: ignoring smfargs for the time being
> + (kopts, kargs, smfargs, kbargs) = \
> + self.process_extra_args(guest.extraargs)
> +
> + bargs = ''
> + if kopts:
> + bargs += '-' + kopts + ' -'
> +
> + if guest.graphics['enabled'] == False:
> + bargs += ' nowin'
> +
> + if guest.autocf:
> + bargs += ' install'
> +
> + if kargs:
> + bargs += ' ' + kargs
> +
> + if guest.location.startswith('nfs:'):
> + try:
> + guestIP = socket.gethostbyaddr(guest.name)[2][0]
> + except:
> + bargs += ' dhcp'
> + else:
> + iserver = guest.location.split(':')[1]
> + ipath = guest.location.split(':')[2]
> + iserverIP = socket.gethostbyaddr(iserver)[2][0]
> + bargs += " -B install_media=" + iserverIP + ":" + ipath
> + bargs += ",host-ip=" + guestIP
> + droute = util.default_route(guest.nics[0].bridge)
> + if droute is not None:
> + bargs += ",router-ip=" + droute
> + if guest.nics[0].macaddr is not None:
> + en = guest.nics[0].macaddr.split(':')
> + for i in range(len(en)):
> + # remove leading '0' from mac address element
> + if len(en[i]) > 1 and en[i][0] == '0':
> + en[i] = en[i][1]
> + boot_mac = ":".join(en)
> + bargs += ",boot-mac=" + boot_mac
> + if guest.autocf:
> + acf_host = guest.autocf.split(":")[1]
> + acf_path = guest.autocf.split(":")[2]
> + try:
> + acf_hostip = socket.gethostbyaddr(acf_host)[2][0]
> + bargs += ",sysid_config=" + acf_hostip + ":" + acf_path
> + bargs += ",install_config=" + acf_hostip + ":" + acf_path
> + except:
> + print "failed to lookup host %s" % acf_host
> + else:
> + bargs += " -B install_media=cdrom"
> +
> + if kbargs:
> + bargs += "," + kbargs
> +
> + return bargs
> +
> + def acquireKernel(self, guest, fetcher, progresscb):
> +
> + try:
> + kernel = fetcher.acquireFile(self.kernelpath, progresscb)
> + except:
> + raise RuntimeError("Solaris PV kernel not found at %s" %
> + self.kernelpath)
> +
> + # strip boot from the kernel path
> + kpath = self.kernelpath.split('/')[1:]
> + args = "/" + "/".join(kpath) + self.install_args(guest)
> +
> + try:
> + initrd = fetcher.acquireFile(self.initrdpath, progresscb)
> + return (kernel, initrd, args)
> + except:
> + os.unlink(kernel)
> + raise RuntimeError(_("Solaris miniroot not found at %s") %
> + self.initrdpath)
> +
> +class OpenSolarisDistro(SunDistro):
> + kernelpath = "platform/i86xpv/kernel/unix"
> + initrdpath = "boot/x86.microroot"
> +
> + def isValidStore(self, fetcher, progresscb):
> + if fetcher.hasFile(self.kernelpath):
> + logging.debug("Detected OpenSolaris")
> + return True
> + return False
> +
> + def install_args(self, guest):
> + """Construct kernel cmdline args for the installer, consisting of:
> + the pathname of the kernel (32/64) to load, kernel options
> + and args, and '-B' boot properties."""
> +
> + # XXX: ignoring smfargs and kargs for the time being
> + (kopts, kargs, smfargs, kbargs) = \
> + self.process_extra_args(guest.extraargs)
> +
> + bargs = ''
> + if kopts:
> + bargs += ' -' + kopts
> + if kbargs:
> + bargs += ' -B ' + kbargs
> +
> + return bargs
> +
> + def acquireKernel(self, guest, fetcher, progresscb):
> +
> + try:
> + kernel = fetcher.acquireFile(self.kernelpath, progresscb)
> + except:
> + raise RuntimeError(_("OpenSolaris PV kernel not found at %s") %
> + self.kernelpath)
> +
> + args = "/" + self.kernelpath + self.install_args(guest)
> +
> + try:
> + initrd = fetcher.acquireFile(self.initrdpath, progresscb)
> + return (kernel, initrd, args)
> + except:
> + os.unlink(kernel)
> + raise RuntimeError(_("OpenSolaris microroot not found at %s") %
> + self.initrdpath)
These new classes seeem to be more or less independant of the bit
os_type/virt_type renaming (baring the obvious fact that it uses the
new names). I've no problem add this bit right away.
> diff --git a/virtinst/VirtualDisk.py b/virtinst/VirtualDisk.py
> --- a/virtinst/VirtualDisk.py
> +++ b/virtinst/VirtualDisk.py
> @@ -519,6 +519,9 @@ class VirtualDisk(VirtualDevice):
>
> if self.target:
> disknode = self.target
> + if self.device == VirtualDisk.DEVICE_CDROM and disknode.startswith('xvd'):
> + disknode = disknode + ':cdrom'
> +
This seems wrong - you should not pass "xvda:cdrom" into the libvirt
XML for a disk. There is an explicit attribute for specifying CDROM
media
<disk type='file' device='cdrom'>
<source file='/some/path'/>
<target dev='xvda'/>
</disk>
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
