virt-xml: Create storage if necessary

7 files changed, 93 insertions, 14 deletions

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 @@
+         <protocol type="raw"/>
+       </backend>
+     </rng>
++    <disk type="file" device="disk">
++      <source file="/tmp/__virtinst_cli_new1.img"/>
++      <target dev="vdd"/>
++    </disk>
+   </devices>
+ </domain>
+
+
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 @@
+         <protocol type="raw"/>
+       </backend>
+     </rng>
++    <disk type="file" device="disk">
++      <source file="/tmp/__virtinst_cli_exist1.img"/>
++      <target dev="vdd"/>
++    </disk>
+   </devices>
+ </domain>
+
+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 <crobinso@redhat.com>
 #
 # 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.
+        def _return(prefix):
+            nummap = {
+                "vd": 1024,
+                "xvd": 1024,
+                "fd": 2,
+                "hd": 4,
+                "sd": 1024,
+            }
+            return prefix, nummap[prefix]
+
         if self.bus == "virtio":
-            return ("vd", 1024)
+            return _return("vd")
         elif self.bus == "xen":
-            return ("xvd", 1024)
+            return _return("xvd")
         elif self.bus == "fdc" or self.is_floppy():
-            return ("fd", 2)
+            return _return("fd")
         elif self.bus == "ide":
-            return ("hd", 4)
-
-        # sata, scsi, usb, sd
-        return ("sd", 1024)
+            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))