On Thu, Nov 15, 2007 at 11:49:17AM -0800, Robert Nelson wrote:
Hugh O. Brock wrote:
On Wed, Nov 14, 2007 at 11:50:47PM -0800, Robert Nelson wrote:
I've been trying to add support for additional para-virtual operating
systems (OpenSolaris and Debian) to virt-manager. There is no problem
getting the initial kernel and ramdisk, however there is no way to
specify the target disk node based on the OS or distro. Also some OS's
don't support the virtual frame buffer driver but there is no way to
prevent the XML for it being created.
I can't see how this could be done without doing some major
restructuring of the code and creating a new class to encapsulate all
the target specific information. Is there any plan on doing something
like this? If someone were to do it, are the changes likely to be
incorporated?
Hello Robert, thanks for your interest.
I'm not sure I understand what you mean by "specify the target disk
node based on the OS or distro"?
Virt-Manager names the target disknodes xvda, xvdb, etc. OpenSolaris
requires 0, 1, 2, etc.
I would have no problem with adding a checkbox to virt-manager that
would have the effect of passing the --nographics flag to virtinst if
you can determine a good place to do it. I don't think we want to
spend a whole ton of time creating a new class to take different PV
guest OSes into account, though. Having said that, have you looked at
the *Installer class family in virtinst? Is there a way we could
extend that to do what you need?
I'll take another look at the Installer class and see if it can be done
there.
In the virtinst/FullVirtGuest.py class, there is already a bunch of
OS specific metadata, eg what of mouse to use, apic/acpi/pae settings,
whether the installer is multi-stage reboots (eg Windows). I'd recommend
moving this metadata into virtinst/DistroManager and have a bunch of
methods in that module for querying distro specific metadata, from the
Installer class.
Dan.
I spent some time reworking the virtinst code to support OpenSolaris
and make it much easier to support additional OSes.* I've attached the
patch file to get some feedback on the work so far.
Unfortunately a bunch for the code from virtinst is duplicated in
virt-manager in the Add Device code.* This means either moving it back
to virtinst with the appropriate additional APIs or duplicating work in
virt-manager.
def get_location(self):
return self._location
+
def set_location(self, val):
if not (val.startswith("http://") or val.startswith("ftp://") or
val.startswith("nfs:") or val.startswith("/")):
@@ -612,18 +40,22 @@
if os.geteuid() != 0 and val.startswith("nfs:"):
raise ValueError(_("NFS installations are only supported as root"))
self._location = val
+
location = property(get_location, set_location)
- def _prepare_cdrom(self, guest, distro, meter):
+ def cleanup(self):
+ for f in self._tmpfiles:
+ logging.debug("Removing " + f)
+ self._distro.releaseFile(f)
+ self._tmpfiles = []
+
+ def _prepare_cdrom(self, guest, meter):
if self.location.startswith("/") and os.path.exists(self.location):
# Huzzah, a local file/device
cdrom = self.location
else:
# Xen needs a boot.iso if its a http://, ftp://, or nfs:/ url
- cdrom = acquireBootDisk(self.location,
- meter,
- scratchdir = self.scratchdir,
- distro = distro)
+ cdrom = self._distro.acquireBootDisk(meter)
self._tmpfiles.append(cdrom)
- def _prepare_kernel_and_initrd(self, guest, distro, meter):
+ def _prepare_kernel_and_initrd(self, guest, meter):
if self.boot is not None:
# Got a local kernel/initrd already
- self.install["kernel"] = self.boot["kernel"]
- self.install["initrd"] = self.boot["initrd"]
- if not self.extraargs is None:
- self.install["extraargs"] = self.extraargs
+ kernelfn = self.boot["kernel"]
+ initrdfn = self.boot["initrd"]
+ args = None
else:
- ostype = None
- if self.type == "xen":
- ostype = "xen"
# Need to fetch the kernel & initrd from a remote site, or
# out of a loopback mounted disk image/device
- (kernelfn, initrdfn, args) = acquireKernel(self.location,
- meter,
- scratchdir = self.scratchdir,
- type = ostype,
- distro = distro)
- 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
-
+ (kernelfn, initrdfn, args) = self._distro.acquireKernel(meter)
self._tmpfiles.append(kernelfn)
self._tmpfiles.append(initrdfn)
+ 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
+
# If they're installing off a local file/device, we map it
- # through to a virtual harddisk
+ # through to a virtual cdrom
if self.location is not None and self.location.startswith("/"):
guest.disks.append(Guest.VirtualDisk(self.location ,
+ device=Guest.VirtualDisk.DEVICE_CDROM,
readOnly=True,
transient=True))
def get_os_type(self):
return self._os_type
+
def set_os_type(self, val):
if FullVirtGuest.OS_TYPES.has_key(val):
self._os_type = val
@@ -133,6 +133,7 @@
def get_os_variant(self):
return self._os_variant
+
def set_os_variant(self, val):
if FullVirtGuest.OS_TYPES[self._os_type]["variants"].has_key(val):
self._os_variant = val
@@ -241,42 +242,24 @@
def _get_disk_xml(self, install = True):
"""Get the disk config in the libvirt XML format"""
ret = ""
- nodes = {}
- for i in range(4):
- n = "%s%c" % (self.disknode, ord('a') + i)
- nodes[n] = None
+ diskIndex = 0
# First assign CDROM device nodes, since they're scarce resource
- cdnode = self.disknode + "c"
+ cdnode = self._installer.getFullTarget("cdrom", 0)
for d in self.disks:
if d.device != Guest.VirtualDisk.DEVICE_CDROM:
continue
- if d.target:
- if d.target != cdnode:
- raise ValueError, "The CDROM must be device %s" % cdnode
- else:
- d.target = cdnode
-
- if nodes[d.target] != None:
- raise ValueError, "The CDROM device %s is already used" % d.target
- nodes[d.target] = d
+ d.target = cdnode
+ break
# Now assign regular disk node with remainder
for d in self.disks:
if d.device == Guest.VirtualDisk.DEVICE_CDROM:
continue
- if d.target is None: # Auto-assign disk
- for n in sorted(nodes.keys()):
- if nodes[n] is None:
- d.target = n
- nodes[d.target] = d
- break
- else:
- if nodes[d.target] != None: # Verify pre-assigned
- raise ValueError, "The disk device %s is already used" % d.target
- nodes[d.target] = d
+ d.target = self._installer.getFullTarget("disk", diskIndex)
+ diskIndex = diskIndex + 1
for d in self.disks:
saved_path = None
--- virtinst-0.300.1-orig/virtinst/Guest.py 2007-09-25 08:01:12.000000000 -0700
+++ virtinst-0.300.1/virtinst/Guest.py 2007-11-22 22:07:42.000000000 -0800
@@ -171,7 +171,11 @@
# get working domain's name
ids = conn.listDomainsID();
for id in ids:
- vm = conn.lookupByID(id)
+ try:
+ vm = conn.lookupByID(id)
+ except libvirt.libvirtError:
+ # logging.error("LookupDomainByID(%d) failed - probably active domain not in xenstore" % id)
+ continue
vms.append(vm)
# get defined domain
names = conn.listDefinedDomains()
@@ -242,7 +246,11 @@
ids = conn.listDomainsID();
vms = []
for id in ids:
- vm = conn.lookupByID(id)
+ try:
+ vm = conn.lookupByID(id)
+ except libvirt.libvirtError:
+ # logging.error("LookupDomainByID(%d) failed - probably active domain not in xenstore" % id)
+ continue
vms.append(vm)
# get inactive Domains
inactive_vm = []
@@ -388,13 +396,8 @@
if not extraargs is None:
self.extraargs = extraargs
- self._tmpfiles = []
-
def cleanup(self):
- for f in self._tmpfiles:
- logging.debug("Removing " + f)
- os.unlink(f)
- self._tmpfiles = []
+ pass
gettext.bindtextdomain(gettext_app, gettext_dir)
--- virtinst-0.300.1-orig/virtinst/OSDistro.py 1969-12-31 16:00:00.000000000 -0800
+++ virtinst-0.300.1/virtinst/OSDistro.py 2007-11-22 20:13:27.000000000 -0800
@@ -0,0 +1,509 @@
+#!/usr/bin/python -tt
+#
+# Abstraction of OS and Distribution specifics
+#
+# Copyright 2006-2007 Red Hat, Inc.
+# Daniel P. Berrange <berrange@redhat.com>
+#
+# This software may be freely redistributed under the terms of the GNU
+# general public license.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+import logging
+import os
+import gzip
+import re
+import tempfile
+
+from virtinst import _virtinst as _
+from ImageFetcher import ImageFetcher
+
+# This is a generic base class for retrieving information particular to an
+# OS Distribution
+class OSDistro:
+
+ @staticmethod
+ def create(location, type, distroName, scratchdir, progresscb):
+ fetcher = ImageFetcher.create(location, scratchdir)
+
+ try:
+ fetcher.prepareLocation(progresscb)
+ except ValueError, e:
+ raise ValueError, _("Invalid install location: ") + str(e)
+
+ if distroName is None:
+ for c in distros.values():
+ d = c(fetcher, type, scratchdir)
+ if d.isValidStore(progresscb):
+ return d
+
+ raise ValueError, _("Could not find an installable distribution at the install location")
+ else:
+ d = distros[distroName](fetcher, type, scratchdir)
+ if d is None:
+ raise ValueError, _("Unknown distribution")
+ elif d.isValidStore(progresscb):
+ return d
+ else:
+ raise ValueError, _("Install location isn't the expected distribution")
+ return
+
+ def __init__(self, fetcher, type, scratchdir):
+ self.fetcher = fetcher
+ self.type = type
+ self.scratchdir = scratchdir
+
+ def releaseFile(self, filename):
+ self.fetcher.releaseFile(filename)
+
+ def prepare(self, guest):
+ pass
+
+ def acquireBootDisk(self, progresscb):
+ raise "Not implemented"
+
+ def acquireKernel(self, progresscb):
+ raise "Not implemented"
+
+ def isValidStore(self, progresscb):
+ raise "Not implemented"
+
+ def getFullTarget(self, type, index):
+ if type == cdrom:
+ if index == 0:
+ return "hdc"
+ return None
+ else:
+ if index == 0:
+ return "hda"
+ elif index == 1:
+ return "hdb"
+ elif index == 2:
+ return "hdd"
+ return None
+
+ def getParaTarget(self, type, index):
+ return "xvd%c" % (ord('a') + index)
+
+
+# Base image store for any Red Hat related distros which have
+# a common layout
+class RedHatDistro(OSDistro):
+
+ def acquireKernel(self, progresscb):
+ if self.type is None:
+ kernelpath = "images/pxeboot/vmlinuz"
+ initrdpath = "images/pxeboot/initrd.img"
+ else:
+ kernelpath = "images/%s/vmlinuz" % (self.type)
+ initrdpath = "images/%s/initrd.img" % (self.type)
+
+ kernel = self.fetcher.acquireFile(kernelpath, progresscb)
+ try:
+ initrd = self.fetcher.acquireFile(initrdpath, progresscb)
+ return (kernel, initrd, "method=" + self.fetcher.location)
+ except:
+ self.fetcher.releaseFile(kernel)
+
+ def acquireBootDisk(self, progresscb):
+ return self.fetcher.acquireFile("images/boot.iso", progresscb)
+
+
+# Fedora distro check
+class FedoraDistro(RedHatDistro):
+
+ def isValidStore(self, progresscb):
+ if self.fetcher.hasFile("fedora.css", progresscb):
+ logging.debug("Detected a Fedora distro")
+ return True
+ if self.fetcher.hasFile("Fedora", progresscb):
+ logging.debug("Detected a Fedora distro")
+ return True
+ return False
+
+
+# Fedora distro check
+class RHELDistro(RedHatDistro):
+
+ def isValidStore(self, progresscb):
+ if self.fetcher.hasFile("Server", progresscb):
+ logging.debug("Detected a RHEL 5 Server distro")
+ return True
+ if self.fetcher.hasFile("Client", progresscb):
+ logging.debug("Detected a RHEL 5 Client distro")
+ return True
+ if self.fetcher.hasFile("RedHat", progresscb):
+ logging.debug("Detected a RHEL 4 distro")
+ return True
+ return False
+
+
+# CentOS distro check
+class CentOSDistro(RedHatDistro):
+
+ def isValidStore(self, progresscb):
+ if self.fetcher.hasFile("CentOS", progresscb):
+ logging.debug("Detected a CentOS distro")
+ return True
+ return False
+
+
+# Suse image store is harder - we fetch the kernel RPM and a helper
+# RPM and then munge bits together to generate a initrd
+class SuseDistro(OSDistro):
+
+ def acquireBootDisk(self, progresscb):
+ return self.fetcher.acquireFile("boot/boot.iso", progresscb)
+
+ def acquireKernel(self, progresscb):
+ kernelrpm = None
+ installinitrdrpm = None
+ filelist = None
+ try:
+ # There is no predictable filename for kernel/install-initrd RPMs
+ # so we have to grok the filelist and find them
+ filelist = self.fetcher.acquireFile("ls-lR.gz", progresscb)
+ (kernelrpmname, installinitrdrpmname) = self.extractRPMNames(filelist)
+
+ # Now fetch the two RPMs we want
+ kernelrpm = self.fetcher.acquireFile(kernelrpmname, progresscb)
+ installinitrdrpm = self.fetcher.acquireFile(installinitrdrpmname, progresscb)
+
+ # Process the RPMs to extract the kernel & generate an initrd
+ return self.buildKernelInitrd(fetcher, kernelrpm, installinitrdrpm, progresscb)
+ finally:
+ if filelist is not None:
+ self.fetcher.releaseFile(filelist)
+ if kernelrpm is not None:
+ self.fetcher.releaseFile(kernelrpm)
+ if installinitrdrpm is not None:
+ self.fetcher.releaseFile(installinitrdrpm)
+
+ # We need to parse the ls-lR.gz file, looking for the kernel &
+ # install-initrd RPM entries - capturing the directory they are
+ # in and the version'd filename.
+ def extractRPMNames(self, filelist):
+ filelistData = gzip.GzipFile(filelist, mode = "r")
+ try:
+ arch = os.uname()[4]
+ arches = [arch]
+ # On i686 arch, we also look under i585 and i386 dirs
+ # in case the RPM is built for a lesser arch. We also
+ # need the PAE variant (for Fedora dom0 at least)
+ #
+ # XXX shouldn't hard code that dom0 is PAE
+ if arch == "i686":
+ arches.append("i586")
+ arches.append("i386")
+ kernelname = "kernel-xenpae"
+
+ installinitrdrpm = None
+ kernelrpm = None
+ dir = None
+ while 1:
+ data = filelistData.readline()
+ if not data:
+ break
+ if dir is None:
+ for arch in arches:
+ wantdir = "/suse/" + arch
+ if data == "." + wantdir + ":
":
+ dir = wantdir
+ break
+ else:
+ if data == "
":
+ dir = None
+ else:
+ if data[:5] != "total":
+ filename = re.split("s+", data)[8]
+
+ if filename[:14] == "install-initrd":
+ installinitrdrpm = dir + "/" + filename
+ elif filename[:len(kernelname)] == kernelname:
+ kernelrpm = dir + "/" + filename
+
+ if kernelrpm is None:
+ raise _("Unable to determine kernel RPM path")
+ if installinitrdrpm is None:
+ raise _("Unable to determine install-initrd RPM path")
+ return (kernelrpm, installinitrdrpm)
+ finally:
+ filelistData.close()
+
+ # We have a kernel RPM and a install-initrd RPM with a generic initrd in it
+ # Now we have to munge the two together to build an initrd capable of
+ # booting the installer.
+ #
+ # Yes, this is crazy ass stuff :-)
+ def buildKernelInitrd(self, kernelrpm, installinitrdrpm, progresscb):
+ progresscb.start(text=_("Building initrd"), size=11)
+ progresscb.update(1)
+ cpiodir = tempfile.mkdtemp(prefix="virtinstcpio.", dir=self.scratchdir)
+ try:
+ # Extract the kernel RPM contents
+ os.mkdir(cpiodir + "/kernel")
+ cmd = "cd " + cpiodir + "/kernel && (rpm2cpio " + kernelrpm + " | cpio --quiet -idm)"
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(2)
+
+ # Determine the raw kernel version
+ kernelinfo = None
+ for f in os.listdir(cpiodir + "/kernel/boot"):
+ if f.startswith("System.map-"):
+ kernelinfo = re.split("-", f)
+ kernel_override = kernelinfo[1] + "-override-" + kernelinfo[3]
+ kernel_version = kernelinfo[1] + "-" + kernelinfo[2] + "-" + kernelinfo[3]
+ logging.debug("Got kernel version " + str(kernelinfo))
+
+ # Build a list of all .ko files
+ modpaths = {}
+ for root, dirs, files in os.walk(cpiodir + "/kernel/lib/modules", topdown=False):
+ for name in files:
+ if name.endswith(".ko"):
+ modpaths[name] = os.path.join(root, name)
+ progresscb.update(3)
+
+ # Extract the install-initrd RPM contents
+ os.mkdir(cpiodir + "/installinitrd")
+ cmd = "cd " + cpiodir + "/installinitrd && (rpm2cpio " + installinitrdrpm + " | cpio --quiet -idm)"
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(4)
+
+ # Read in list of mods required for initrd
+ modnames = []
+ fn = open(cpiodir + "/installinitrd/usr/lib/install-initrd/" + kernelinfo[3] + "/module.list", "r")
+ try:
+ while 1:
+ line = fn.readline()
+ if not line:
+ break
+ line = line[:len(line)-1]
+ modnames.append(line)
+ finally:
+ fn.close()
+ progresscb.update(5)
+
+ # Uncompress the basic initrd
+ cmd = "gunzip -c " + cpiodir + "/installinitrd/usr/lib/install-initrd/initrd-base.gz > " + cpiodir + "/initrd.img"
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(6)
+
+ # Create temp tree to hold stuff we're adding to initrd
+ moddir = cpiodir + "/initrd/lib/modules/" + kernel_override + "/initrd/"
+ moddepdir = cpiodir + "/initrd/lib/modules/" + kernel_version
+ os.makedirs(moddir)
+ os.makedirs(moddepdir)
+ os.symlink("../" + kernel_override, moddepdir + "/updates")
+ os.symlink("lib/modules/" + kernel_override + "/initrd", cpiodir + "/initrd/modules")
+ cmd = "cp " + cpiodir + "/installinitrd/usr/lib/install-initrd/" + kernelinfo[3] + "/module.config" + " " + moddir
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(7)
+
+ # Copy modules we need into initrd staging dir
+ for modname in modnames:
+ if modpaths.has_key(modname):
+ src = modpaths[modname]
+ dst = moddir + "/" + modname
+ os.system("cp " + src + " " + dst)
+ progresscb.update(8)
+
+ # Run depmod across the staging area
+ cmd = "depmod -a -b " + cpiodir + "/initrd -F " + cpiodir + "/kernel/boot/System.map-" + kernel_version + " " + kernel_version
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(9)
+
+ # Add the extra modules to the basic initrd
+ cmd = "cd " + cpiodir + "/initrd && ( find . | cpio --quiet -o -H newc -A -F " + cpiodir + "/initrd.img)"
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.update(10)
+
+ # Compress the final initrd
+ cmd = "gzip -f9N " + cpiodir + "/initrd.img"
+ logging.debug("Running " + cmd)
+ os.system(cmd)
+ progresscb.end(11)
+
+ # Save initrd & kernel to temp files for booting...
+ initrdname = self.fetcher.saveTemp(open(cpiodir + "/initrd.img.gz", "r"), "initrd.img")
+ logging.debug("Saved " + initrdname)
+ try:
+ kernelname = self.fetcher.saveTemp(open(cpiodir + "/kernel/boot/vmlinuz-" + kernel_version, "r"), "vmlinuz")
+ logging.debug("Saved " + kernelname)
+ return (kernelname, initrdname, "install=" + self.fetcher.location)
+ except:
+ self.fetcher.releaseFile(initrdname)
+ finally:
+ #pass
+ os.system("rm -rf " + cpiodir)
+
+
+ def isValidStore(self, progresscb):
+ # Suse distros always have a 'directory.yast' file in the top
+ # level of install tree, which we use as the magic check
+ if self.fetcher.hasFile("directory.yast", progresscb):
+ logging.debug("Detected a Suse distro")
+ return True
+ return False
+
+
+class DebianDistro(OSDistro):
+
+ def isValidStore(self, progresscb):
+ # Don't support any paravirt installs
+ if self.type is not None:
+ return False
+
+ file = None
+ try:
+ try:
+ file = self.fetcher.acquireFile("current/images/MANIFEST", progresscb)
+ except ValueError, e:
+ logging.debug("Doesn't look like a Debian distro " + str(e))
+ return False
+ f = open(file, "r")
+ try:
+ while 1:
+ buf = f.readline()
+ if not buf:
+ break
+ if re.match(".*debian.*", buf):
+ logging.debug("Detected a Debian distro")
+ return True
+ finally:
+ f.close()
+ finally:
+ if file is not None:
+ self.fetcher.releaseFile(file)
+ return False
+
+ def acquireBootDisk(self, progresscb):
+ # eg from http://ftp.egr.msu.edu/debian/dists/sarge/main/installer-i386/
+ return self.fetcher.acquireFile("current/images/netboot/mini.iso", progresscb)
+
+
+class UbuntuDistro(OSDistro):
+
+ def isValidStore(self, progresscb):
+ # Don't support any paravirt installs
+ if self.type is not None:
+ return False
+ return False
+
+
+class GentooDistro(OSDistro):
+
+ def isValidStore(self, progresscb):
+ # Don't support any paravirt installs
+ if self.type is not None:
+ return False
+ return False
+
+class MandrivaDistro(OSDistro):
+
+ def isValidStore(self, progresscb):
+ # Don't support any paravirt installs
+ if self.type is not None:
+ return False
+
+ # Mandriva websites / media appear to have a VERSION
+ # file in top level which we can use as our 'magic'
+ # check for validity
+ version = None
+ try:
+ try:
+ version = self.fetcher.acquireFile("VERSION")
+ except:
+ return False
+ f = open(version, "r")
+ try:
+ info = f.readline()
+ if info.startswith("Mandriva"):
+ logging.debug("Detected a Mandriva distro")
+ return True
+ finally:
+ f.close()
+ finally:
+ if version is not None:
+ self.fetcher.releaseFile(version)
+
+ return False
+
+ def acquireBootDisk(self, fetcher, progresscb):
+ #
+ return self.fetcher.acquireFile("install/images/boot.iso", progresscb)
+
+
+class OpenSolarisDistro(OSDistro):
+
+ def prepare(self, guest):
+ guest.graphics = False
+
+ def acquireKernel(self, progresscb):
+ arch = os.uname()[4]
+ if arch == "i386":
+ kernelpath = "boot/platform/i86%s/kernel/unix"
+ initrdname = "boot/x86.miniroot"
+ else:
+ kernelpath = "boot/platform/i86%s/kernel/amd64/unix"
+ initrdname = "boot/amd64/x86.miniroot"
+
+ if self.type is None:
+ kernelname = kernelpath % "pc"
+ else:
+ kernelname = kernelpath % "xpv"
+
+ kernel = self.fetcher.acquireFile(kernelname, progresscb)
+ try:
+ initrd = self.fetcher.acquireFile(initrdname, progresscb)
+ if self.fetcher.location.startswith("nfs://"):
+ install_media = self.fetcher.location[6:]
+ else:
+ install_media = "cdrom"
+ return (kernel, initrd, kernelname + " - nowin -B install_media=" + install_media + ",console=ttya")
+ except:
+ os.unlink(kernel)
+
+ def acquireBootDisk(self, progresscb):
+ return self.fetcher.acquireFile("images/boot.iso", progresscb)
+
+ def isValidStore(self, progresscb):
+ if self.fetcher.hasFile("boot/platform/i86xpv/kernel/unix", progresscb):
+ logging.debug("Detected a Solaris distro")
+ return True
+ return False
+
+ def _getTargetDevice(self, type, index):
+ if index >= 0 and index < 6:
+ if type == "disk":
+ return str(index)
+ else:
+ return str(index + 6)
+ return None
+
+ def getFullTarget(self, type, index):
+ return self._getTargetDevice(type, index)
+
+ def getParaTarget(self, type, index):
+ return self._getTargetDevice(type, index)
+
+
+distros = {}
+distros["fedora"] = FedoraDistro
+distros["rhel"] = RHELDistro
+distros["centos"] = CentOSDistro
+distros["suse"] = SuseDistro
+distros["debian"] = DebianDistro
+distros["ubuntu"] = UbuntuDistro
+distros["gentoo"] = GentooDistro
+distros["mandriva"] = MandrivaDistro
+distros["opensolaris"] = OpenSolarisDistro
+
--- virtinst-0.300.1-orig/virtinst/ParaVirtGuest.py 2007-09-25 08:01:12.000000000 -0700
+++ virtinst-0.300.1/virtinst/ParaVirtGuest.py 2007-11-22 22:07:42.000000000 -0800
@@ -23,7 +23,6 @@
if not installer:
installer = DistroManager.DistroInstaller(type = type)
Guest.Guest.__init__(self, type, connection, hypervisorURI, installer)
- self.disknode = "xvd"
def _get_osblob(self, install):
return self.installer._get_osblob(install, hvm = False)
@@ -50,21 +49,19 @@
def _get_disk_xml(self, install = True):
"""Get the disk config in the libvirt XML format"""
ret = ""
- nodes = {}
- for i in range(16):
- n = "%s%c" % (self.disknode, ord('a') + i)
- nodes[n] = None
+ diskIndex = 0
+ cdromIndex = 0
for d in self.disks:
if d.transient and not install:
continue
- target = d.target
+ if d.device == Guest.VirtualDisk.DEVICE_CDROM:
+ target = self._installer.getParaTarget("cdrom", cdromIndex)
+ cdromIndex = cdromIndex + 1
+ else:
+ target = self._installer.getParaTarget("disk", diskIndex)
+ diskIndex = diskIndex + 1
+
if target is None:
- for t in sorted(nodes.keys()):
- if nodes[t] is None:
- target = t
- break
- if target is None or nodes[target] is not None:
raise ValueError, _("Can't use more than 16 disks on a PV guest")
- nodes[target] = True
ret += d.get_xml_config(target)
return ret
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
Robert Nelson wrote:
> I spent some time reworking the virtinst code to support OpenSolaris and
> make it much easier to support additional OSes. I've attached the patch
> file to get some feedback on the work so far.
>
> Unfortunately a bunch for the code from virtinst is duplicated in
> virt-manager in the Add Device code. This means either moving it back
> to virtinst with the appropriate additional APIs or duplicating work in
> virt-manager.
>
>
>
Hi Robert,
Could you take a couple paragraphs explaining exactly how you reorganized
everything? And also I think posting the patch to its own thread will get
it more attention.
One small thing before you repost: __init__.py had ::LOCALEDIR:: replaced
with /usr/share/locale. This shouldn't be hardcoded and is filled in as
appropriate at build time by setup.py.
Thanks,
Cole
--
Cole Robinson
crobinso@redhat.com
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
On Thu, Nov 22, 2007 at 11:06:58PM -0800, Robert Nelson wrote:
> Daniel P. Berrange wrote:
> >
> >In the virtinst/FullVirtGuest.py class, there is already a bunch of
> >OS specific metadata, eg what of mouse to use, apic/acpi/pae settings,
> >whether the installer is multi-stage reboots (eg Windows). I'd recommend
> >moving this metadata into virtinst/DistroManager and have a bunch of
> >methods in that module for querying distro specific metadata, from the
> >Installer class.
> >
> >Dan.
> >
> I spent some time reworking the virtinst code to support OpenSolaris and
> make it much easier to support additional OSes. I've attached the patch
> file to get some feedback on the work so far.
Thanks for this. Took me a little while to get to grips with it, but I
like the resulting structure. I'm going todo a few tests and if it
works I'll commit it to the repo.
BTW, for any large patches in the future, its handy for review if you
do separate patches for re-factoring existing code, vs adding new
capabilities. It would have made it easier to review if the splitting
up of the DistroManager.py file were a simple no-functional-change
refactoring. Also helps people browsing the SCM history in the future
to distinguish the changes. No need to change this existing patch
though, I'll just add as is...
> Unfortunately a bunch for the code from virtinst is duplicated in
> virt-manager in the Add Device code. This means either moving it back
> to virtinst with the appropriate additional APIs or duplicating work in
> virt-manager.
We should make virt-manager call into the virtinst APIs really. virtinst
is intended as the library for virt-manager to get all OS install/setup
logic from.
On Thu, Nov 22, 2007 at 11:06:58PM -0800, Robert Nelson wrote:
Daniel P. Berrange wrote:
In the virtinst/FullVirtGuest.py class, there is already a bunch of
OS specific metadata, eg what of mouse to use, apic/acpi/pae settings,
whether the installer is multi-stage reboots (eg Windows). I'd recommend
moving this metadata into virtinst/DistroManager and have a bunch of
methods in that module for querying distro specific metadata, from the
Installer class.
Dan.
I spent some time reworking the virtinst code to support OpenSolaris and
make it much easier to support additional OSes. I've attached the patch
file to get some feedback on the work so far.
Thanks for this. Took me a little while to get to grips with it, but I
like the resulting structure. I'm going todo a few tests and if it
works I'll commit it to the repo.
BTW, for any large patches in the future, its handy for review if you
do separate patches for re-factoring existing code, vs adding new
capabilities. It would have made it easier to review if the splitting
up of the DistroManager.py file were a simple no-functional-change
refactoring. Also helps people browsing the SCM history in the future
to distinguish the changes. No need to change this existing patch
though, I'll just add as is...
Please hold off on committing this patch.* I have an updated and better
version coming shortly.* I'll post it as two patches to make the SCM
history more readable.* I'll also post it to a new thread.
Unfortunately a bunch for the code from virtinst is duplicated in
virt-manager in the Add Device code. This means either moving it back
to virtinst with the appropriate additional APIs or duplicating work in
virt-manager.
We should make virt-manager call into the virtinst APIs really. virtinst
is intended as the library for virt-manager to get all OS install/setup
logic from.
If you don't mind, I'll put together a prototype of this for review.
Regards,
Dan.
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
I spent some time reworking the virtinst code to support OpenSolaris and
make it much easier to support additional OSes. I've attached the patch
file to get some feedback on the work so far.
Unfortunately a bunch for the code from virtinst is duplicated in
virt-manager in the Add Device code. This means either moving it back
to virtinst with the appropriate additional APIs or duplicating work in
virt-manager.
Hi Robert,
Could you take a couple paragraphs explaining exactly how you reorganized
everything? And also I think posting the patch to its own thread will get
it more attention.
I'll put more explanation in the next version of the patch.
One small thing before you repost: __init__.py had ::LOCALEDIR:: replaced
with /usr/share/locale. This shouldn't be hardcoded and is filled in as
appropriate at build time by setup.py.
This happens during the build.* Since I diffed against my built version
it had been updated.* Usually I delete that diff from the patch but I
guess I missed it in that version.
Thanks,
Cole
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
On Thu, Nov 29, 2007 at 08:14:50PM -0800, Robert Nelson wrote:
> Daniel P. Berrange wrote:
> >>I spent some time reworking the virtinst code to support OpenSolaris and
> >>make it much easier to support additional OSes. I've attached the patch
> >>file to get some feedback on the work so far.
> >>
> >
> >Thanks for this. Took me a little while to get to grips with it, but I
> >like the resulting structure. I'm going todo a few tests and if it
> >works I'll commit it to the repo.
> >
> >BTW, for any large patches in the future, its handy for review if you
> >do separate patches for re-factoring existing code, vs adding new
> >capabilities. It would have made it easier to review if the splitting
> >up of the DistroManager.py file were a simple no-functional-change
> >refactoring. Also helps people browsing the SCM history in the future
> >to distinguish the changes. No need to change this existing patch
> >though, I'll just add as is...
>
> Please hold off on committing this patch. I have an updated and better
> version coming shortly. I'll post it as two patches to make the SCM
> history more readable. I'll also post it to a new thread.
So it turned out to be a little more complicated than I expected. At first
I liked the idea of having OSDistro create & use the ImageFetcher class
directly, however, there are circumstances where we need to use the OSDistro
class without the ImageFetcher, so this dependancy causes problems.
Second, the FullVirtGuest/ParaVirtGuest classes now call into the installer
class to get the disk target - this is a problem because this code is only
relevant for the DistroInstaller class - the ImageInstaller works in a
different way.
Really, theFullVirt/ParaVirt guest class should not try todo any assignement
of disk targets at all. The 'prepare' method fo the DistroInstaller class
should contain the code to assign disk targets directly. This simplifies
the get_disk_xml methods in the Full/ParaVirtGuest classes, and avoids
the hard dependancy.
So, in the end I just committed two small parts. I pulled the ImageFetcher
classes out into a separate python module, and pulled the OSDistro
classes out into a separate python module. So just 2 no-functional-change
refactorings for now.
If you post your latest patches I'll give them another try & see if there's
more bits we can safely commit as we go.
> >>Unfortunately a bunch for the code from virtinst is duplicated in
> >>virt-manager in the Add Device code. This means either moving it back
> >>to virtinst with the appropriate additional APIs or duplicating work in
> >>virt-manager.
> >>
> >
> >We should make virt-manager call into the virtinst APIs really. virtinst
> >is intended as the library for virt-manager to get all OS install/setup
> >logic from.
> >
> >
>
> If you don't mind, I'll put together a prototype of this for review.
Sure. The complicated thing about the 'Add device' code is that we no
longer have any record of the OS Distro type at that point - this info
is only available at install time. So its not clear how we'd be able
to get an instance of the OSDistro class neccessary to figure out the
disk target names here. Also this is one use case I mentioned where
you need OSDistro to be independant of the ImageFetcher class.
BTW, I'm not convinced the Solaris distro class is correct. I know the
paravirt guests have to name their disk targets 0, 1, 2 ... etc isntead
of xvda, xvdb, etc. For fully-virt Solaris installs though it must
still use hda, hdb, hdc, etc as per Linux. This is because the hda, hdb,
etc names translate directly to QEMU command line args - they have no
resemblance to what the guest OS calls its disks - indeed modern Linux
will call them sda, sdb, sdc, etc regardless of what QEMU calls tem.
On Fri, Nov 30, 2007 at 02:57:16PM +0000, Daniel P. Berrange wrote:
> On Thu, Nov 29, 2007 at 08:14:50PM -0800, Robert Nelson wrote:
> > Daniel P. Berrange wrote:
> > >>I spent some time reworking the virtinst code to support OpenSolaris and
> > >>make it much easier to support additional OSes. I've attached the patch
> > >>file to get some feedback on the work so far.
> > >>
> > >
> > >Thanks for this. Took me a little while to get to grips with it, but I
> > >like the resulting structure. I'm going todo a few tests and if it
> > >works I'll commit it to the repo.
> > >
> > >BTW, for any large patches in the future, its handy for review if you
> > >do separate patches for re-factoring existing code, vs adding new
> > >capabilities. It would have made it easier to review if the splitting
> > >up of the DistroManager.py file were a simple no-functional-change
> > >refactoring. Also helps people browsing the SCM history in the future
> > >to distinguish the changes. No need to change this existing patch
> > >though, I'll just add as is...
> >
> > Please hold off on committing this patch. I have an updated and better
> > version coming shortly. I'll post it as two patches to make the SCM
> > history more readable. I'll also post it to a new thread.
>
> So it turned out to be a little more complicated than I expected. At first
> I liked the idea of having OSDistro create & use the ImageFetcher class
> directly, however, there are circumstances where we need to use the OSDistro
> class without the ImageFetcher, so this dependancy causes problems.
>
> Second, the FullVirtGuest/ParaVirtGuest classes now call into the installer
> class to get the disk target - this is a problem because this code is only
> relevant for the DistroInstaller class - the ImageInstaller works in a
> different way.
>
> Really, theFullVirt/ParaVirt guest class should not try todo any assignement
> of disk targets at all. The 'prepare' method fo the DistroInstaller class
> should contain the code to assign disk targets directly. This simplifies
> the get_disk_xml methods in the Full/ParaVirtGuest classes, and avoids
> the hard dependancy.
>
> So, in the end I just committed two small parts. I pulled the ImageFetcher
> classes out into a separate python module, and pulled the OSDistro
> classes out into a separate python module. So just 2 no-functional-change
> refactorings for now.
I've also just commited the 'LocalImageFetcher' and the modifications to
the Guest.py class to catch exceptions when looking up guests for the
disk check since they're both worthwhile changes.
Virt-Manager: Supporting additional para-virtual OS's
