VirtualDisk: Rework provisioning and validation behavior

We separate all the provisioning bits to diskbackend.py. VirtualDisk
users now need to explicitly opt in to storage creation by using
set_create_storage().

validation is no longer done automatically, users must call the
validation() command.

__init__ drops all extra parameters.

This will eventually get us to a point where we can unify the manual
XML building and XML parsing machinery, and get consistent validation
behavior between devices.

tests/cli-test-xml/compare/clone-auto1.xml

@@ -20,10 +20,10 @@
   <devices>
     <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
     <disk type="block" device="floppy">
-      <source dev="/disk-pool/diskvol1-clone"/>
       <target dev="fda" bus="fdc"/>
       <address type="drive" controller="0" bus="0" target="0" unit="0"/>
       <driver name="qemu" type="raw"/>
+      <source dev="/disk-pool/diskvol1-clone"/>
     </disk>
     <disk type="block" device="disk">
       <source dev="/disk-pool/diskvol2"/>
@@ -46,13 +46,13 @@
     <disk type="file" device="disk">
       <target dev="hda" bus="ide"/>
       <address type="drive" controller="0" bus="0" target="0" unit="0"/>
-      <driver name="qemu" type="raw"/>
+      <driver name="qemu"/>
       <source file="/default-pool/default-vol-clone"/>
     </disk>
     <disk type="file" device="disk">
       <target dev="hdb" bus="ide"/>
       <address type="drive" controller="0" bus="0" target="0" unit="1"/>
-      <driver name="qemu" type="raw"/>
+      <driver name="qemu"/>
       <source file="/default-pool/testvol2-clone.img"/>
     </disk>
     <controller type="scsi" index="0"/>

tests/clone-xml/empty-disks-out.xml

@@ -19,7 +19,7 @@
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type="block" device="disk">
       <target dev="hda" bus="ide"/>
-      <source dev="/dev/loop0"/>
+      <source dev="/disk-pool/disk-vol1"/>
     </disk>
     <disk type="file" device="floppy">
       <target dev="fdb" bus="fdc"/>

tests/clone-xml/force-out.xml

@@ -17,9 +17,9 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-kvm</emulator>
-    <disk type="block" device="disk">
+    <disk type="file" device="disk">
       <target dev="hda" bus="ide"/>
-      <source dev="/dev/loop0"/>
+      <source file="/default-pool/1234.img"/>
     </disk>
     <disk type="block" device="floppy">
       <target dev="fdb" bus="fdc"/>

tests/clone-xml/general-cfg-out.xml

@@ -19,7 +19,7 @@
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type="block" device="disk">
       <target dev="hda" bus="ide"/>
-      <source dev="/dev/loop0"/>
+      <source dev="/disk-pool/disk-vol1"/>
     </disk>
     <disk type="file" device="disk">
       <target dev="hdb" bus="ide"/>

tests/clone-xml/managed-storage-in.xml

@@ -18,6 +18,7 @@
   <devices>
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type='file' device='disk'>
+      <driver name="qemu" type="vmdk"/>
       <source file='/default-pool/testvol1.img'/>
       <target dev='hda' bus='ide'/>
     </disk>

tests/clone-xml/managed-storage-out.xml

@@ -19,11 +19,12 @@
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type="file" device="disk">
       <target dev="hda" bus="ide"/>
+      <driver type="vmdk"/>
       <source file="/default-pool/new1.img"/>
     </disk>
     <disk type="block" device="disk">
-      <source dev="/disk-pool/new2.img"/>
       <target dev="hdb" bus="ide"/>
+      <source dev="/disk-pool/new2.img"/>
     </disk>
     <interface type="network">
       <mac address="22:23:45:67:89:00"/>

tests/clone-xml/readonly-disks-out.xml

@@ -19,7 +19,7 @@
     <emulator>/usr/bin/qemu-kvm</emulator>
     <disk type="block" device="disk">
       <target dev="hda" bus="ide"/>
-      <source dev="/dev/loop0"/>
+      <source dev="/disk-pool/disk-vol1"/>
     </disk>
     <disk type="file" device="disk">
       <source file="/tmp/virtinst-test1.img"/>

tests/clone-xml/skip-out.xml

@@ -28,9 +28,9 @@
       <target dev="sda" bus="scsi"/>
       <readonly/>
     </disk>
-    <disk type="block" device="disk">
+    <disk type="file" device="disk">
       <target dev="sdb" bus="scsi"/>
-      <source dev="/dev/loop0"/>
+      <source file="/default-pool/1234.img"/>
     </disk>
     <interface type="network">
       <mac address="22:23:45:67:89:00"/>

tests/clonetest.py

@@ -92,7 +92,7 @@ class TestClone(unittest.TestCase):
         cloneobj.clone_macs = ["22:23:45:67:89:00", "22:23:45:67:89:01"]
 
         if disks is None:
