Per Dan's advice, I've changed the previously proposed virt-install cli options for specifying libvirt managed storage. This patch does not change any possible semantics of --file or --cdrom: they remain strictly for specifying local media. In this way they are basically deprecated. A new cli option is added: --disk. The format is: --disk path:///some/file/path[:device=cdrom|floppy][:permissions=ro|sh] --disk vol:///poolname/volname[:device=cdrom|floppy][:permissions=ro|sh] --disk pool:///poolname[:device=cdrom|floppy][:permissions=ro|sh] The changes from Dan's proposal are: - use of path:/// instead of file:///, since this will hopefully clear up any confusion users previously had with specifying block devices. - For vol:///, I used / as a delimiter vs. ':' between the pool and vol name, to avoid any extra collisions (since I assume '/' is not part of a legal pool name). - Explictly list what type of extra option we are appending (device, permissions). This adds clarity to the command line, reduces chance of name collision, and is similar to the approach used by qemu. The --disk option is quite useful even for the local case, since users can now add cdrom and floppy drives (and eventually other device types) via the cli. This could also be expanded to allow specifying file formats (qcow2, etc.) for creating new storage. The other functionality change is that disk:/// can be used to specify install media. If none of --pxe, --cdrom, or --location are specified, we look to see if a --disk device=cdrom was specified, and if so, use that for installation. We can eventually expand this to allow installing off floppies. One question: Is ':' an appropriate delimiter? qemu typically uses ',' for these types of cli options. May be an arbitrary choice, but perhaps someone has an opinion. There are some small changes in the patch to allow installers to skip setting up an install cdrom if no 'location' is specified, and just defering to the guests disks. The rest of the change is centered in virt-install. virt-install | 174 ++++++++++++++++++++++++++++++++++---------- virtinst/DistroManager.py | 6 + virtinst/LiveCDInstaller.py | 16 ++-- 3 files changed, 150 insertions(+), 46 deletions(-) Thanks, Cole
diff -r bd9e79483f70 virt-install --- a/virt-install Mon Aug 11 18:20:28 2008 -0400 +++ b/virt-install Tue Aug 12 17:04:29 2008 -0400 @@ -44,26 +44,101 @@ gettext.bindtextdomain(virtinst.gettext_app, virtinst.gettext_dir) gettext.install(virtinst.gettext_app, virtinst.gettext_dir, unicode=1) -### General input gathering functions -def get_disk(disk, size, sparse, guest, hvm, conn): +def parse_disk_option(guest, path, size): + """helper to properly parse --disk options""" + abspath = None + voltuple = None + volinst = None + devtype = None + ro = False + shared = False - # Getting disk size - if not os.path.exists(disk) and size is None: - fail(_("Must specify size for non-existent disks")) + origpath = path + + # Strip media type + if path.startswith("path:///"): + path_type = "path:///" + elif path.startswith("vol:///"): + path_type = "vol:///" + elif path.startswith("pool:///"): + path_type = "pool:///" + else: + fail(_("--disk path must start with path:///, pool:///, or vol:///.")) + path = path[len(path_type):] + + # Parse endings for permissions and device type + while True: + if not path.count(":"): + break + path, opts = path.split(":", 1) + for opt in opts.split(":"): + opt_type = None + opt_val = None + if opt.count("="): + opt_type, opt_val = opt.split("=", 1) + if opt_type == "device": + devtype = opt_val + elif opt_type == "permissions": + if opt_val == "ro": + ro = True + elif opt_val == "sh": + shared = True + else: + fail(_("Unknown permissions value '%s'." % opt_val)) + else: + fail(_("Unknown disk option '%s'." % opt)) + + # We return (path, (poolname, volname), volinst, device, readonly, shared) + if path_type == "path:///": + abspath = os.path.abspath(path) + elif path_type == "pool:///": + if not size: + raise ValueError(_("Size must be specified with all 'pool:///'")) + vc = virtinst.Storage.StorageVolume.get_volume_for_pool(pool_name=path, + conn=guest.conn) + vname = virtinst.Storage.StorageVolume.find_free_name(conn=guest.conn, + pool_name=path, + name=guest.name) + volinst = vc(pool_name=path, name=vname, conn=guest.conn, + capacity=(size and size*1024*1024*1024)) + elif path_type == "vol:///": + if not path.count("/"): + raise ValueError(_("Storage volume must be specified as " + "disk:///poolname/volname")) + vollist = path.split("/") + voltuple = (vollist[0], vollist[1]) + logging.debug("Parsed volume: as pool='%s' vol='%s'" % \ + (voltuple[0], voltuple[1])) + + if not devtype: + devtype = virtinst.VirtualDisk.DEVICE_DISK + ret = (abspath, voltuple, volinst, devtype, ro, shared) + logging.debug("parse_disk: returning %s" % str(ret)) + return ret + +def get_disk(disk, size, sparse, guest, hvm, conn, is_file_path): + try: - size = float(size) - except Exception, e: - fail(e) + # Get disk parameters + if is_file_path: + (path, voltuple, volinst, + device, readOnly, shared) = (disk, None, None, + virtinst.VirtualDisk.DEVICE_DISK, + False, False) + else: + (path, voltuple, volinst, + device, readOnly, shared) = parse_disk_option(guest, disk, size) - # Build disk object - try: - d = virtinst.VirtualDisk(disk, size, sparse = sparse) + d = virtinst.VirtualDisk(path=path, size=size, sparse=sparse, + volInstall=volinst, volName=voltuple, + readOnly=readOnly, device=device, + conn=guest.conn) # Default file backed PV guests to tap driver if d.type == virtinst.VirtualDisk.TYPE_FILE \ and not(hvm) and virtinst.util.is_blktap_capable(): d.driver_name = virtinst.VirtualDisk.DRIVER_TAP except ValueError, e: - fail(e) + fail(_("Error with storage parameters: %s" % str(e))) # Check disk conflicts if d.is_conflict_disk(conn) is True: @@ -71,7 +146,7 @@ if not cli.prompt_for_yes_or_no(warnmsg + _("Do you really want to use the disk (yes or no)? ")): cli.nice_exit() - ret = d.size_conflict() + ret = d.is_size_conflict() if ret[0]: fail(ret[1]) elif ret[1]: @@ -80,11 +155,19 @@ guest.disks.append(d) -def get_disks(disk, size, sparse, nodisks, guest, hvm, conn): +def get_disks(file_paths, disk_paths, size, sparse, nodisks, guest, hvm, conn): if nodisks: - if disk or size: - fail(_("Cannot use --file with --nodisks")) + if file_paths or disk_paths or size: + fail(_("Cannot use --file, --size, or --disk with --nodisks")) return + if file_paths and disk_paths: + fail(_("Cannot mix --file and --disk options.")) + elif not file_paths and not disk_paths: + fail(_("A disk must be specified (use --nodisks to override)")) + + is_file_path = (file_paths or False) + disk = (file_paths or disk_paths) + # ensure we have equal length lists if (type(disk) == type(size) == list): if len(disk) != len(size): @@ -94,14 +177,11 @@ elif type(size) == list: disk = [ None ] * len(size) - if (type(disk) == list): - map(lambda d, s: get_disk(d, s, sparse, guest, hvm, conn), - disk, size) - elif (type(size) == list): - map(lambda d, s: get_disk(d, s, sparse, guest, hvm, conn), - disk, size) + if type(disk) == list or type(size) == list: + map(lambda d, s: get_disk(d, s, sparse, guest, hvm, conn, + is_file_path), disk, size) else: - get_disk(disk, size, sparse, guest, hvm, conn) + get_disk(disk, size, sparse, guest, hvm, conn, is_file_path) def get_networks(macs, bridges, networks, guest): (macs, networks) = cli.digest_networks(macs, bridges, networks) @@ -128,22 +208,34 @@ if location is None: fail(_("location must be specified for paravirtualized guests.")) - if not (pxe or location or cdpath): - fail(_("One of --pxe, --location or --cdrom must be specified.")) + if location and virtinst.util.is_uri_remote(guest.conn.getURI()): + fail(_("--location can not be specified for remote connections.")) + + cdinstall = (cdpath or False) + if not (pxe or cdpath or location): + # Look at Guest disks: if there is a cdrom, use for install + for disk in guest.disks: + if disk.device == virtinst.VirtualDisk.DEVICE_CDROM: + cdinstall = True + if not cdinstall: + fail(_("One of --pxe, --location, or cdrom media must be " + "specified.")) if pxe: return try: - # guest.cdrom is deprecated - guest.location = (location or cdpath) + if location or cdpath: + guest.location = (location or cdpath) if cdpath and os.path.exists(cdpath): # Build a throwaway disk for validation for local CDs only - cddisk = virtinst.VirtualDisk(path=guest.location, + cddisk = virtinst.VirtualDisk(path=cdpath, + conn=guest.conn, transient=True, device=virtinst.VirtualDisk.DEVICE_CDROM, readOnly=True) + if cdinstall: guest.installer.cdrom = True except ValueError, e: - fail(e) + fail(_("Error creating cdrom disk: %s" % str(e))) ### Option parsing @@ -177,8 +269,12 @@ # disk options parser.add_option("-f", "--file", type="string", - dest="diskfile", action="callback", callback=cli.check_before_append, + dest="file_path", action="callback", callback=cli.check_before_append, help=_("File to use as the disk image")) + parser.add_option("", "--disk", type="string", dest="disk_path", + action="callback", callback=cli.check_before_append, + help=_("Specify storage to use as a disk with various " + "options.")) parser.add_option("-s", "--file-size", type="float", action="append", dest="disksize", help=_("Size of the disk image (if it doesn't exist) in gigabytes")) @@ -337,7 +433,6 @@ conn = cli.getConnection(options.connect) capabilities = virtinst.CapabilitiesParser.parse(conn.getCapabilities()) - if options.fullvirt and options.paravirt: fail(_("Can't do both --hvm and --paravirt")) @@ -372,12 +467,14 @@ logging.debug("Hypervisor type is '%s'" % type) if options.livecd: - installer = virtinst.LiveCDInstaller(type = type, os_type = os_type) + installer = virtinst.LiveCDInstaller(type = type, os_type = os_type, + conn = conn) elif options.pxe: - installer = virtinst.PXEInstaller(type = type, os_type = os_type) + installer = virtinst.PXEInstaller(type = type, os_type = os_type, + conn = conn) else: - installer = virtinst.DistroInstaller(type = type, os_type = os_type) - + installer = virtinst.DistroInstaller(type = type, os_type = os_type, + conn = conn) if hvm: guest = virtinst.FullVirtGuest(connection=conn, installer=installer, @@ -394,10 +491,9 @@ if hvm: cli.get_sound(options.sound, guest) - # set up disks - get_disks(options.diskfile, options.disksize, options.sparse, options.nodisks, - guest, hvm, conn) + get_disks(options.file_path, options.disk_path, options.disksize, + options.sparse, options.nodisks, guest, hvm, conn) # set up network information get_networks(options.mac, options.bridge, options.network, guest) diff -r bd9e79483f70 virtinst/DistroManager.py --- a/virtinst/DistroManager.py Mon Aug 11 18:20:28 2008 -0400 +++ b/virtinst/DistroManager.py Tue Aug 12 17:04:29 2008 -0400 @@ -258,7 +258,11 @@ } if self.cdrom: - self._prepare_cdrom(guest, distro, meter) + if self.location: + self._prepare_cdrom(guest, distro, meter) + else: + # Booting from a cdrom directly allocated to the guest + pass else: self._prepare_kernel_and_initrd(guest, distro, meter) diff -r bd9e79483f70 virtinst/LiveCDInstaller.py --- a/virtinst/LiveCDInstaller.py Mon Aug 11 18:20:28 2008 -0400 +++ b/virtinst/LiveCDInstaller.py Tue Aug 12 17:04:29 2008 -0400 @@ -47,19 +47,23 @@ break if not found: - raise LiveCDInstallerException(_("HVM virtualisation not supported; cannot boot LiveCD")) + raise LiveCDInstallerException(_("Connection does not support HVM virtualisation, cannot boot live CD")) path = None vol_tuple = None if type(self.location) is tuple: vol_tuple = self.location - else: + elif self.location: path = self.location + elif not self.cdrom: + raise LiveCDInstallerException(_("CDROM media must be specified " + "for the live CD installer.")) - disk = VirtualDisk(path=path, conn=guest.conn, volName=vol_tuple, - device = VirtualDisk.DEVICE_CDROM, - readOnly = True) - guest._install_disks.insert(0, disk) + if path or vol_tuple: + disk = VirtualDisk(path=path, conn=guest.conn, volName=vol_tuple, + device = VirtualDisk.DEVICE_CDROM, + readOnly = True) + guest._install_disks.insert(0, disk) def _get_osblob(self, install, hvm, arch = None, loader = None, conn = None): if install:
_______________________________________________ et-mgmt-tools mailing list et-mgmt-tools@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/et-mgmt-tools