The attached patch removes most parameter prompting from
the virt command line tools. I think any usefulness they
once had is largely gone, and they are becoming a maintainance
burden. This burden is even more apparent trying to
update this stuff to accommodate remote installs.
Currently, if not specified, we prompt for:
vm name (will now fail if not specified)
vm ram (will now fail if not specified)
install media (this is a real pain now. originally the
install type matrix was far simpler, so
abstracting what type of media the user
'really' means is actually impossible
in some cases from this prompt)
pv vs. hvm (this was actually dropped in last release)
enable vnc (should just default to vnc if display is set,
nographics otherwise)
disk path (will now fail, and inform user they need to
specify --nodisks if they don't want storage)
disk size, if disk doesn't exist (will now fail)
There are also some yes/no prompts to keep users from
shooting themselves in the foot
sparse disk max size exceeds physical disk capacity
specified more vcpus than physical cpus
I say we leave these in. I think these are generally useful,
and any automated virt-install commands should use --force
anyways. I've tried to test all the above cases and it
seems to work as expected.
Any feedback appreciated, wondering what people think about
this.
Thanks,
Cole
# HG changeset patch
# User "Cole Robinson <crobinso@xxxxxxxxxx>"
# Date 1217536060 14400
# Node ID a0c7e10b1e921c2fe46cab9439e6d82713f06185
# Parent 6a207373b908ab521d33cd675c7c8d3854bdc1f1
Remove most prompting from virt-* tools. Leave only yes or no questions that will help users not shoot themselves in the foot.
diff -r 6a207373b908 -r a0c7e10b1e92 virt-install
--- a/virt-install Tue Jul 29 11:21:07 2008 -0400
+++ b/virt-install Thu Jul 31 16:27:40 2008 -0400
@@ -45,60 +45,40 @@
gettext.install(virtinst.gettext_app, virtinst.gettext_dir, unicode=1)
### General input gathering functions
-def get_full_virt():
- while 1:
- return cli.prompt_for_yes_or_no(_("Would you like a fully virtualized guest (yes or no)? This will allow you to run unmodified operating systems."))
+def get_disk(disk, size, sparse, guest, hvm, conn):
+ # Getting disk size
+ if not os.path.exists(disk) and size is None:
+ fail(_("Must specify size for non-existent disks"))
+ try:
+ size = float(size)
+ except Exception, e:
+ fail(e)
-def get_disk(disk, size, sparse, guest, hvm, conn):
- while 1:
- msg = _("What would you like to use as the disk (file path)?")
- if not size is None:
- msg = _("Please enter the path to the file you would like to use for storage. It will have size %sGB.") %(size,)
- disk = cli.prompt_for_input(msg, disk)
+ # Build disk object
+ try:
+ d = virtinst.VirtualDisk(disk, size, sparse = sparse)
+ # 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)
- # Getting disk size
- while 1:
- if os.path.exists(disk):
- break
- size = cli.prompt_for_input(_("How large would you like the disk (%s) to be (in gigabytes)?") %(disk,), size)
- try:
- size = float(size)
- break
- except Exception, e:
- print _("ERROR: "), e
- size = None
+ # Check disk conflicts
+ if d.is_conflict_disk(conn) is True:
+ warnmsg = _("Disk %s is already in use by another guest!\n") % d.path
+ if not cli.prompt_for_yes_or_no(warnmsg + _("Do you really want to use the disk (yes or no)? ")):
+ cli.nice_exit()
- # Build disk object
- try:
- d = virtinst.VirtualDisk(disk, size, sparse = sparse)
- # 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:
- print _("ERROR: "), e
- disk = size = None
- continue
+ ret = d.size_conflict()
+ if ret[0]:
+ fail(ret[1])
+ elif ret[1]:
+ if not cli.prompt_for_yes_or_no(ret[1] + _(" Do you really want to use the disk (yes or no)?")):
+ cli.nice_exit()
- # Check disk conflicts
- if d.is_conflict_disk(conn) is True:
- warnmsg = _("Disk %s is already in use by another guest!\n") \
- % d.path
- if not cli.prompt_for_yes_or_no(warnmsg + _("Do you really want to use the disk (yes or no)? ")):
- disk = size = None
- continue
-
- ret = d.size_conflict()
- if ret[0]:
- raise ValueError, ret[1]
- elif ret[1]:
- if not cli.prompt_for_yes_or_no(ret[1] + _(" Do you really want to use the disk (yes or no)?")):
- disk = size = None
- continue
-
- guest.disks.append(d)
- break
+ guest.disks.append(d)
def get_disks(disk, size, sparse, nodisks, guest, hvm, conn):
if nodisks:
@@ -130,43 +110,40 @@
### Paravirt input gathering functions
-def get_paravirt_install(src, guest):
- while 1:
- src = cli.prompt_for_input(_("What is the install location?"), src)
- try:
- guest.location = src
- break
- except ValueError, e:
- print _("ERROR: "), e
- src = None
-
def get_extraargs(extra, guest):
guest.extraargs = extra
-### fullvirt input gathering functions
-def get_fullvirt_cdrom(cdpath, location, guest):
- # Got a location, then ignore CDROM
- if location is not None:
- guest.location = location
+def get_install_media(location, cdpath, pxe, livecd, guest, ishvm):
+
+ if (pxe and location) or (location and cdpath) or (cdpath and pxe):
+ fail(_("Only one of --pxe, --location and --cdrom can be used"))
+
+ if not ishvm:
+ if pxe:
+ fail(_("Network PXE boot is not supported for paravirtualized "
+ "guests"))
+ if cdpath or livecd:
+ fail(_("Paravirtualized guests cannot install off cdrom media."))
+ 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 pxe:
return
-
- while 1:
- cdpath = cli.prompt_for_input(_("What is the virtual CD image, CD device or install location?"), cdpath)
- try:
- # guest.cdrom is deprecated
- guest.location = cdpath
- if os.path.exists(guest.location):
- # Build a throwaway disk for validation for local CDs only
- cddisk = virtinst.VirtualDisk(path=guest.location,
- transient=True,
- device=virtinst.VirtualDisk.DEVICE_CDROM,
- readOnly=True)
+ try:
+ # guest.cdrom is deprecated
+ 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,
+ transient=True,
+ device=virtinst.VirtualDisk.DEVICE_CDROM,
+ readOnly=True)
guest.installer.cdrom = True
- break
- except ValueError, e:
- print _("ERROR: "), e
- cdpath = None
+ except ValueError, e:
+ fail(e)
### Option parsing
@@ -384,6 +361,7 @@
fail(msg)
os_type = guest.os_type
+ logging.debug("Received virt method '%s'" % os_type)
if os_type == "hvm":
hvm = True
else:
@@ -394,25 +372,17 @@
logging.debug("Hypervisor type is '%s'" % type)
if options.livecd:
- if not hvm:
- fail(_("LiveCD installations are not supported for paravirt guests"))
installer = virtinst.LiveCDInstaller(type = type, os_type = os_type)
elif options.pxe:
installer = virtinst.PXEInstaller(type = type, os_type = os_type)
else:
installer = virtinst.DistroInstaller(type = type, os_type = os_type)
- if (options.pxe and options.location) or (options.location and options.cdrom) or (options.cdrom and options.pxe):
- fail(_("Only one of --pxe, --location and --cdrom can be used"))
if hvm:
- # Xen only supports CDROM
- if type == "xen":
- installer.cdrom = True
- guest = virtinst.FullVirtGuest(connection=conn, installer=installer, arch=guest.arch)
+ guest = virtinst.FullVirtGuest(connection=conn, installer=installer,
+ arch=guest.arch)
else:
- if options.pxe:
- fail(_("Network PXE boot is not support for paravirtualized guests"))
guest = virtinst.ParaVirtGuest(connection=conn, installer=installer)
# now let's get some of the common questions out of the way
@@ -433,17 +403,17 @@
get_networks(options.mac, options.bridge, options.network, guest)
# set up graphics information
- cli.get_graphics(options.vnc, options.vncport, options.nographics, options.sdl, options.keymap, guest)
+ cli.get_graphics(options.vnc, options.vncport, options.nographics,
+ options.sdl, options.keymap, guest)
get_extraargs(options.extra, guest)
# and now for the full-virt vs paravirt specific questions
+ get_install_media(options.location, options.cdrom, options.pxe,
+ options.livecd, guest, hvm)
if not hvm: # paravirt
- get_paravirt_install(options.location, guest)
continue_inst = False
else:
- if not options.pxe:
- get_fullvirt_cdrom(options.cdrom, options.location, guest)
if options.noacpi:
guest.features["acpi"] = False
if options.noapic:
diff -r 6a207373b908 -r a0c7e10b1e92 virtinst/cli.py
--- a/virtinst/cli.py Tue Jul 29 11:21:07 2008 -0400
+++ b/virtinst/cli.py Thu Jul 31 16:27:40 2008 -0400
@@ -107,6 +107,10 @@
logging.error(msg)
sys.exit(1)
+def nice_exit():
+ print _("Exiting at user request.")
+ sys.exit(0)
+
def getConnection(connect):
if connect is None or connect.lower()[0:3] == "xen":
if os.geteuid() != 0:
@@ -159,59 +163,45 @@
#
def get_name(name, guest):
- while 1:
- name = prompt_for_input(_("What is the name of your virtual machine?"), name)
- try:
- guest.name = name
- break
- except ValueError, e:
- print "ERROR: ", e
- name = None
+ if name is None:
+ fail(_("A name is required for the virtual machine."))
+ try:
+ guest.name = name
+ except ValueError, e:
+ fail(e)
def get_memory(memory, guest):
- while 1:
- try:
- memory = int(prompt_for_input(_("How much RAM should be allocated (in megabytes)?"), memory))
- if memory < MIN_RAM:
- print _("ERROR: Installs currently require %d megs of RAM.") %(MIN_RAM,)
- print ""
- memory = None
- continue
- guest.memory = memory
- break
- except ValueError, e:
- print _("ERROR: "), e
- memory = None
+ if memory is None:
+ fail(_("Memory amount is required for the virtual machine."))
+ if memory < MIN_RAM:
+ fail(_("Installs currently require %d megs of RAM.") % MIN_RAM)
+ try:
+ guest.memory = memory
+ except ValueError, e:
+ fail(e)
def get_uuid(uuid, guest):
if uuid:
try:
guest.uuid = uuid
except ValueError, e:
- print _("ERROR: "), e
- sys.exit(1)
+ fail(e)
def get_vcpus(vcpus, check_cpu, guest, conn):
- while 1:
- if check_cpu is None:
- break
+
+ if check_cpu:
hostinfo = conn.getInfo()
cpu_num = hostinfo[4] * hostinfo[5] * hostinfo[6] * hostinfo[7]
if vcpus <= cpu_num:
- break
- res = prompt_for_input(_("You have asked for more virtual CPUs (%d) than there are physical CPUs (%d) on the host. This will work, but performance will be poor. Are you sure? (yes or no)") %(vcpus, cpu_num))
- try:
- if yes_or_no(res):
- break
- vcpus = int(prompt_for_input(_("How many VCPUs should be attached?")))
- except ValueError, e:
- print _("ERROR: "), e
+ pass
+ elif not prompt_for_yes_or_no(_("You have asked for more virtual CPUs (%d) than there are physical CPUs (%d) on the host. This will work, but performance will be poor. Are you sure? (yes or no)") % (vcpus, cpu_num)):
+ nice_exit()
+
if vcpus is not None:
try:
guest.vcpus = vcpus
except ValueError, e:
- print _("ERROR: "), e
- sys.exit(1)
+ fail(e)
def get_cpuset(cpuset, mem, guest, conn):
if cpuset and cpuset != "auto":
@@ -303,28 +293,25 @@
if (vnc and nographics) or \
(vnc and sdl) or \
(sdl and nographics):
- raise ValueError, _("Can't specify more than one of VNC, SDL, or nographics")
+ raise ValueError, _("Can't specify more than one of VNC, SDL, "
+ "or --nographics")
+
+ if not (vnc or nographics or sdl):
+ if "DISPLAY" in os.environ.keys():
+ logging.debug("DISPLAY is set: graphics defaulting to VNC.")
+ vnc = True
+ else:
+ logging.debug("DISPLAY is not set: defaulting to nographics.")
+ nographics = True
+
if nographics is not None:
guest.graphics_dev = None
return
+ if sdl is not None:
+ guest.graphics_dev = VirtualGraphics(type=VirtualGraphics.TYPE_SDL)
+ return
if vnc is not None:
guest.graphics_dev = VirtualGraphics(type=VirtualGraphics.TYPE_VNC)
- if sdl is not None:
- guest.graphics_dev = VirtualGraphics(type=VirtualGraphics.TYPE_SDL)
- while 1:
- if guest.graphics_dev:
- break
- res = prompt_for_input(_("Would you like to enable graphics support? (yes or no)"))
- try:
- vnc = yes_or_no(res)
- except ValueError, e:
- print _("ERROR: "), e
- continue
- if vnc:
- guest.graphics_dev = VirtualGraphics(type=VirtualGraphics.TYPE_VNC)
- else:
- guest.graphics_dev = None
- break
if vncport:
guest.graphics_dev.port = vncport
if keymap:
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