-            disks = ["/dev/loop0", "/tmp/clone2.img",
+            disks = ["/disk-pool/disk-vol1", "/tmp/clone2.img",
                      "/tmp/clone3.img", "/tmp/clone4.img",
                      "/tmp/clone5.img", None]
 
@@ -166,7 +166,7 @@ class TestClone(unittest.TestCase):
                 # Exception expected
                 logging.debug("Received expected exception: %s", str(e))
 
-    def testCloneStorage(self):
+    def testCloneStorageManaged(self):
         base = "managed-storage"
         self._clone_helper(base, ["%s/new1.img" % POOL1,
                                   "%s/new2.img" % DISKPOOL])
@@ -179,13 +179,15 @@ class TestClone(unittest.TestCase):
     def testCloneStorageForce(self):
         base = "force"
         self._clone_helper(base,
-                           disks=["/dev/loop0", None, "/tmp/clone2.img"],
+                           disks=["/default-pool/1234.img",
+                                  None, "/tmp/clone2.img"],
                            force_list=["hda", "fdb", "sdb"])
 
     def testCloneStorageSkip(self):
         base = "skip"
         self._clone_helper(base,
-                           disks=["/dev/loop0", None, "/tmp/clone2.img"],
+                           disks=["/default-pool/1234.img",
+                                  None, "/tmp/clone2.img"],
                            skip_list=["hda", "fdb"])
 
     def testCloneFullPool(self):
@@ -206,7 +208,8 @@ class TestClone(unittest.TestCase):
         # operation (no libvirt calls), but since /default-pool doesn't exist,
         # this should fail.
         try:
-            self._clone_helper(base, ["/tmp/new1.img", "/tmp/new2.img"])
+            self._clone_helper(base, ["/tmp/new1.img", "/tmp/new2.img"],
+                               compare=False)
 
             raise AssertionError("Managed to unmanaged succeeded, expected "
                                  "failure.")

tests/utils.py

@@ -229,11 +229,11 @@ def make_pxe_installer(gtype="xen"):
     return inst
 
 
-def build_win_kvm(path=None):
+def build_win_kvm(path=None, fake=True):
     g = get_basic_fullyvirt_guest("kvm")
     g.os_type = "windows"
     g.os_variant = "winxp"
-    g.add_device(get_filedisk(path))
+    g.add_device(get_filedisk(path, fake=fake))
     g.add_device(get_blkdisk())
     g.add_device(get_virtual_network())
     g.add_device(VirtualAudio(g.conn))
@@ -245,17 +245,31 @@ def build_win_kvm(path=None):
 def get_floppy(path=None):
     if not path:
         path = "/default-pool/testvol1.img"
-    return VirtualDisk(_conn, path, device=VirtualDisk.DEVICE_FLOPPY)
+    d = VirtualDisk(_conn)
+    d.path = path
+    d.device = d.DEVICE_FLOPPY
+    d.validate()
+    return d
 
 
-def get_filedisk(path=None):
+def get_filedisk(path=None, fake=True):
     if not path:
         path = "/tmp/test.img"
-    return VirtualDisk(_conn, path, size=.0001)
+    d = VirtualDisk(_conn)
+    d.path = path
+    size = None
+    if not fake:
+        size = .000001
+    d.set_create_storage(fake=fake, size=size)
+    d.validate()
+    return d
 
 
 def get_blkdisk(path="/dev/loop0"):
-    return VirtualDisk(_conn, path)
+    d = VirtualDisk(_conn)
+    d.path = path
+    d.validate()
+    return d
 
 
 def get_virtual_network():

tests/validation.py

@@ -15,7 +15,6 @@
 # MA 02110-1301 USA.
 
 import virtinst
-from virtinst import VirtualDisk
 from virtinst import Interface
 
 from tests import utils
@@ -102,29 +101,6 @@ args = {
 },
 
 
-'disk' : {
-  'init_conns' : [testconn, None],
-  '__init__' : {
-
-   'invalid' : [
-    {'path' : 'valid', 'size' : 1, 'driverCache' : 'invalid'},
-    {'conn' : testconn, "path" : "/full-pool/newvol.img", "size" : 1,
-      'sparse' : False},
-    # Inactive pool w/ volume
-    {'conn' : testconn, "path" : "/inactive-pool/inactive-vol"},
- ],
-
-   'valid' : [
-    {'conn' : testconn, 'path' : "/default-pool/default-vol"},
-    {'conn' : testconn, 'path' : "/default-pool/vol-noexist", 'size' : 1},
-    {'conn' : testconn, 'volInstall': volinst},
-    # Full pool, but we are nonsparse
-    {'conn' : testconn, "path" : "/full-pool/newvol.img", "size" : 1},
- ]
-},
-
-},
-
 'network'   : {
     'init_conns' : [testconn],
     '__init__'  : {
@@ -323,6 +299,8 @@ class TestValidation(unittest.TestCase):
                 self._runObjInit(testclass, paramvalue)
             else:
                 setattr(obj, paramname, paramvalue)
+                if hasattr(obj, "validate"):
+                    obj.validate()
 
             msg = ("Expected TypeError or ValueError: None Raised.\n"
                    "For '%s' object, paramname '%s', val '%s':" %
@@ -353,6 +331,8 @@ class TestValidation(unittest.TestCase):
                     self._runObjInit(testclass, paramvalue)
             else:
                 setattr(obj, paramname, paramvalue)
+            if hasattr(obj, "validate"):
+                obj.validate()
         except Exception, e:
             msg = ("Validation case failed, expected success.\n" +
                    "Exception received was: %s\n" % e +
@@ -390,10 +370,6 @@ class TestValidation(unittest.TestCase):
         g = virtinst.Guest(testconn, type="xen")
         self._testArgs(g, virtinst.Guest, 'guest')
 
-    def testDiskValidation(self):
-        disk = VirtualDisk(testconn, "/dev/loop0")
-        self._testArgs(disk, VirtualDisk, 'disk')
-
     def testNetworkValidation(self):
         network = virtinst.VirtualNetworkInterface(testconn)
         self._testArgs(network, virtinst.VirtualNetworkInterface, 'network')

tests/xmlconfig-xml/boot-many-disks2.xml

@@ -33,7 +33,7 @@
       <readonly/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu" type="qcow2"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdd" bus="ide"/>
     </disk>

tests/xmlconfig-xml/install-f10.xml

@@ -24,7 +24,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/install-f11-ac97.xml

@@ -28,7 +28,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/install-f11-noac97.xml

@@ -28,7 +28,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/install-f11-qemu.xml

@@ -28,7 +28,7 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdb" bus="ide"/>
     </disk>

tests/xmlconfig-xml/install-f11.xml

@@ -28,7 +28,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/misc-qemu-driver-name.xml

@@ -20,12 +20,12 @@
   <devices>
     <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/misc-qemu-driver-overwrite.xml

@@ -25,7 +25,7 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/misc-qemu-driver-type.xml

@@ -30,7 +30,7 @@
       <target dev="hdb" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/misc-qemu-iso-disk.xml

@@ -25,7 +25,7 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/rhel6-kvm-stage1.xml

@@ -30,7 +30,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/rhel6-kvm-stage2.xml

@@ -28,7 +28,7 @@
       <target dev="vda" bus="virtio"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="vdb" bus="virtio"/>
     </disk>

tests/xmlconfig-xml/winxp-kvm-stage1.xml

@@ -24,12 +24,12 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdb" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/winxp-kvm-stage2.xml

@@ -23,12 +23,12 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdb" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig-xml/winxp-kvm-stage3.xml

@@ -23,12 +23,12 @@
       <target dev="hda" bus="ide"/>
     </disk>
     <disk type="block" device="disk">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdb" bus="ide"/>
     </disk>
     <disk type="block" device="cdrom">
-      <driver name="qemu"/>
+      <driver name="qemu" type="raw"/>
       <source dev="/dev/loop0"/>
       <target dev="hdc" bus="ide"/>
       <readonly/>

tests/xmlconfig.py

@@ -541,36 +541,56 @@ class TestXMLConfig(unittest.TestCase):
 
         g.add_device(utils.get_filedisk())
         g.add_device(utils.get_blkdisk())
-        g.add_device(VirtualDisk(g.conn, path="/dev/loop0",
+
-                                 device=VirtualDisk.DEVICE_CDROM,
+        d = VirtualDisk(g.conn)
-                                 driverType="raw"))
+        d.path = "/dev/loop0"
-        g.add_device(VirtualDisk(g.conn, path="/dev/loop0",
+        d.device = d.DEVICE_CDROM
-                                 device=VirtualDisk.DEVICE_DISK,
+        d.driver_type = "raw"
-                                 driverName="qemu", format="qcow2"))
+        d.validate()
-        g.add_device(VirtualDisk(g.conn, path=None,
-                                 device=VirtualDisk.DEVICE_CDROM,
-                                 bus="scsi"))
-        d = VirtualDisk(g.conn, path=None,
-                        device=VirtualDisk.DEVICE_FLOPPY)
-        d.iotune_tbs = 1
-        d.iotune_tis = 2
         g.add_device(d)
 
-        d = VirtualDisk(g.conn, path="/dev/loop0",
+        d = VirtualDisk(g.conn)
-                        device=VirtualDisk.DEVICE_FLOPPY,
+        d.path = "/dev/loop0"
-                        driverName="phy")
+        d.device = d.DEVICE_DISK
+        d.driver_name = "qemu"
+        d.validate()
+        g.add_device(d)
+
+        d = VirtualDisk(g.conn)
+        d.path = None
+        d.device = d.DEVICE_CDROM
+        d.bus = "scsi"
+        d.validate()
+        g.add_device(d)
+
+        d = VirtualDisk(g.conn)
+        d.path = None
+        d.device = d.DEVICE_FLOPPY
+        d.iotune_tbs = 1
+        d.iotune_tis = 2
+        d.validate()
+        g.add_device(d)
+
+        d = VirtualDisk(g.conn)
+        d.path = "/dev/loop0"
+        d.device = d.DEVICE_FLOPPY
+        d.driver_name = "phy"
         d.driver_cache = "none"
         d.iotune_rbs = 5555
         d.iotune_ris = 1234
         d.iotune_wbs = 3
         d.iotune_wis = 4
+        d.validate()
         g.add_device(d)
 
-        d = VirtualDisk(g.conn, path="/dev/loop0",
+        d = VirtualDisk(g.conn)
-                        bus="virtio", driverName="qemu",
+        d.path = "/dev/loop0"
-                        driverType="qcow2")
+        d.bus = "virtio"
+        d.driver_name = "qemu"
+        d.driver_type = "qcow2"
         d.driver_cache = "none"
         d.driver_io = "threads"
+        d.validate()
         g.add_device(d)
 
         self._compare(g, "boot-many-disks2", False)
@@ -701,14 +721,31 @@ class TestXMLConfig(unittest.TestCase):
         g.add_device(VirtualAudio(g.conn, "es1370"))
 
         # Disk devices
-        g.add_device(VirtualDisk(g.conn, path="/dev/loop0",
+        d = VirtualDisk(g.conn)
-                                 device=VirtualDisk.DEVICE_FLOPPY))
+        d.path = "/dev/loop0"
-        g.add_device(VirtualDisk(g.conn, path="/dev/loop0", bus="scsi"))
+        d.device = d.DEVICE_FLOPPY
-        g.add_device(VirtualDisk(g.conn, path="/tmp", device="floppy"))
+        d.validate()
-        d3 = VirtualDisk(g.conn, path="/default-pool/testvol1.img",
+        g.add_device(d)
-                         bus="scsi", driverName="qemu")
+
-        d3.address.type = "spapr-vio"
+        d = VirtualDisk(g.conn)
-        g.add_device(d3)
+        d.path = "/dev/loop0"
+        d.bus = "scsi"
+        d.validate()
+        g.add_device(d)
+
+        d = VirtualDisk(g.conn)
+        d.path = "/tmp"
+        d.device = d.DEVICE_FLOPPY
+        d.validate()
+        g.add_device(d)
+
+        d = VirtualDisk(g.conn)
+        d.path = "/default-pool/testvol1.img"
+        d.bus = "scsi"
+        d.driver_name = "qemu"
+        d.address.type = "spapr-vio"
+        d.validate()
+        g.add_device(d)
 
         # Controller devices
         c1 = VirtualController.get_class_for_type(VirtualController.CONTROLLER_TYPE_IDE)(g.conn)
@@ -863,9 +900,6 @@ class TestXMLConfig(unittest.TestCase):
 
         self._compare(g, "boot-usb2", False)
 
-    #
-    # Full Install tests: try to mimic virt-install as much as possible
-    #
 
     def testFullKVMRHEL6(self):
         utils.set_conn(_plainkvm)
@@ -874,7 +908,7 @@ class TestXMLConfig(unittest.TestCase):
                                   gtype="kvm")
         g = utils.get_basic_fullyvirt_guest("kvm", installer=i)
         g.add_device(utils.get_floppy())
-        g.add_device(utils.get_filedisk("/default-pool/rhel6.img"))
+        g.add_device(utils.get_filedisk("/default-pool/rhel6.img", fake=False))
         g.add_device(utils.get_blkdisk())
         g.add_device(utils.get_virtual_network())
         g.add_device(VirtualAudio(g.conn))
@@ -893,7 +927,7 @@ class TestXMLConfig(unittest.TestCase):
 
     def testFullKVMWinxp(self):
         utils.set_conn(_plainkvm)
-        g = utils.build_win_kvm("/default-pool/winxp.img")
+        g = utils.build_win_kvm("/default-pool/winxp.img", fake=False)
         self._testInstall(g, "winxp-kvm-stage1",
                           "winxp-kvm-stage3", "winxp-kvm-stage2")
 
@@ -906,8 +940,10 @@ class TestXMLConfig(unittest.TestCase):
         sizebytes = long(sizegigs * 1024L * 1024L * 1024L)
 
         for sparse in [True, False]:
-            disk = VirtualDisk(utils.get_conn(), path=path, size=sizegigs,
+            disk = VirtualDisk(utils.get_conn())
-                               sparse=sparse)
+            disk.path = path
+            disk.set_create_storage(size=sizegigs, sparse=sparse)
+            disk.validate()
             disk.setup()
 
             actualsize = long(os.path.getsize(path))
@@ -970,8 +1006,9 @@ class TestXMLConfig(unittest.TestCase):
                           conn, "16")
 
     def testManyVirtio(self):
-        d = VirtualDisk(utils.get_conn(), bus="virtio",
+        d = VirtualDisk(utils.get_conn())
-                        path="/default-pool/testvol1.img")
+        d.bus = "virtio"
+        d.path = "/default-pool/testvol1.img"
 
         targetlist = []
         for ignore in range(0, (26 * 2) + 1):

tests/xmlparse-xml/change-disk-out.xml

@@ -18,10 +18,10 @@
   <vcpu>5</vcpu>
   <devices>
     <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
-    <disk type="file" device="disk">
+    <disk type="block" device="disk">
-      <source file="/dev/loop0"/>
       <target dev="hda" bus="ide"/>
       <serial>frob</serial>
+      <source dev="/dev/loop0"/>
       <driver name="test" type="raw"/>
     </disk>
     <disk type="block" device="disk">

tests/xmlparse-xml/change-media-out.xml

@@ -15,10 +15,10 @@
       <driver name="qemu" type="qcow2"/>
       <source file="/default-pool/default-vol"/>
     </disk>
-    <disk type="file" device="cdrom">
+    <disk type="block" device="cdrom">
       <target dev="hdb" bus="ide"/>
-      <driver name="qemu" type="qcow2"/>
+      <driver name="qemu" type="raw"/>
-      <source file="/disk-pool/diskvol1"/>
+      <source dev="/disk-pool/diskvol1"/>
     </disk>
     <disk type="block" device="cdrom">
       <target dev="hdc" bus="ide"/>

tests/xmlparse.py

@@ -764,11 +764,18 @@ class XMLParseTest(unittest.TestCase):
         # test000ClearProps resets the 'set' list, and this test
         # ensures that every property we know about has been touched
         # by one of the above tests.
-
         from virtinst import XMLBuilderDomain
         fail = [p for p in XMLBuilderDomain._allprops
                 if p not in XMLBuilderDomain._seenprops]
-        self.assertEquals(fail, [])
+        try:
+            self.assertEquals([], fail)
+        except AssertionError:
+            msg = "".join(traceback.format_exc()) + "\n\n"
+            msg += ("This means that there are XML properties that are\n"
+                    "untested in tests/xmlparse.py. This could be caused\n"
+                    "by a previous test suite failure, or if you added\n"
+                    "a new property and didn't add corresponding tests!")
+            self.fail(msg)
 
 if __name__ == "__main__":
     unittest.main()

virt-install

@@ -164,7 +164,7 @@ def get_disk(diskopts, size, sparse, guest, is_file_path):
         else:
             dev, size = cli.parse_disk(guest, diskopts)
             path = dev.path
-            sparse = dev.sparse
+            sparse = dev.get_sparse()
 
         d = cli.disk_prompt(guest.conn, path, size, sparse,
                             origdev=dev)

virtManager/addhardware.py

@@ -1304,29 +1304,30 @@ class vmmAddHardware(vmmGObjectUI):
                     if do_use:
                         diskpath = ideal
 
-            disk = virtinst.VirtualDisk(conn,
+            disk = virtinst.VirtualDisk(conn)
-                                        path=diskpath,
+            disk.path = diskpath
-                                        size=disksize,
+            disk.read_only = readonly
-                                        sparse=sparse,
+            disk.device = device
-                                        readOnly=readonly,
+            disk.bus = bus
-                                        device=device,
+            disk.set_create_storage(size=disksize, sparse=sparse,
-                                        bus=bus,
+                                    fmt=fmt or None)
-                                        format=fmt)
             disk.driver_cache = cache
 
             if not fmt:
                 fmt = self.config.get_storage_format()
                 if (self.is_default_storage() and
-                    disk.vol_install and
+                    disk.get_vol_install() and
-                    fmt in disk.vol_install.formats):
+                    fmt in disk.get_vol_install().formats):
                     logging.debug("Setting disk format from prefs: %s", fmt)
-                    disk.vol_install.format = fmt
+                    disk.get_vol_install().format = fmt
 
             if (disk.type == virtinst.VirtualDisk.TYPE_FILE and
                 not self.vm.is_hvm() and
                 virtinst.util.is_blktap_capable(self.conn.get_backend())):
                 disk.driver_name = virtinst.VirtualDisk.DRIVER_TAP
 
+            disk.validate()
+
         except Exception, e:
             return self.err.val_err(_("Storage parameter error."), e)
 

virtManager/create.py

@@ -843,7 +843,7 @@ class vmmCreate(vmmGObjectUI):
         disks = self.guest.get_devices("disk")
         if disks:
             disk = disks[0]
-            storage = "%s" % self.pretty_storage(disk.size)
+            storage = "%s" % self.pretty_storage(disk.get_size())
             storage += " " + (storagetmpl % disk.path)
         elif len(self.guest.get_devices("filesystem")):
             fs = self.guest.get_devices("filesystem")[0]
@@ -1677,17 +1677,17 @@ class vmmCreate(vmmGObjectUI):
             if not diskpath:
                 return self.err.val_err(_("A storage path must be specified."))
 
-            disk = virtinst.VirtualDisk(conn,
+            disk = virtinst.VirtualDisk(conn)
-                                        path=diskpath,
+            disk.path = diskpath
-                                        size=disksize,
+            disk.set_create_storage(size=disksize, sparse=sparse)
-                                        sparse=sparse)
 
             fmt = self.config.get_storage_format()
             if (self.is_default_storage() and
-                disk.vol_install and
+                disk.get_vol_install() and
-                fmt in disk.vol_install.formats):
+                fmt in disk.get_vol_install().formats):
                 logging.debug("Setting disk format from prefs: %s", fmt)
-                disk.vol_install.format = fmt
+                disk.get_vol_install().format = fmt
+            disk.validate()
 
         except Exception, e:
             return self.err.val_err(_("Storage parameter error."), e)

virtManager/domain.py

@@ -699,8 +699,8 @@ class vmmDomain(vmmLibvirtObject):
             editdev.iotune_wis = val
         return self._redefine_device(change, devobj)
 
-    # Network define methods
 
+    # Network define methods
     def define_network_source(self, devobj, newtype, newsource, newmode):
         def change(editdev):
             if not newtype:

virtManager/uihelpers.py

@@ -829,6 +829,8 @@ def mediadev_added(ignore_helper, newobj, widget, devtype):
 
     if newobj.get_media_type() != devtype:
         return
+    if model is None:
+        return
 
     if len(model) == 1 and model[0][OPTICAL_IS_VALID] is False:
         # Only entry is the 'No device' entry

virtinst/CloneManager.py

@@ -132,10 +132,17 @@ class Cloner(object):
                 if not path:
                     device = VirtualDisk.DEVICE_CDROM
 
-                disk = VirtualDisk(self.conn, path, size=.0000001,
+                disk = VirtualDisk(self.conn)
-                                   device=device)
+                disk.path = path
+                disk.device = device
+
+                # We fake storage creation params for now, but we will
+                # update it later
+                disk.set_create_storage(fake=True)
+                disk.validate()
                 disklist.append(disk)
             except Exception, e:
+                logging.debug("Error setting clone path.", exc_info=True)
                 raise ValueError(_("Could not use path '%s' for cloning: %s") %
                                  (path, str(e)))
 
@@ -264,8 +271,7 @@ class Cloner(object):
 
         logging.debug("Original XML:\n%s", self.original_xml)
 
-        self._guest = Guest(self.conn,
+        self._guest = Guest(self.conn, parsexml=self.original_xml)
-                                  parsexml=self.original_xml)
         self._guest.replace = self.replace
 
         # Pull clonable storage info from the original xml
@@ -274,7 +280,7 @@ class Cloner(object):
         logging.debug("Original paths: %s",
                       [d.path for d in self.original_disks])
         logging.debug("Original sizes: %s",
-                      [d.size for d in self.original_disks])
+                      [d.get_size() for d in self.original_disks])
 
         # If domain has devices to clone, it must be 'off' or 'paused'
         if (not self.clone_running and
@@ -293,7 +299,7 @@ class Cloner(object):
         if self.preserve_dest_disks:
             return
 
-        if clone_disk.vol_object:
+        if clone_disk.get_vol_object():
             # XXX We could always do this with vol upload?
 
             # Special case: non remote cloning of a guest using
@@ -312,27 +318,34 @@ class Cloner(object):
                       "currently supported: '%s'") % clone_disk.path)
 
         # Sync 'size' between the two
