diff --git a/tests/cli-test-xml/compare/virtxml-add-disk-create-storage.xml b/tests/cli-test-xml/compare/virtxml-add-disk-create-storage.xml new file mode 100644 index 00000000..48bdab5d --- /dev/null +++ b/tests/cli-test-xml/compare/virtxml-add-disk-create-storage.xml @@ -0,0 +1,16 @@ +--- Original XML ++++ Altered XML +@@ -301,5 +301,9 @@ + + + ++ ++ ++ ++ + + + + Creating storage file __virtinst_cli_new1.img | 10 MB 00:00 +Domain 'test-many-devices' defined successfully. +Changes will take effect after the next domain shutdown. \ No newline at end of file diff --git a/tests/cli-test-xml/compare/virtxml-add-disk-notarget.xml b/tests/cli-test-xml/compare/virtxml-add-disk-notarget.xml new file mode 100644 index 00000000..c899a91d --- /dev/null +++ b/tests/cli-test-xml/compare/virtxml-add-disk-notarget.xml @@ -0,0 +1,15 @@ +--- Original XML ++++ Altered XML +@@ -301,5 +301,9 @@ + + + ++ ++ ++ ++ + + + +Domain 'test-many-devices' defined successfully. +Changes will take effect after the next domain shutdown. \ No newline at end of file diff --git a/tests/clitest.py b/tests/clitest.py index 9db675bf..2ba918e1 100644 --- a/tests/clitest.py +++ b/tests/clitest.py @@ -827,6 +827,8 @@ c.add_invalid("--remove-device --clock utc") # --remove-device without a dev c.add_compare("--add-device --host-device net_00_1c_25_10_b1_e4", "virtxml-add-host-device") c.add_compare("--add-device --soundhw pcspk", "virtxml-add-sound") c.add_compare("--add-device --disk %(EXISTIMG1)s,bus=virtio,target=vdf", "virtxml-add-disk-basic") +c.add_compare("--add-device --disk %(EXISTIMG1)s", "virtxml-add-disk-notarget") # filling in acceptable target +c.add_compare("--add-device --disk %(NEWIMG1)s,size=.01", "virtxml-add-disk-create-storage") c.add_compare("--remove-device --soundhw ich6", "virtxml-remove-sound-model") c.add_compare("--remove-device --disk 6", "virtxml-remove-disk-index") c.add_compare("--remove-device --disk /dev/null", "virtxml-remove-disk-path") diff --git a/virt-xml b/virt-xml index 578a73a1..fed1046b 100755 --- a/virt-xml +++ b/virt-xml @@ -1,6 +1,6 @@ #!/usr/bin/python -tt # -# Copyright 2013 Red Hat, Inc. +# Copyright 2013-2014 Red Hat, Inc. # Cole Robinson # # This program is free software; you can redistribute it and/or modify @@ -24,6 +24,7 @@ import os import sys import libvirt +import urlgrabber.progress as progress import virtinst from virtinst import cli @@ -232,12 +233,31 @@ def action_build_xml(conn, options, parsermap, parserobj): return ret -def define_changes(conn, inactive_xmlobj, confirm): +def setup_device(dev): + if getattr(dev, "virtual_device_type", None) != "disk": + return + if getattr(dev, "virt_xml_setup", None) is True: + return + + logging.debug("Doing setup for disk=%s", dev) + meter = (cli.quiet and + progress.BaseMeter() or + progress.TextMeter(fo=sys.stdout)) + + dev.setup(meter) + dev.virt_xml_setup = True + + +def define_changes(conn, inactive_xmlobj, devs, action, confirm): if confirm: if not prompt_yes_or_no( _("Define '%s' with the changed XML?" % inactive_xmlobj.name)): return + if action == "hotplug": + for dev in devs: + setup_device(dev) + conn.defineXML(inactive_xmlobj.get_xml_config()) print_stdout(_("Domain '%s' defined successfully." % inactive_xmlobj.name)) @@ -252,6 +272,9 @@ def update_changes(domain, devs, action, confirm): if not prompt_yes_or_no(msg): continue + if action == "hotplug": + setup_device(dev) + try: if action == "hotplug": domain.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_LIVE) @@ -422,7 +445,7 @@ def main(conn=None): if options.update and active_xmlobj: update_changes(domain, devs, action, options.confirm) if options.define: - define_changes(conn, inactive_xmlobj, options.confirm) + define_changes(conn, inactive_xmlobj, devs, action, options.confirm) if not options.update and active_xmlobj: print_stdout( _("Changes will take effect after the next domain shutdown.")) diff --git a/virtinst/cli.py b/virtinst/cli.py index ac8ff257..e02cf196 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -1743,8 +1743,13 @@ class ParserDisk(VirtCLIParser): "vol_install": volinst, "backing_store": backing_store} if any(create_kwargs.values()): inst.set_create_storage(**create_kwargs) - inst.cli_size = size + + if not inst.target: + skip_targets = [d.target for d in self.guest.get_devices("disk")] + inst.generate_target(skip_targets) + inst.cli_set_target = True + return inst diff --git a/virtinst/devicedisk.py b/virtinst/devicedisk.py index 469d968b..795b9214 100644 --- a/virtinst/devicedisk.py +++ b/virtinst/devicedisk.py @@ -824,7 +824,7 @@ class VirtualDisk(VirtualDevice): return ret - def get_target_prefix(self): + def get_target_prefix(self, used_targets=None): """ Returns the suggested disk target prefix (hd, xvd, sd ...) for the disk. @@ -832,17 +832,35 @@ class VirtualDisk(VirtualDevice): """ # The upper limits here aren't necessarilly 1024, but let the HV # error as appropriate. - if self.bus == "virtio": - return ("vd", 1024) - elif self.bus == "xen": - return ("xvd", 1024) - elif self.bus == "fdc" or self.is_floppy(): - return ("fd", 2) - elif self.bus == "ide": - return ("hd", 4) + def _return(prefix): + nummap = { + "vd": 1024, + "xvd": 1024, + "fd": 2, + "hd": 4, + "sd": 1024, + } + return prefix, nummap[prefix] - # sata, scsi, usb, sd - return ("sd", 1024) + if self.bus == "virtio": + return _return("vd") + elif self.bus == "xen": + return _return("xvd") + elif self.bus == "fdc" or self.is_floppy(): + return _return("fd") + elif self.bus == "ide": + return _return("hd") + elif self.bus or not used_targets: + # sata, scsi, usb, sd + return _return("sd") + + # If guest already has some disks defined + preforder = ["vd", "xvd", "sd", "hd"] + for pref in preforder: + for target in used_targets: + if target.startswith(pref): + return _return(pref) + return _return("sd") def generate_target(self, skip_targets): """ @@ -856,7 +874,7 @@ class VirtualDisk(VirtualDevice): @returns generated target @rtype C{str} """ - prefix, maxnode = self.get_target_prefix() + prefix, maxnode = self.get_target_prefix(skip_targets) skip_targets = [t for t in skip_targets if t and t.startswith(prefix)] skip_targets.sort() diff --git a/virtinst/guest.py b/virtinst/guest.py index a2204f42..c50bc03d 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -793,7 +793,7 @@ class Guest(XMLBuilder): set_disk_bus(disk) # Generate disk targets - if disk.target: + if disk.target and not getattr(disk, "cli_set_target", False): used_targets.append(disk.target) else: used_targets.append(disk.generate_target(used_targets))