-        if orig_disk.size:
+        size = orig_disk.get_size()
-            clone_disk.size = orig_disk.size
+        vol_install = None
+        clone_path = None
 
         # Setup proper cloning inputs for the new virtual disks
-        if orig_disk.vol_object and clone_disk.vol_install:
+        if (orig_disk.get_vol_object() and
+            clone_disk.get_vol_install()):
+            clone_vol_install = clone_disk.get_vol_install()
 
             # Source and dest are managed. If they share the same pool,
             # replace vol_install with a CloneVolume instance, otherwise
             # simply set input_vol on the dest vol_install
-            if (clone_disk.vol_install.pool.name() ==
+            if (clone_vol_install.pool.name() ==
-                orig_disk.vol_object.storagePoolLookupByVolume().name()):
+                orig_disk.get_vol_object().storagePoolLookupByVolume().name()):
-                newname = clone_disk.vol_install.name
+                newname = clone_vol_install.name
-                clone_disk.vol_install = Storage.CloneVolume(self.conn,
+                vol_install = Storage.CloneVolume(self.conn,
-                                                    newname,
+                                                  newname,
-                                                    orig_disk.vol_object)
+                                                  orig_disk.get_vol_object())
 
             else:
-                clone_disk.vol_install.input_vol = orig_disk.vol_object
+                clone_vol_install.input_vol = orig_disk.get_vol_object()
-
+                vol_install = clone_vol_install
         else:
-            clone_disk.clone_path = orig_disk.path
+            clone_path = orig_disk.path
+
+        clone_disk.set_create_storage(
+                size=size, vol_install=vol_install, clone_path=clone_path)
+        clone_disk.validate()
 
 
     def setup_clone(self):
@@ -382,8 +395,9 @@ class Cloner(object):
             # Change the XML
             xmldisk.path = None
             xmldisk.type = clone_disk.type
-            xmldisk.path = clone_disk.path
+            xmldisk.driver_name = orig_disk.driver_name
             xmldisk.driver_type = orig_disk.driver_type
+            xmldisk.path = clone_disk.path
 
         # Save altered clone xml
         self._clone_xml = self._guest.get_xml_config()
@@ -405,7 +419,6 @@ class Cloner(object):
         Actually perform the duplication: cloning disks if needed and defining
         the new clone xml.
         """
-
         logging.debug("Starting duplicate.")
 
         if not meter:
@@ -421,19 +434,7 @@ class Cloner(object):
 
             if self.preserve:
                 for dst_dev in self.clone_disks:
-                    if dst_dev.clone_path == "/dev/null":
-                        # Not really sure why this check is here,
-                        # but keeping for compat
-                        logging.debug("Source dev was /dev/null. Skipping")
-                        continue
-                    elif dst_dev.clone_path == dst_dev.path:
-                        logging.debug("Source and destination are the "
-                                      "same. Skipping.")
-                        continue
-
-                    # VirtualDisk.setup handles everything
                     dst_dev.setup(meter=meter)
-
         except Exception, e:
             logging.debug("Duplicate failed: %s", str(e))
             if dom:
@@ -513,25 +514,26 @@ class Cloner(object):
             validate = not self.preserve_dest_disks
 
             try:
-                if (disk.path and validate and
-                    not VirtualDisk.path_exists(self.conn, disk.path)):
-                    raise ValueError(_("Disk '%s' does not exist.") %
-                                     disk.path)
-
                 device = VirtualDisk.DEVICE_DISK
                 if not disk.path:
                     # Tell VirtualDisk we are a cdrom to allow empty media
                     device = VirtualDisk.DEVICE_CDROM
 
-                d = VirtualDisk(self.conn, disk.path,
+                newd = VirtualDisk(self.conn)
-                                device=device, driverType=disk.driver_type,
+                newd.path = disk.path
-                                validate=validate)
+                newd.device = device
-                d.target = disk.target
+                newd.driver_type = disk.driver_type
+                newd.target = disk.target
+                if validate:
+                    newd.set_create_storage(fake=True)
+                    if newd.creating_storage() and disk.path is not None:
+                        raise ValueError("Disk path '%s' does not exist." %
+                                         newd.path)
             except Exception, e:
                 logging.debug("", exc_info=True)
                 raise ValueError(_("Could not determine original disk "
                                    "information: %s" % str(e)))
-            retdisks.append(d)
+            retdisks.append(newd)
 
         return retdisks
 

virtinst/DistroInstaller.py

@@ -107,13 +107,14 @@ def _upload_file(conn, meter, destpool, src):
     if name != basename:
         logging.debug("Generated non-colliding volume name %s", name)
 
-    disk = VirtualDisk(conn,
+    disk = VirtualDisk(conn)
-                       path=os.path.join(poolpath, name),
+    disk.path = os.path.join(poolpath, name)
-                       sizebytes=size,
+    disk.set_create_storage(size=(float(size) / 1024.0 / 1024.0 / 1024.0),
-                       sparse=True)
+                            sparse=True)
+    disk.validate()
 
     disk.setup(meter=meter)
-    vol = disk.vol_object
+    vol = disk.get_vol_object()
     if not vol:
         raise RuntimeError(_("Failed to lookup scratch media volume"))
 
@@ -217,11 +218,7 @@ class DistroInstaller(Installer.Installer):
             # Pass the parameters off to VirtualDisk to validate, and pull
             # out the path
             try:
-                d = VirtualDisk(self.conn,
+                d = self._make_cdrom_dev(val)
-                                path=val,
-                                device=VirtualDisk.DEVICE_CDROM,
-                                transient=True,
-                                readOnly=True)
                 val = d.path
             except:
                 logging.debug("Error validating install location",
@@ -255,11 +252,8 @@ class DistroInstaller(Installer.Installer):
         else:
             cdrom = self.location
 
-        disk = VirtualDisk(guest.conn,
+        disk = self._make_cdrom_dev(cdrom)
-                           path=cdrom,
+        disk.transient = transient
-                           device=VirtualDisk.DEVICE_CDROM,
-                           readOnly=True,
-                           transient=transient)
         self.install_devices.append(disk)
 
     def _perform_initrd_injections(self, initrd):
@@ -340,26 +334,13 @@ class DistroInstaller(Installer.Installer):
     def _prepare_kernel_and_initrd(self, guest, meter):
         disk = None
 
-        # If installing off a local path, map it through to a virtual CD/disk
+        # If installing off a local path, map it through to a virtual CD
         if (self.location is not None and
             self._location_is_path and
             not os.path.isdir(self.location)):
 
-            device = VirtualDisk.DEVICE_CDROM
+            disk = self._make_cdrom_dev(self.location)
-
+            disk.transient = True
-            # pylint: disable=W0212
-            # Access to protected member lookup_osdict_key
-            can_cdrom = guest._lookup_osdict_key('pv_cdrom_install')
-            # pylint: enable=W0212
-
-            if self.is_xenpv() and can_cdrom:
-                device = VirtualDisk.DEVICE_DISK
-
-            disk = VirtualDisk(guest.conn,
-                               device=device,
-                               path=self.location,
-                               readOnly=True,
-                               transient=True)
 
         # Make sure we always fetch kernel here if required
         if self._install_bootconfig.kernel and not self.scratchdir_required():

virtinst/ImageInstaller.py

@@ -120,14 +120,13 @@ class ImageInstaller(Installer.Installer):
             if drive.disk.format == ImageParser.Disk.FORMAT_ISO:
                 device = VirtualDisk.DEVICE_CDROM
 
-
+            disk = VirtualDisk(self.conn)
-            disk = VirtualDisk(conn=self.conn,
+            disk.path = path
-                               path=path,
+            disk.device = device
-                               size=size,
-                               device=device,
-                               format=drive.disk.format)
             disk.target = drive.target
 
+            disk.set_create_storage(size=size, fmt=drive.disk.format)
+            disk.validate()
             self.install_devices.append(disk)
 
     def _abspath(self, p):

virtinst/Installer.py

@@ -271,6 +271,14 @@ class Installer(XMLBuilderDomain.XMLBuilderDomain):
                 return "/sbin/init"
         return "/bin/sh"
 
+    def _make_cdrom_dev(self, path):
+        dev = virtinst.VirtualDisk(self.conn)
+        dev.path = path
+        dev.device = dev.DEVICE_CDROM
+        dev.read_only = True
+        dev.validate()
+        return dev
+
     def _get_osblob_helper(self, guest, isinstall, bootconfig):
         arch = self.arch
         machine = self.machine

virtinst/LiveCDInstaller.py

@@ -20,20 +20,17 @@
 # MA 02110-1301 USA.
 
 from virtinst import Installer
-from virtinst.VirtualDisk import VirtualDisk
 
 
 class LiveCDInstaller(Installer.Installer):
     _has_install_phase = False
 
+
     # LiveCD specific methods/overwrites
     def _validate_location(self, val):
         if not val:
             return None
-        return VirtualDisk(self.conn,
+        return self._make_cdrom_dev(val)
-                           path=val,
-                           device=VirtualDisk.DEVICE_CDROM,
-                           readOnly=True)
     def _get_location(self):
         return self._location
     def _set_location(self, val):

virtinst/Storage.py

@@ -1076,7 +1076,7 @@ class StorageVolume(StorageObject):
     def get_capacity(self):
         return self._capacity
     def set_capacity(self, val):
-        if type(val) not in (int, float, long) or val <= 0:
+        if type(val) not in (int, float, long) or val < 0:
             raise ValueError(_("Capacity must be a positive number"))
         newcap = int(val)
         origcap = self.capacity

virtinst/XMLBuilderDomain.py

@@ -227,7 +227,7 @@ def _xml_property(fget=None, fset=None, doc=None,
                   xpath=None, get_converter=None, set_converter=None,
                   xml_get_xpath=None, xml_set_xpath=None,
                   xml_set_list=None, is_bool=False, is_multi=False,
-                  default_converter=None):
+                  default_converter=None, clear_first=None):
     """
     Set a XMLBuilder class property that represents a value in the
     <domain> XML. For example
@@ -260,6 +260,9 @@ def _xml_property(fget=None, fset=None, doc=None,
     @param is_multi: Whether data is coming multiple or a single node
     @param default_converter: If the virtinst value is "default", use
                               this function to get the actual XML value
+    @param clear_first: List of xpaths to unset before any 'set' operation.
+        For those weird interdependent XML props like disk source type and
+        path attribute.
     """
     # pylint: disable=W0212
     # Accessing _xml vals of self. This should be a class method, but
@@ -352,13 +355,24 @@ def _xml_property(fget=None, fset=None, doc=None,
             return
 
         nodes = util.listify(_get_xpath_node(self._xml_ctx,
-                                              node_xpath, is_multi))
+                                             node_xpath, is_multi))
+        clear_nodes = []
+        for cpath in clear_first or []:
+            cnode = _get_xpath_node(self._xml_ctx, cpath, False)
+            if not cnode:
+                continue
+            if any([(n and n.nodePath() == cnode.nodePath()) for n in nodes]):
+                continue
+            clear_nodes.append(cnode)
 
         xpath_list = node_xpath
         if xml_set_list:
             xpath_list = xml_set_list(self)
 
-        node_map = _tuplify_lists(nodes, val, xpath_list)
+        node_map = []
+        if clear_nodes:
+            node_map += _tuplify_lists(clear_nodes, None, "")
+        node_map += _tuplify_lists(nodes, val, xpath_list)
         for node, val, use_xpath in node_map:
             if node:
                 use_xpath = node.nodePath()

virtinst/cli.py

@@ -465,8 +465,8 @@ def disk_prompt(conn, origpath, origsize, origsparse,
     retry_path = True
 
     no_path_needed = (origdev and
-                      (origdev.vol_install or
+                      (origdev.get_vol_install() or
-                       origdev.vol_object or
+                       origdev.get_vol_object() or
                        origdev.can_be_empty()))
 
     def prompt_path(chkpath, chksize):
@@ -578,13 +578,15 @@ def disk_prompt(conn, origpath, origsize, origsparse,
         try:
             if origdev:
                 dev = origdev
-                if path is not None:
+                if path is not None and path != dev.path:
                     dev.path = path
-                if size is not None:
+                if size is not None and size != dev.get_size():
-                    dev.size = size
+                    dev.set_create_storage(size=size, sparse=origsparse)
             else:
-                dev = VirtualDisk(conn=conn, path=path, size=size,
+                dev = VirtualDisk(conn)
-                                  sparse=origsparse)
+                dev.path = path
+                dev.set_create_storage(size=size, sparse=origsparse)
+            dev.validate()
         except ValueError, e:
             if is_prompt():
                 logging.error(e)
@@ -1485,11 +1487,12 @@ def parse_disk(guest, optstr, dev=None):
 
         return val
 
+    if not dev:
+        dev = virtinst.VirtualDisk(guest.conn)
+
     # Parse out comma separated options
     opts = parse_optstr(optstr, remove_first="path")
 
-    # We annoyingly need these params ahead of time to deal with
-    # VirtualDisk validation
     path = opt_get("path")
     pool = opt_get("pool")
     vol = opt_get("vol")
@@ -1497,36 +1500,22 @@ def parse_disk(guest, optstr, dev=None):
     fmt = opt_get("format")
     sparse = parse_sparse(opt_get("sparse"))
     ro, shared = parse_perms(opt_get("perms"))
-    device = opt_get("device")
 
     abspath, volinst, volobj = _parse_disk_source(guest, path, pool, vol,
                                                   size, fmt, sparse)
 
-    if not dev:
+    if volobj:
-        # Build a stub device that should always validate cleanly
+        dev.set_vol_object(volobj)
-        dev = virtinst.VirtualDisk(conn=guest.conn,
+    else:
-                                   path=abspath,
+        dev.path = abspath
-                                   volObject=volobj,
+    dev.read_only = ro
-                                   volInstall=volinst,
+    dev.shareable = shared
-                                   size=size,
+    dev.set_create_storage(size=size, fmt=fmt, sparse=sparse,
-                                   readOnly=ro,
+                           vol_install=volinst)
-                                   sparse=sparse,
-                                   shareable=shared,
-                                   device=device,
-                                   format=fmt)
 
     set_param = _build_set_param(dev, opts)
 
-    set_param("path", "path", abspath)
+    set_param("device", "device")
-    set_param("vol", "vol_object", volobj)
-    set_param("pool", "vol_install", volinst)
-    set_param("size", "size", size)
-    set_param("format", "format", fmt)
-    set_param("sparse", "sparse", sparse)
-    set_param("read_only", "perms", ro)
-    set_param("shareable", "perms", shared)
-    set_param("device", "device", device)
-
     set_param("bus", "bus")
     set_param("driver_cache", "cache")
     set_param("driver_name", "driver_name")
@@ -1538,13 +1527,14 @@ def parse_disk(guest, optstr, dev=None):
     if opts:
         fail(_("Unknown options %s") % opts.keys())
 
+    dev.validate()
     return dev, size
 
+
 #####################
 # --network parsing #
 #####################
 
-
 def parse_network(guest, optstring, dev=None, mac=None):
     # Handle old format of bridge:foo instead of bridge=foo
     for prefix in ["network", "bridge"]:

virtinst/diskbackend.py

@@ -0,0 +1,569 @@
+#
+# Storage lookup/creation helpers
+#
+# Copyright 2013  Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free  Software Foundation; either version 2 of the License, or
+# (at your option)  any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301 USA.
+
+import logging
+import os
+import statvfs
+
+import libvirt
+
+from virtinst import Storage
+from virtinst import util
+
+
+def _check_if_pool_source(conn, path):
+    """
+    If passed path is a host disk device like /dev/sda, want to let the user
+    use it
+    """
+    if not conn.check_conn_support(conn.SUPPORT_CONN_STORAGE):
+        return None
+
+    def check_pool(poolname, path):
+        pool = conn.storagePoolLookupByName(poolname)
+        xml = pool.XMLDesc(0)
+
+        for element in ["dir", "device", "adapter"]:
+            xml_path = util.xpath(xml, "/pool/source/%s/@path" % element)
+            if xml_path == path:
+                return pool
+
+    running_list = conn.listStoragePools()
+    inactive_list = conn.listDefinedStoragePools()
+    for plist in [running_list, inactive_list]:
+        for name in plist:
+            p = check_pool(name, path)
+            if p:
+                return p
+    return None
+
+
+def check_if_path_managed(conn, path):
+    """
+    Determine if we can use libvirt storage APIs to create or lookup
+    the passed path. If we can't, throw an error
+    """
+    vol = None
+    pool = None
+    verr = None
+    path_is_pool = False
+
+    def lookup_vol_by_path():
+        try:
+            vol = conn.storageVolLookupByPath(path)
+            vol.info()
+            return vol, None
+        except libvirt.libvirtError, e:
+            if (hasattr(libvirt, "VIR_ERR_NO_STORAGE_VOL")
+                and e.get_error_code() != libvirt.VIR_ERR_NO_STORAGE_VOL):
+                raise
+            return None, e
+
+    def lookup_vol_name(name):
+        try:
+            name = os.path.basename(path)
+            if pool and name in pool.listVolumes():
+                return pool.lookupByName(name)
+        except:
+            pass
+        return None
+
+    vol = lookup_vol_by_path()[0]
+    if not vol:
+        pool = util.lookup_pool_by_path(conn, os.path.dirname(path))
+
+        # Is pool running?
+        if pool and pool.info()[0] != libvirt.VIR_STORAGE_POOL_RUNNING:
+            pool = None
+
+    # Attempt to lookup path as a storage volume
+    if pool and not vol:
+        try:
+            # Pool may need to be refreshed, but if it errors,
+            # invalidate it
+            pool.refresh(0)
+            vol, verr = lookup_vol_by_path()
+            if verr:
+                vol = lookup_vol_name(os.path.basename(path))
+        except Exception, e:
+            vol = None
+            pool = None
+            verr = str(e)
+
+    if not vol:
+        # See if path is a pool source, and allow it through
+        trypool = _check_if_pool_source(conn, path)
+        if trypool:
+            path_is_pool = True
+            pool = trypool
+
+    if not vol and not pool:
+        if not conn.is_remote():
+            # Building local disk
+            return None, None, False
+
+        if not verr:
+            # Since there is no error, no pool was ever found
+            err = (_("Cannot use storage '%(path)s': '%(rootdir)s' is "
+                     "not managed on the remote host.") %
+                      {'path' : path,
+                        'rootdir' : os.path.dirname(path)})
+        else:
+            err = (_("Cannot use storage %(path)s: %(err)s") %
+                    {'path' : path, 'err' : verr})
+
+        raise ValueError(err)
+
+    return vol, pool, path_is_pool
+
+
+def _build_vol_install(conn, path, pool, size, sparse):
+    # Path wasn't a volume. See if base of path is a managed
+    # pool, and if so, setup a StorageVolume object
+    if size is None:
+        raise ValueError(_("Size must be specified for non "
+                           "existent volume path '%s'" % path))
+
+    logging.debug("Path '%s' is target for pool '%s'. "
+                  "Creating volume '%s'.",
+                  os.path.dirname(path), pool.name(),
+                  os.path.basename(path))
+
+    volclass = Storage.StorageVolume.get_volume_for_pool(pool_object=pool)
+    cap = (size * 1024 * 1024 * 1024)
+    if sparse:
+        alloc = 0
+    else:
+        alloc = cap
+
+    volinst = volclass(conn, name=os.path.basename(path),
+                       capacity=cap, allocation=alloc, pool=pool)
+    return volinst
+
+
+
+class _StorageBase(object):
+    def get_size(self):
+        raise NotImplementedError()
+    def get_dev_type(self):
+        raise NotImplementedError()
+    def is_managed(self):
+        raise NotImplementedError()
+    def get_driver_type(self):
+        raise NotImplementedError()
+
+
+class StorageCreator(_StorageBase):
+    def __init__(self, conn, path, pool,
+                 vol_install, clone_path,
+                 size, sparse, fmt):
+        _StorageBase.__init__(self)
+
+        self._conn = conn
+        self._pool = pool
+        self._vol_install = vol_install
+        self._path = path
+        self._size = size
+        self._sparse = sparse
+        self._clone_path = clone_path
+        self.fake = False
+
+        if not self._vol_install and self._pool:
+            self._vol_install = _build_vol_install(conn, path, pool,
+                                                   size, sparse)
+        self._set_format(fmt)
+        if self._vol_install:
+            self._path = None
+            self._size = None
+
+        # Cached bits
+        self._dev_type = None
+
+
+    ###############
+    # Private API #
+    ###############
+
+    def _set_format(self, val):
+        if val is None:
+            return
+
+        if self._vol_install:
+            if not hasattr(self._vol_install, "format"):
+                raise ValueError(_("Storage type does not support format "
+                                   "parameter."))
+            if getattr(self._vol_install, "format", None) != val:
+                setattr(self._vol_install, "format", val)
+
+        elif val != "raw":
+            raise RuntimeError(_("Format cannot be specified for "
+                                 "unmanaged storage."))
+
+
+    ##############
+    # Public API #
+    ##############
+
+    def _get_path(self):
+        if self._vol_install and not self._path:
+            self._path = (util.xpath(self._vol_install.pool.XMLDesc(0),
+                                     "/pool/target/path") + "/" +
+                                     self._vol_install.name)
+        return self._path
+    path = property(_get_path)
+
+    def get_vol_install(self):
+        return self._vol_install
+    def get_sparse(self):
+        return self._sparse
+
+    def get_size(self):
+        if not self._size:
+            self._size = (float(self._vol_install.capacity) /
+                          1024.0 / 1024.0 / 1024.0)
+        return self._size
+
+    def get_dev_type(self):
+        if not self._dev_type:
+            if self._vol_install:
+                if self._vol_install.file_type == libvirt.VIR_STORAGE_VOL_FILE:
+                    self._dev_type = "file"
+                else:
+                    self._dev_type = "block"
+            else:
+                self._dev_type = "file"
+        return self._dev_type
+
+    def get_driver_type(self):
+        if self._vol_install:
+            if hasattr(self._vol_install, "format"):
+                return self._vol_install.format
+        return "raw"
+
+    def is_managed(self):
+        return bool(self._vol_install)
+
+    def validate(self, device, devtype):
+        if device in ["floppy", "cdrom"]:
+            raise ValueError(_("Cannot create storage for %s device.") %
+                               device)
+
+        if self.is_managed():
+            return
+
+        if devtype == "block":
+            raise ValueError(_("Local block device path '%s' must "
+                               "exist.") % self.path)
+        if not os.access(os.path.dirname(self.path), os.R_OK):
+            raise ValueError("No read access to directory '%s'" %
+                             os.path.dirname(self.path))
+        if self._size is None:
+            raise ValueError(_("size is required for non-existent disk "
+                               "'%s'" % self.path))
+        if not os.access(os.path.dirname(self.path), os.W_OK):
+            raise ValueError(_("No write access to directory '%s'") %
+                               os.path.dirname(self.path))
+
+    def is_size_conflict(self):
+        if self._vol_install:
+            return self._vol_install.is_size_conflict()
+
+        ret = False
+        msg = None
+        vfs = os.statvfs(os.path.dirname(self._path))
+        avail = vfs[statvfs.F_FRSIZE] * vfs[statvfs.F_BAVAIL]
+        need = long(self._size * 1024L * 1024L * 1024L)
+        if need > avail:
+            if self._sparse:
+                msg = _("The filesystem will not have enough free space"
+                        " to fully allocate the sparse file when the guest"
+                        " is running.")
+            else:
+                ret = True
+                msg = _("There is not enough free space to create the disk.")
+
+
+            if msg:
+                msg += (_(" %d M requested > %d M available") %
+                        ((need / (1024 * 1024)), (avail / (1024 * 1024))))
+        return (ret, msg)
+
+
+    #############################
+    # Storage creation routines #
+    #############################
+
+    def create(self, progresscb):
+        if self.fake:
+            raise RuntimeError("Storage creator is fake but creation "
+                               "requested.")
+        # If a clone_path is specified, but not vol_install.input_vol,
+        # that means we are cloning unmanaged -> managed, so skip this
+        if (self._vol_install and
+            (not self._clone_path or self._vol_install.input_vol)):
+            return self._vol_install.install(meter=progresscb)
+
+        if self._clone_path:
+            text = (_("Cloning %(srcfile)s") %
+                    {'srcfile' : os.path.basename(self._clone_path)})
+        else:
+            text = _("Creating storage file %s") % os.path.basename(self._path)
+
+        size_bytes = long(self._size * 1024L * 1024L * 1024L)
+        progresscb.start(filename=self._path, size=long(size_bytes),
+                         text=text)
+
+        if self._clone_path:
+            # Plain file clone
+            self._clone_local(progresscb, size_bytes)
+        else:
+            # Plain file creation
+            self._create_local_file(progresscb, size_bytes)
+
+    def _create_local_file(self, progresscb, size_bytes):
+        """
+        Helper function which attempts to build self.path
+        """
+        fd = None
+        path = self._path
+        sparse = self._sparse
+
+        try:
+            try:
+                fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_DSYNC)
+
+                if sparse:
+                    os.ftruncate(fd, size_bytes)
+                else:
+                    # 1 meg of nulls
+                    mb = 1024 * 1024
+                    buf = '\x00' * mb
+
+                    left = size_bytes
+                    while left > 0:
+                        if left < mb:
+                            buf = '\x00' * left
+                        left = max(left - mb, 0)
+
+                        os.write(fd, buf)
+                        progresscb.update(size_bytes - left)
+            except OSError, e:
+                raise RuntimeError(_("Error creating diskimage %s: %s") %
+                                   (path, str(e)))
+        finally:
+            if fd is not None:
+                os.close(fd)
+            progresscb.end(size_bytes)
+
+    def _clone_local(self, meter, size_bytes):
+        if self._clone_path == "/dev/null":
+            # Not really sure why this check is here,
+            # but keeping for compat
+            logging.debug("Source dev was /dev/null. Skipping")
+            return
+        if self._clone_path == self._path:
+            logging.debug("Source and destination are the same. Skipping.")
+            return
+
+        # if a destination file exists and sparse flg is True,
+        # this priority takes a existing file.
+
+        if (not os.path.exists(self._path) and self._sparse):
+            clone_block_size = 4096
+            sparse = True
+            fd = None
+            try:
+                fd = os.open(self._path, os.O_WRONLY | os.O_CREAT)
+                os.ftruncate(fd, size_bytes)
+            finally:
+                if fd:
+                    os.close(fd)
+        else:
+            clone_block_size = 1024 * 1024 * 10
+            sparse = False
+
+        logging.debug("Local Cloning %s to %s, sparse=%s, block_size=%s",
+                      self._clone_path, self._path, sparse, clone_block_size)
+
+        zeros = '\0' * 4096
+
+        src_fd, dst_fd = None, None
+        try:
+            try:
+                src_fd = os.open(self._clone_path, os.O_RDONLY)
+                dst_fd = os.open(self._path, os.O_WRONLY | os.O_CREAT)
+
+                i = 0
+                while 1:
+                    l = os.read(src_fd, clone_block_size)
+                    s = len(l)
+                    if s == 0:
+                        meter.end(size_bytes)
+                        break
+                    # check sequence of zeros
+                    if sparse and zeros == l:
+                        os.lseek(dst_fd, s, 1)
+                    else:
+                        b = os.write(dst_fd, l)
+                        if s != b:
+                            meter.end(i)
+                            break
+                    i += s
+                    if i < size_bytes:
+                        meter.update(i)
+            except OSError, e:
+                raise RuntimeError(_("Error cloning diskimage %s to %s: %s") %
+                                   (self._clone_path, self._path, str(e)))
+        finally:
+            if src_fd is not None:
+                os.close(src_fd)
+            if dst_fd is not None:
+                os.close(dst_fd)
+
+
+class StorageBackend(_StorageBase):
+    """
+    Class that carries all the info about any existing storage that
+    the disk references
+    """
+    def __init__(self, conn, path, vol_object, pool_object):
+        _StorageBase.__init__(self)
+
+        self._conn = conn
+        self._vol_object = vol_object
+        self._pool_object = pool_object
+        self._path = path
+
+        if self._vol_object is not None:
+            self._pool_object = None
+            self._path = None
+        elif self._pool_object is not None:
+            if self._path is None:
+                raise ValueError("path must be specified is backend is "
+                                 "pool object.")
+
+        # Cached bits
+        self._pool_xml = None
+        self._vol_xml = None
+        self._exists = None
+        self._size = None
+        self._dev_type = None
+
+
+    ################
+    # Internal API #
+    ################
+
+    def _get_pool_xml(self):
+        if self._pool_xml is None:
+            self._pool_xml = self._pool_object.XMLDesc(0)
+        return self._pool_xml
+
+    def _get_vol_xml(self):
+        if self._vol_xml is None:
+            self._vol_xml = self._vol_object.XMLDesc(0)
+        return self._vol_xml
+
+
+    ##############
+    # Public API #
+    ##############
+
+    def _get_path(self):
+        if self._vol_object:
+            return self._vol_object.path()
+        return self._path
+    path = property(_get_path)
+
+    def get_vol_object(self):
+        return self._vol_object
+
+    def get_size(self):
+        """
+        Return size of existing storage
+        """
+        if self._size is None:
+            ret = 0
+            if self._vol_object:
+                ret = util.xpath(self._get_vol_xml(), "/volume/capacity")
+            elif self._pool_object:
+                ret = util.xpath(self._get_pool_xml(), "/pool/capacity")
+            elif self._path:
+                ignore, ret = util.stat_disk(self.path)
+            self._size = (float(ret) / 1024.0 / 1024.0 / 1024.0)
+        return self._size
+
+    def exists(self):
+        if self._exists is None:
+            if self.path is None:
+                self._exists = True
+            elif self._vol_object or self._pool_object:
+                self._exists = True
+            elif not self._conn.is_remote() and os.path.exists(self._path):
+                self._exists = True
+            else:
+                self._exists = False
+        return self._exists
+
+    def get_dev_type(self):
+        """
+        Return disk 'type' value per storage settings
+        """
+        if self._dev_type is None:
+            if self._vol_object:
+                t = self._vol_object.info()[0]
+                if t == libvirt.VIR_STORAGE_VOL_FILE:
+                    self._dev_type = "file"
+                elif t == libvirt.VIR_STORAGE_VOL_BLOCK:
+                    self._dev_type = "block"
+                else:
+                    self._dev_type = "file"
+
+            elif self._pool_object:
+                xml = self._get_pool_xml()
+                for source, source_type in [
+                    ("dir", "dir"),
+                    ("device", "block"),
+                    ("adapter", "block")]:
+                    if util.xpath(xml, "/pool/source/%s/@dev" % source):
+                        self._dev_type = source_type
+                        break
+
+            elif self._path:
+                if os.path.isdir(self._path):
+                    self._dev_type = "dir"
+                elif util.stat_disk(self._path)[0]:
+                    self._dev_type = "file"
+                else:
+                    self._dev_type = "block"
+
+            if not self._dev_type:
+                self._dev_type = "block"
+        return self._dev_type
+
+    def get_driver_type(self):
+        if self._vol_object:
+            fmt = util.xpath(self._get_vol_xml(),
+                             "/volume/target/format/@type")
+            return fmt
+        return None
+
+    def is_managed(self):
+        return bool(self._vol_object or self._pool_object)

virtinst/osdict.py

@@ -73,7 +73,6 @@ DEFAULTS = {
     "continue":         False,
     "distro":           None,
     "label":            None,
-    "pv_cdrom_install": False,
     "supported":        False,
 
     "devices" : {
@@ -699,7 +698,6 @@ OS_TYPES = {
 "solaris": {
     "label": "Solaris",
     "clock": "localtime",
-    "pv_cdrom_install": True,
     "variants": {
 
     "solaris9": {
@@ -776,7 +774,6 @@ OS_TYPES = {
    },
     "netware6": {
         "label": "Novell Netware 6",
-        "pv_cdrom_install": True,
    },
 
     "generic": {