diff --git a/tests/cli-test-xml/compare/clone-auto1.xml b/tests/cli-test-xml/compare/clone-auto1.xml index a42306685..a84a147f1 100644 --- a/tests/cli-test-xml/compare/clone-auto1.xml +++ b/tests/cli-test-xml/compare/clone-auto1.xml @@ -20,10 +20,10 @@ /usr/lib/xen/bin/qemu-dm -
+ @@ -46,13 +46,13 @@
- +
- + diff --git a/tests/clone-xml/empty-disks-out.xml b/tests/clone-xml/empty-disks-out.xml index 2b768a65d..fbf079f61 100644 --- a/tests/clone-xml/empty-disks-out.xml +++ b/tests/clone-xml/empty-disks-out.xml @@ -19,7 +19,7 @@ /usr/bin/qemu-kvm - + diff --git a/tests/clone-xml/force-out.xml b/tests/clone-xml/force-out.xml index 656ae1ca9..1a44f3de9 100644 --- a/tests/clone-xml/force-out.xml +++ b/tests/clone-xml/force-out.xml @@ -17,9 +17,9 @@ destroy /usr/bin/qemu-kvm - + - + diff --git a/tests/clone-xml/general-cfg-out.xml b/tests/clone-xml/general-cfg-out.xml index 20508dc62..361ca749f 100644 --- a/tests/clone-xml/general-cfg-out.xml +++ b/tests/clone-xml/general-cfg-out.xml @@ -19,7 +19,7 @@ /usr/bin/qemu-kvm - + diff --git a/tests/clone-xml/managed-storage-in.xml b/tests/clone-xml/managed-storage-in.xml index 0f328eb66..616a8d885 100644 --- a/tests/clone-xml/managed-storage-in.xml +++ b/tests/clone-xml/managed-storage-in.xml @@ -18,6 +18,7 @@ /usr/bin/qemu-kvm + diff --git a/tests/clone-xml/managed-storage-out.xml b/tests/clone-xml/managed-storage-out.xml index 2b5c6f3db..0e1a9b0a8 100644 --- a/tests/clone-xml/managed-storage-out.xml +++ b/tests/clone-xml/managed-storage-out.xml @@ -19,11 +19,12 @@ /usr/bin/qemu-kvm + - + diff --git a/tests/clone-xml/readonly-disks-out.xml b/tests/clone-xml/readonly-disks-out.xml index f8d3ac401..19c22d31c 100644 --- a/tests/clone-xml/readonly-disks-out.xml +++ b/tests/clone-xml/readonly-disks-out.xml @@ -19,7 +19,7 @@ /usr/bin/qemu-kvm - + diff --git a/tests/clone-xml/skip-out.xml b/tests/clone-xml/skip-out.xml index ac68746e9..90319888f 100644 --- a/tests/clone-xml/skip-out.xml +++ b/tests/clone-xml/skip-out.xml @@ -28,9 +28,9 @@ - + - + diff --git a/tests/clonetest.py b/tests/clonetest.py index 0c776ac41..11370654d 100644 --- a/tests/clonetest.py +++ b/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.") diff --git a/tests/utils.py b/tests/utils.py index b2e29b008..e88da6aeb 100644 --- a/tests/utils.py +++ b/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(): diff --git a/tests/validation.py b/tests/validation.py index 79b4cf764..d01e47fef 100644 --- a/tests/validation.py +++ b/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') diff --git a/tests/xmlconfig-xml/boot-many-disks2.xml b/tests/xmlconfig-xml/boot-many-disks2.xml index 3dabf3cdd..213bf5256 100644 --- a/tests/xmlconfig-xml/boot-many-disks2.xml +++ b/tests/xmlconfig-xml/boot-many-disks2.xml @@ -33,7 +33,7 @@ - + diff --git a/tests/xmlconfig-xml/install-f10.xml b/tests/xmlconfig-xml/install-f10.xml index 2529fa338..eda3098d6 100644 --- a/tests/xmlconfig-xml/install-f10.xml +++ b/tests/xmlconfig-xml/install-f10.xml @@ -24,7 +24,7 @@ - + diff --git a/tests/xmlconfig-xml/install-f11-ac97.xml b/tests/xmlconfig-xml/install-f11-ac97.xml index d97e5d6f0..1c4cf3a1f 100644 --- a/tests/xmlconfig-xml/install-f11-ac97.xml +++ b/tests/xmlconfig-xml/install-f11-ac97.xml @@ -28,7 +28,7 @@ - + diff --git a/tests/xmlconfig-xml/install-f11-noac97.xml b/tests/xmlconfig-xml/install-f11-noac97.xml index ca7689c1a..0d150ec23 100644 --- a/tests/xmlconfig-xml/install-f11-noac97.xml +++ b/tests/xmlconfig-xml/install-f11-noac97.xml @@ -28,7 +28,7 @@ - + diff --git a/tests/xmlconfig-xml/install-f11-qemu.xml b/tests/xmlconfig-xml/install-f11-qemu.xml index 5be7f5433..34934b0d6 100644 --- a/tests/xmlconfig-xml/install-f11-qemu.xml +++ b/tests/xmlconfig-xml/install-f11-qemu.xml @@ -28,7 +28,7 @@ - + diff --git a/tests/xmlconfig-xml/install-f11.xml b/tests/xmlconfig-xml/install-f11.xml index d7d62abd1..5dc292ea6 100644 --- a/tests/xmlconfig-xml/install-f11.xml +++ b/tests/xmlconfig-xml/install-f11.xml @@ -28,7 +28,7 @@ - + diff --git a/tests/xmlconfig-xml/misc-qemu-driver-name.xml b/tests/xmlconfig-xml/misc-qemu-driver-name.xml index 2bb398fb5..b896a94a0 100644 --- a/tests/xmlconfig-xml/misc-qemu-driver-name.xml +++ b/tests/xmlconfig-xml/misc-qemu-driver-name.xml @@ -20,12 +20,12 @@ /usr/lib/xen/bin/qemu-dm - + - + diff --git a/tests/xmlconfig-xml/misc-qemu-driver-overwrite.xml b/tests/xmlconfig-xml/misc-qemu-driver-overwrite.xml index d3ec28591..0d5d95da2 100644 --- a/tests/xmlconfig-xml/misc-qemu-driver-overwrite.xml +++ b/tests/xmlconfig-xml/misc-qemu-driver-overwrite.xml @@ -25,7 +25,7 @@ - + diff --git a/tests/xmlconfig-xml/misc-qemu-driver-type.xml b/tests/xmlconfig-xml/misc-qemu-driver-type.xml index d3207a6e5..633d14a6f 100644 --- a/tests/xmlconfig-xml/misc-qemu-driver-type.xml +++ b/tests/xmlconfig-xml/misc-qemu-driver-type.xml @@ -30,7 +30,7 @@ - + diff --git a/tests/xmlconfig-xml/misc-qemu-iso-disk.xml b/tests/xmlconfig-xml/misc-qemu-iso-disk.xml index b3f290b20..72992be88 100644 --- a/tests/xmlconfig-xml/misc-qemu-iso-disk.xml +++ b/tests/xmlconfig-xml/misc-qemu-iso-disk.xml @@ -25,7 +25,7 @@ - + diff --git a/tests/xmlconfig-xml/rhel6-kvm-stage1.xml b/tests/xmlconfig-xml/rhel6-kvm-stage1.xml index c3b52e91a..5e8bf8c55 100644 --- a/tests/xmlconfig-xml/rhel6-kvm-stage1.xml +++ b/tests/xmlconfig-xml/rhel6-kvm-stage1.xml @@ -30,7 +30,7 @@ - + diff --git a/tests/xmlconfig-xml/rhel6-kvm-stage2.xml b/tests/xmlconfig-xml/rhel6-kvm-stage2.xml index b2880c16a..998b1ddc0 100644 --- a/tests/xmlconfig-xml/rhel6-kvm-stage2.xml +++ b/tests/xmlconfig-xml/rhel6-kvm-stage2.xml @@ -28,7 +28,7 @@ - + diff --git a/tests/xmlconfig-xml/winxp-kvm-stage1.xml b/tests/xmlconfig-xml/winxp-kvm-stage1.xml index 8d2a0081b..077a528c2 100644 --- a/tests/xmlconfig-xml/winxp-kvm-stage1.xml +++ b/tests/xmlconfig-xml/winxp-kvm-stage1.xml @@ -24,12 +24,12 @@ - + - + diff --git a/tests/xmlconfig-xml/winxp-kvm-stage2.xml b/tests/xmlconfig-xml/winxp-kvm-stage2.xml index 815dfee57..24f910844 100644 --- a/tests/xmlconfig-xml/winxp-kvm-stage2.xml +++ b/tests/xmlconfig-xml/winxp-kvm-stage2.xml @@ -23,12 +23,12 @@ - + - + diff --git a/tests/xmlconfig-xml/winxp-kvm-stage3.xml b/tests/xmlconfig-xml/winxp-kvm-stage3.xml index 55edf9c40..3bfa18ee9 100644 --- a/tests/xmlconfig-xml/winxp-kvm-stage3.xml +++ b/tests/xmlconfig-xml/winxp-kvm-stage3.xml @@ -23,12 +23,12 @@ - + - + diff --git a/tests/xmlconfig.py b/tests/xmlconfig.py index d5fbfccef..093e632f1 100644 --- a/tests/xmlconfig.py +++ b/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, - driverType="raw")) - g.add_device(VirtualDisk(g.conn, path="/dev/loop0", - device=VirtualDisk.DEVICE_DISK, - driverName="qemu", format="qcow2")) - 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 + + d = VirtualDisk(g.conn) + d.path = "/dev/loop0" + d.device = d.DEVICE_CDROM + d.driver_type = "raw" + d.validate() g.add_device(d) - d = VirtualDisk(g.conn, path="/dev/loop0", - device=VirtualDisk.DEVICE_FLOPPY, - driverName="phy") + d = VirtualDisk(g.conn) + d.path = "/dev/loop0" + 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", - bus="virtio", driverName="qemu", - driverType="qcow2") + d = VirtualDisk(g.conn) + d.path = "/dev/loop0" + 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", - device=VirtualDisk.DEVICE_FLOPPY)) - g.add_device(VirtualDisk(g.conn, path="/dev/loop0", bus="scsi")) - g.add_device(VirtualDisk(g.conn, path="/tmp", device="floppy")) - d3 = VirtualDisk(g.conn, path="/default-pool/testvol1.img", - bus="scsi", driverName="qemu") - d3.address.type = "spapr-vio" - g.add_device(d3) + d = VirtualDisk(g.conn) + d.path = "/dev/loop0" + d.device = d.DEVICE_FLOPPY + d.validate() + g.add_device(d) + + d = VirtualDisk(g.conn) + 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, - sparse=sparse) + disk = VirtualDisk(utils.get_conn()) + 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", - path="/default-pool/testvol1.img") + d = VirtualDisk(utils.get_conn()) + d.bus = "virtio" + d.path = "/default-pool/testvol1.img" targetlist = [] for ignore in range(0, (26 * 2) + 1): diff --git a/tests/xmlparse-xml/change-disk-out.xml b/tests/xmlparse-xml/change-disk-out.xml index 736ccbb51..82e877b45 100644 --- a/tests/xmlparse-xml/change-disk-out.xml +++ b/tests/xmlparse-xml/change-disk-out.xml @@ -18,10 +18,10 @@ 5 /usr/lib/xen/bin/qemu-dm - - + frob + diff --git a/tests/xmlparse-xml/change-media-out.xml b/tests/xmlparse-xml/change-media-out.xml index 10e2df482..878fc2231 100644 --- a/tests/xmlparse-xml/change-media-out.xml +++ b/tests/xmlparse-xml/change-media-out.xml @@ -15,10 +15,10 @@ - + - - + + diff --git a/tests/xmlparse.py b/tests/xmlparse.py index baba73abe..f583c5444 100644 --- a/tests/xmlparse.py +++ b/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() diff --git a/virt-install b/virt-install index e4922cb34..6faff9d5f 100755 --- a/virt-install +++ b/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) diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py index 9287e16c4..1ee6d131f 100644 --- a/virtManager/addhardware.py +++ b/virtManager/addhardware.py @@ -1304,29 +1304,30 @@ class vmmAddHardware(vmmGObjectUI): if do_use: diskpath = ideal - disk = virtinst.VirtualDisk(conn, - path=diskpath, - size=disksize, - sparse=sparse, - readOnly=readonly, - device=device, - bus=bus, - format=fmt) + disk = virtinst.VirtualDisk(conn) + disk.path = diskpath + disk.read_only = readonly + disk.device = device + disk.bus = bus + disk.set_create_storage(size=disksize, sparse=sparse, + fmt=fmt or None) disk.driver_cache = cache if not fmt: fmt = self.config.get_storage_format() if (self.is_default_storage() and - disk.vol_install and - fmt in disk.vol_install.formats): + disk.get_vol_install() and + 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) diff --git a/virtManager/create.py b/virtManager/create.py index a84108073..ce42c95b5 100644 --- a/virtManager/create.py +++ b/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, - path=diskpath, - size=disksize, - sparse=sparse) + disk = virtinst.VirtualDisk(conn) + disk.path = diskpath + disk.set_create_storage(size=disksize, sparse=sparse) fmt = self.config.get_storage_format() if (self.is_default_storage() and - disk.vol_install and - fmt in disk.vol_install.formats): + disk.get_vol_install() and + 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) diff --git a/virtManager/domain.py b/virtManager/domain.py index ac739306a..8d30b4eed 100644 --- a/virtManager/domain.py +++ b/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: diff --git a/virtManager/uihelpers.py b/virtManager/uihelpers.py index 66f5921c2..f88d69180 100644 --- a/virtManager/uihelpers.py +++ b/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 diff --git a/virtinst/CloneManager.py b/virtinst/CloneManager.py index ff840d9b1..e2df75016 100644 --- a/virtinst/CloneManager.py +++ b/virtinst/CloneManager.py @@ -132,10 +132,17 @@ class Cloner(object): if not path: device = VirtualDisk.DEVICE_CDROM - disk = VirtualDisk(self.conn, path, size=.0000001, - device=device) + disk = VirtualDisk(self.conn) + 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, - parsexml=self.original_xml) + self._guest = Guest(self.conn, 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: - clone_disk.size = orig_disk.size + size = orig_disk.get_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() == - orig_disk.vol_object.storagePoolLookupByVolume().name()): - newname = clone_disk.vol_install.name - clone_disk.vol_install = Storage.CloneVolume(self.conn, - newname, - orig_disk.vol_object) + if (clone_vol_install.pool.name() == + orig_disk.get_vol_object().storagePoolLookupByVolume().name()): + newname = clone_vol_install.name + vol_install = Storage.CloneVolume(self.conn, + newname, + 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, - device=device, driverType=disk.driver_type, - validate=validate) - d.target = disk.target + newd = VirtualDisk(self.conn) + newd.path = disk.path + newd.device = device + 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 diff --git a/virtinst/DistroInstaller.py b/virtinst/DistroInstaller.py index d242dd6ad..8f4eb9cee 100644 --- a/virtinst/DistroInstaller.py +++ b/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, - path=os.path.join(poolpath, name), - sizebytes=size, - sparse=True) + disk = VirtualDisk(conn) + disk.path = os.path.join(poolpath, name) + disk.set_create_storage(size=(float(size) / 1024.0 / 1024.0 / 1024.0), + 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, - path=val, - device=VirtualDisk.DEVICE_CDROM, - transient=True, - readOnly=True) + d = self._make_cdrom_dev(val) 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, - path=cdrom, - device=VirtualDisk.DEVICE_CDROM, - readOnly=True, - transient=transient) + disk = self._make_cdrom_dev(cdrom) + disk.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 - - # 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) + disk = self._make_cdrom_dev(self.location) + disk.transient = True # Make sure we always fetch kernel here if required if self._install_bootconfig.kernel and not self.scratchdir_required(): diff --git a/virtinst/ImageInstaller.py b/virtinst/ImageInstaller.py index 29fa6c821..eab8313c6 100644 --- a/virtinst/ImageInstaller.py +++ b/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(conn=self.conn, - path=path, - size=size, - device=device, - format=drive.disk.format) + disk = VirtualDisk(self.conn) + disk.path = path + disk.device = device 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): diff --git a/virtinst/Installer.py b/virtinst/Installer.py index 499cd00f5..cda556263 100644 --- a/virtinst/Installer.py +++ b/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 diff --git a/virtinst/LiveCDInstaller.py b/virtinst/LiveCDInstaller.py index 679177cc3..3d2d4ef86 100644 --- a/virtinst/LiveCDInstaller.py +++ b/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, - path=val, - device=VirtualDisk.DEVICE_CDROM, - readOnly=True) + return self._make_cdrom_dev(val) def _get_location(self): return self._location def _set_location(self, val): diff --git a/virtinst/Storage.py b/virtinst/Storage.py index 5f36d9078..8a3f51ad9 100644 --- a/virtinst/Storage.py +++ b/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 diff --git a/virtinst/VirtualDisk.py b/virtinst/VirtualDisk.py index c18b4be04..489888cdf 100644 --- a/virtinst/VirtualDisk.py +++ b/virtinst/VirtualDisk.py @@ -22,18 +22,15 @@ import os import stat import pwd -import statvfs import subprocess import logging import re import urlgrabber.progress as progress -import libvirt import virtinst - +from virtinst import diskbackend from virtinst import util -from virtinst import Storage from virtinst.VirtualDevice import VirtualDevice from virtinst.XMLBuilderDomain import _xml_property @@ -101,166 +98,47 @@ def _is_dir_searchable(uid, username, path): return bool(re.search("user:%s:..x" % username, out)) -def _check_if_pool_source(conn, path): +def _distill_storage(conn, do_create, nomanaged, + path, vol_object, vol_install, clone_path, *args): """ - If passed path is a host disk device like /dev/sda, want to let the user - use it + Validates and updates params when the backing storage is changed """ - 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 + storage_capable = conn.check_conn_support(conn.SUPPORT_CONN_STORAGE) - 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 + if vol_object: + pass + elif not storage_capable: + pass + elif path and not nomanaged: + path = os.path.abspath(path) + vol_object, pool, path_is_pool = diskbackend.check_if_path_managed( + conn, path) - 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 + creator = None + backend = diskbackend.StorageBackend(conn, path, vol_object, + path_is_pool and pool or None) + if not do_create: + return backend, None - vol = lookup_vol_by_path()[0] - if not vol: - pool = util.lookup_pool_by_path(conn, os.path.dirname(path)) + if backend.exists() and path is not None: + if vol_install: + raise ValueError("vol_install specified but %s exists." % + backend.path) + elif not clone_path: + return backend, None - # 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 + if path or vol_install or pool or clone_path: + creator = diskbackend.StorageCreator(conn, path, pool, + vol_install, clone_path, *args) + return backend, creator -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 +_TARGET_PROPS = ["file", "dev", "dir"] class VirtualDisk(VirtualDevice): - """ - Builds a libvirt domain disk xml description - - The VirtualDisk class is used for building libvirt domain xml descriptions - for disk devices. If creating a disk object from an existing local block - device or file, a path is all that should be required. If you want to - create a local file, a size also needs to be specified. - - The remote case is a bit more complex. The options are: - 1. A libvirt virStorageVol instance (passed as 'volObject') for an - existing storage volume. - 2. A virtinst L{StorageVolume} instance for creating a volume (passed - as 'volInstall'). - 3. An active connection ('conn') and a path to a storage volume on - that connection. - 4. An active connection and a tuple of the form ("poolname", - "volumename") - 5. An active connection and a path. The base of the path must - point to the target path for an active pool. - - For cases 3 and 4, the lookup will be performed, and 'vol_object' - will be set to the returned virStorageVol. For the last case, 'volInstall' - will be populated for a StorageVolume instance. All the above cases also - work on a local connection as well, the only difference being that - option 3 won't necessarily error out if the volume isn't found. - - __init__ and setting all properties performs lots of validation, - and will throw ValueError's if problems are found. - """ # pylint: disable=W0622 # Redefining built-in 'type', but it matches the XML so keep it @@ -299,8 +177,6 @@ class VirtualDisk(VirtualDevice): TYPE_DIR = "dir" types = [TYPE_FILE, TYPE_BLOCK, TYPE_DIR] - _target_props = ["file", "dev", "dir"] - IO_MODE_NATIVE = "native" IO_MODE_THREADS = "threads" io_modes = [IO_MODE_NATIVE, IO_MODE_THREADS] @@ -342,7 +218,8 @@ class VirtualDisk(VirtualDevice): vol = None path_is_pool = False try: - vol, ignore, path_is_pool = _check_if_path_managed(conn, path) + vol, ignore, path_is_pool = diskbackend.check_if_path_managed( + conn, path) except: pass @@ -511,298 +388,215 @@ class VirtualDisk(VirtualDevice): except Exception, e: raise ValueError(_("Couldn't lookup volume object: %s" % str(e))) - def __init__(self, conn, path=None, size=None, transient=False, type=None, - device=None, driverName=None, driverType=None, - readOnly=False, sparse=True, volObject=None, - volInstall=None, bus=None, shareable=False, - format=None, validate=True, parsexml=None, parsexmlnode=None, - sizebytes=None, nomanaged=False): - """ - @param path: filesystem path to the disk image. - @type path: C{str} - @param size: size of local file to create in gigabytes - @type size: C{int} or C{long} or C{float} - @param transient: whether to keep disk around after guest install - @type transient: C{bool} - @param type: disk media type (file, block, ...) - @type type: C{str} - @param device: Emulated device type (disk, cdrom, floppy, ...) - @type device: member of devices - @param driverName: name of driver - @type driverName: member of driver_names - @param driverType: type of driver - @type driverType: member of driver_types - @param readOnly: Whether emulated disk is read only - @type readOnly: C{bool} - @param sparse: Create file as a sparse file - @type sparse: C{bool} - @param conn: Connection disk is being installed on - @type conn: libvirt.virConnect - @param volObject: libvirt storage volume object to use - @type volObject: libvirt.virStorageVol - @param volInstall: StorageVolume instance to build for new storage - @type volInstall: L{StorageVolume} - @param bus: Emulated bus type (ide, scsi, virtio, ...) - @type bus: C{str} - @param shareable: If disk can be shared among VMs - @type shareable: C{bool} - @param format: Storage volume format to use when creating storage - @type format: C{str} - @param validate: Whether to validate passed parameters against the - local system. Omitting this may cause issues, be - warned! - @type validate: C{bool} - @param sizebytes: Optionally specify storage size in bytes. Takes - precedence over size if specified. - @type sizebytes: C{int} - """ - + _DEFAULT_SENTINEL = -1234 + def __init__(self, conn, parsexml=None, parsexmlnode=None): VirtualDevice.__init__(self, conn, parsexml, parsexmlnode) - self._path = None - self._size = None - self._type = None - self._device = None - self._sparse = None + self._device = self.DEVICE_DISK self._readOnly = None - self._vol_object = None - self._pool_object = None - self._vol_install = None self._bus = None self._shareable = None self._driver_cache = None - self._clone_path = None - self._format = None - self._driverName = driverName - self._driverType = driverType self._driver_io = None self._target = None - self._validate = validate - self._nomanaged = nomanaged - # XXX: No property methods for these - self.transient = transient + self._type = self._DEFAULT_SENTINEL + self._driverName = self._DEFAULT_SENTINEL + self._driverType = self._DEFAULT_SENTINEL - if sizebytes is not None: - size = (float(sizebytes) / float(1024 ** 3)) + self._storage_backend = diskbackend.StorageBackend(self.conn, + None, None, None) + self._storage_creator = None + self._override_default = True + self.nomanaged = False + self.transient = False + + + def set_create_storage(self, size=None, sparse=True, + fmt=None, vol_install=None, clone_path=None, + fake=False): + """ + Function that sets storage creation parameters. If this isn't + called, we assume that no storage creation is taking place and + will error accordingly. + + @size is in gigs + @fake: If true, make like we are creating storage but fail + if we ever asked to do so. + """ if self._is_parse(): - self._validate = False - return + raise ValueError("Cannot create storage for a parsed disk.") + path = self.path - self.set_read_only(readOnly, validate=False) - self.set_sparse(sparse, validate=False) - self.set_type(type, validate=False) - self.set_device(device or self.DEVICE_DISK, validate=False) - self._set_path(path, validate=False) - self._set_size(size, validate=False) - self._set_vol_object(volObject, validate=False) - self._set_vol_install(volInstall, validate=False) - self._set_bus(bus, validate=False) - self._set_shareable(shareable, validate=False) - self._set_format(format, validate=False) + # Validate clone_path + if clone_path is not None: + clone_path = os.path.abspath(clone_path) - self.__change_storage(self.path, - self.vol_object, - self.vol_install) - self.__validate_params() + try: + # If this disk isn't managed, make sure we only perform + # non-managed lookup + d = VirtualDisk(self.conn) + d.path = clone_path + d.nomanaged = not self.__managed_storage() + d.set_create_storage(fake=True) + d.validate() + except Exception, e: + raise ValueError(_("Error validating clone path: %s") % e) + if fake and size is None: + size = .000001 - # - # Parameters for specifying the backing storage - # + backend, creator = _distill_storage( + self.conn, True, self.nomanaged, path, None, + vol_install, clone_path, + size, sparse, fmt) + ignore = backend + self._storage_creator = creator + if self._storage_creator and fake: + self._storage_creator.fake = True + + if not self._storage_creator and fmt: + if self.driver_name == self.DRIVER_QEMU: + self.driver_type = fmt + + if not self._storage_creator and (vol_install or clone_path): + raise RuntimeError("Need storage creation but it didn't happen.") def _get_path(self): - retpath = self._path - if self.vol_object: - retpath = self.vol_object.path() - elif self.vol_install: - retpath = (util.xpath(self.vol_install.pool.XMLDesc(0), - "/pool/target/path") + "/" + - self.vol_install.name) - - return retpath - def _set_path(self, val, validate=True): - if val is not None: - val = os.path.abspath(val) - - if validate: - self.__change_storage(path=val) - self.__validate_wrapper("_path", val, validate, self.path) + if self._storage_creator: + return self._storage_creator.path + return self._storage_backend.path + def _set_path(self, val): + if self._storage_creator: + raise ValueError("Can't change disk path if storage creation info " + "has been set.") + if val != self.path: + self._change_storage(path=val) def _xml_get_xpath(self): xpath = None - for prop in self._target_props: + ret = "./source/@file" + for prop in _TARGET_PROPS: xpath = "./source/@" + prop if self._xml_ctx.xpathEval(xpath): - return xpath - return "./source/@file" + ret = xpath + break + return ret def _xml_set_xpath(self): return "./source/@" + self.disk_type_to_target_prop(self.type) path = _xml_property(_get_path, _set_path, xml_get_xpath=_xml_get_xpath, - xml_set_xpath=_xml_set_xpath,) + xml_set_xpath=_xml_set_xpath, + clear_first=["./source/@" + target for target in + _TARGET_PROPS]) + def get_sparse(self): + if self._storage_creator: + return self._storage_creator.get_sparse() + return None - def _get_vol_object(self): - return self._vol_object - def _set_vol_object(self, val, validate=True): - if val is not None and not isinstance(val, libvirt.virStorageVol): - raise ValueError(_("vol_object must be a virStorageVol instance")) + def get_vol_object(self): + return self._storage_backend.get_vol_object() + def set_vol_object(self, val): + if self.path: + raise ValueError("Can't change disk vol_object if path is set.") + if self._storage_creator: + raise ValueError("Can't change disk vol_object if storage_creator " + "is set.") + if val != self.get_vol_object(): + self._change_storage(vol_object=val) + def get_vol_install(self): + if not self._storage_creator: + return None + return self._storage_creator.get_vol_install() - if validate: - self.__change_storage(vol_object=val) - self.__validate_wrapper("_vol_object", val, validate, self.vol_object) - vol_object = property(_get_vol_object, _set_vol_object) - - def _get_vol_install(self): - return self._vol_install - def _set_vol_install(self, val, validate=True): - if val is not None and not isinstance(val, Storage.StorageVolume): - raise ValueError(_("vol_install must be a StorageVolume " - " instance.")) - - if validate: - self.__change_storage(vol_install=val) - self.__validate_wrapper("_vol_install", val, validate, self.vol_install) - vol_install = property(_get_vol_install, _set_vol_install) - - # - # Other properties - # - def _get_clone_path(self): - return self._clone_path - def _set_clone_path(self, val, validate=True): - if val is not None: - val = os.path.abspath(val) - - try: - VirtualDisk(self.conn, path=val, nomanaged=True) - except Exception, e: - raise ValueError(_("Error validating clone path: %s") % e) - self.__validate_wrapper("_clone_path", val, validate, self.clone_path) - clone_path = property(_get_clone_path, _set_clone_path) - - def _get_size(self): - retsize = self.__existing_storage_size() - if retsize is None: - if self.vol_install: - retsize = self.vol_install.capacity / 1024.0 / 1024.0 / 1024.0 - else: - retsize = self._size - - return retsize - def _set_size(self, val, validate=True): - if val is not None: - if type(val) not in [int, float, long] or val < 0: - raise ValueError(_("'size' must be a number greater than 0.")) - - self.__validate_wrapper("_size", val, validate, self.size) - size = property(_get_size, _set_size) + def get_size(self): + if self._storage_creator: + return self._storage_creator.get_size() + return self._storage_backend.get_size() def get_type(self): - if self._type: + if self._type != self._DEFAULT_SENTINEL: return self._type - return self.__existing_storage_dev_type() - def set_type(self, val, validate=True): - if val is not None: - if val not in self.types: - raise ValueError(_("Unknown storage type '%s'" % val)) - self.__validate_wrapper("_type", val, validate, self.type) + return self._get_default_type() + def set_type(self, val): + if self._override_default: + self._type = val type = _xml_property(get_type, set_type, xpath="./@type") def get_device(self): return self._device - def set_device(self, val, validate=True): - if val not in self.devices: - raise ValueError(_("Unknown device type '%s'" % val)) - + def set_device(self, val): if val == self._device: return if self._is_parse(): self.bus = None self.target = None - self.__validate_wrapper("_device", val, validate, self.device) + self._device = val device = _xml_property(get_device, set_device, xpath="./@device") def get_driver_name(self): - retname = self._driverName - if not retname: - retname, ignore = self.__get_default_driver() - return retname - def set_driver_name(self, val, validate=True): - ignore = validate - self._driverName = val + if self._driverName != self._DEFAULT_SENTINEL: + return self._driverName + return self._get_default_driver()[0] + def set_driver_name(self, val): + if self._override_default: + self._driverName = val driver_name = _xml_property(get_driver_name, set_driver_name, xpath="./driver/@name") def get_driver_type(self): - rettype = self._driverType - if not rettype: - ignore, rettype = self.__get_default_driver() - return rettype - def set_driver_type(self, val, validate=True): - ignore = validate - self._driverType = val + if self._driverType != self._DEFAULT_SENTINEL: + return self._driverType + return self._get_default_driver()[1] + def set_driver_type(self, val): + if self._override_default: + self._driverType = val driver_type = _xml_property(get_driver_type, set_driver_type, xpath="./driver/@type") - def get_sparse(self): - return self._sparse - def set_sparse(self, val, validate=True): - self.__validate_wrapper("_sparse", val, validate, self.sparse) - sparse = property(get_sparse, set_sparse) - def get_read_only(self): return self._readOnly - def set_read_only(self, val, validate=True): - self.__validate_wrapper("_readOnly", val, validate, self.read_only) + def set_read_only(self, val): + self._readOnly = val read_only = _xml_property(get_read_only, set_read_only, xpath="./readonly", is_bool=True) def _get_bus(self): return self._bus - def _set_bus(self, val, validate=True): - self.__validate_wrapper("_bus", val, validate, self.bus) + def _set_bus(self, val): + self._bus = val bus = _xml_property(_get_bus, _set_bus, xpath="./target/@bus") def _get_target(self): return self._target - def _set_target(self, val, validate=True): - ignore = validate + def _set_target(self, val): self._target = val target = _xml_property(_get_target, _set_target, xpath="./target/@dev") def _get_shareable(self): return self._shareable - def _set_shareable(self, val, validate=True): - self.__validate_wrapper("_shareable", val, validate, self.shareable) + def _set_shareable(self, val): + self._shareable = val shareable = _xml_property(_get_shareable, _set_shareable, xpath="./shareable", is_bool=True) def _get_driver_cache(self): return self._driver_cache - def _set_driver_cache(self, val, validate=True): - if val is not None: - if val not in self.cache_types: - raise ValueError(_("Unknown cache mode '%s'" % val)) - self.__validate_wrapper("_driver_cache", val, validate, - self.driver_cache) + def _set_driver_cache(self, val): + self._driver_cache = val driver_cache = _xml_property(_get_driver_cache, _set_driver_cache, xpath="./driver/@cache") def _get_driver_io(self): return self._driver_io - def _set_driver_io(self, val, validate=True): - if val is not None: - if val not in self.io_modes: - raise ValueError(_("Unknown io mode '%s'" % val)) - self.__validate_wrapper("_driver_io", val, validate, - self.driver_io) + def _set_driver_io(self, val): + self._driver_io = val driver_io = _xml_property(_get_driver_io, _set_driver_io, xpath="./driver/@io") @@ -828,163 +622,36 @@ class VirtualDisk(VirtualDevice): get_converter=lambda s, x: int(x or 0), set_converter=lambda s, x: int(x)) - def _get_format(self): - return self._format - def _set_format(self, val, validate=True): - self.__validate_wrapper("_format", val, validate, self.format) - format = property(_get_format, _set_format) - # Validation assistance methods - - # Initializes attribute if it hasn't been done, then validates args. - # If validation fails, reset attribute to original value and raise error - def __validate_wrapper(self, varname, newval, validate, origval): - orig = origval - setattr(self, varname, newval) - - if validate: - try: - self.__validate_params() - except: - setattr(self, varname, orig) - raise + ################################# + # Validation assistance methods # + ################################# def can_be_empty(self): return (self.device == self.DEVICE_FLOPPY or self.device == self.DEVICE_CDROM) - def __change_storage(self, path=None, vol_object=None, vol_install=None): - """ - Validates and updates params when the backing storage is changed - """ - pool = None - storage_capable = self.conn.check_conn_support( - self.conn.SUPPORT_CONN_STORAGE) + def _change_storage(self, path=None, vol_object=None): + backend, ignore = _distill_storage( + self.conn, False, self.nomanaged, + path, vol_object, None, None) + self._storage_backend = backend - # Try to lookup self.path storage objects - if vol_object or vol_install: - pass - elif not storage_capable: - pass - elif path: - vol_object, pool, path_is_pool = _check_if_path_managed(self.conn, - path) - if (pool and - not vol_object and - not path_is_pool and - not self._is_parse()): - vol_install = _build_vol_install(self.conn, - path, pool, - self.size, - self.sparse) - - if not path_is_pool: - pool = None - - # Finally, set the relevant params - self._set_path(path, validate=False) - self._set_vol_object(vol_object, validate=False) - self._set_vol_install(vol_install, validate=False) - self._pool_object = pool - - # XXX: Hack, we shouldn't have to conditionalize for parsing if self._is_parse(): - self.type = self.get_type() - self.driver_name = self.get_driver_name() - self.driver_type = self.get_driver_type() - - - def __set_format(self): - if not self.format: - return - - if not self.creating_storage(): - return - - if self.vol_install: - if not hasattr(self.vol_install, "format"): - raise ValueError(_("Storage type does not support format " - "parameter.")) - if self.vol_install.format != self.format: - self.vol_install.format = self.format - - elif self.format != "raw": - raise RuntimeError(_("Format cannot be specified for " - "unmanaged storage.")) - - def __existing_storage_size(self): - """ - Return size of existing storage - """ - if self.creating_storage(): - return - - if self.vol_object: - newsize = util.xpath(self.vol_object.XMLDesc(0), - "/volume/capacity") try: - newsize = float(newsize) / 1024.0 / 1024.0 / 1024.0 - except: - newsize = 0 - elif self._pool_object: - newsize = util.xpath(self.vol_object.XMLDesc(0), - "/pool/capacity") - try: - newsize = float(newsize) / 1024.0 / 1024.0 / 1024.0 - except: - newsize = 0 - elif self.path is None: - newsize = 0 - else: - ignore, newsize = util.stat_disk(self.path) - newsize = newsize / 1024.0 / 1024.0 / 1024.0 + self._override_default = False + self.type = self.type + self.driver_name = self.driver_name + self.driver_type = self.driver_type + finally: + self._override_default = True - return newsize + def _get_default_type(self): + if self._storage_creator: + return self._storage_creator.get_dev_type() + return self._storage_backend.get_dev_type() - def __existing_storage_dev_type(self): - """ - Detect disk 'type' () from passed storage parameters - """ - - dtype = None - if self.vol_object: - # vol info is [vol type (file or block), capacity, allocation] - t = self.vol_object.info()[0] - if t == libvirt.VIR_STORAGE_VOL_FILE: - dtype = self.TYPE_FILE - elif t == libvirt.VIR_STORAGE_VOL_BLOCK: - dtype = self.TYPE_BLOCK - else: - dtype = self.TYPE_FILE - - elif self.vol_install: - if self.vol_install.file_type == libvirt.VIR_STORAGE_VOL_FILE: - dtype = self.TYPE_FILE - else: - dtype = self.TYPE_BLOCK - elif self._pool_object: - xml = self._pool_object.XMLDesc(0) - for source, source_type in [("dir", self.TYPE_DIR), - ("device", self.TYPE_BLOCK), - ("adapter", self.TYPE_BLOCK)]: - if util.xpath(xml, "/pool/source/%s/@dev" % source): - dtype = source_type - break - - elif self.path: - if os.path.isdir(self.path): - dtype = self.TYPE_DIR - elif util.stat_disk(self.path)[0]: - dtype = self.TYPE_FILE - else: - dtype = self.TYPE_BLOCK - - if not dtype: - dtype = self._type or self.TYPE_BLOCK - - return dtype - - def __get_default_driver(self): + def _get_default_driver(self): """ Set driverName and driverType from passed parameters @@ -995,31 +662,21 @@ class VirtualDisk(VirtualDevice): http://lists.gnu.org/archive/html/qemu-devel/2008-04/msg00675.html """ drvname = self._driverName + if drvname == self._DEFAULT_SENTINEL: + drvname = None drvtype = self._driverType + if drvtype == self._DEFAULT_SENTINEL: + drvtype = None if self.conn.is_qemu() and not drvname: drvname = self.DRIVER_QEMU - if self.format: - if drvname == self.DRIVER_QEMU: - drvtype = _qemu_sanitize_drvtype(self.type, self.format, - manual_format=True) - - elif self.vol_object: - fmt = util.xpath(self.vol_object.XMLDesc(0), - "/volume/target/format/@type") - if drvname == self.DRIVER_QEMU: - drvtype = _qemu_sanitize_drvtype(self.type, fmt) - - elif self.vol_install: - if drvname == self.DRIVER_QEMU: - if hasattr(self.vol_install, "format"): - drvtype = _qemu_sanitize_drvtype(self.type, - self.vol_install.format) - - elif self.creating_storage(): - if drvname == self.DRIVER_QEMU: - drvtype = self.DRIVER_QEMU_RAW + if drvname == self.DRIVER_QEMU: + if self._storage_creator: + drvtype = self._storage_creator.get_driver_type() + else: + drvtype = self._storage_backend.get_driver_type() + drvtype = _qemu_sanitize_drvtype(self.type, drvtype) return drvname or None, drvtype or None @@ -1028,52 +685,24 @@ class VirtualDisk(VirtualDevice): Return bool representing if managed storage parameters have been explicitly specified or filled in """ - if self._nomanaged: - return False - return bool(self.vol_object is not None or - self.vol_install is not None or - self._pool_object is not None) + if self._storage_creator: + return self._storage_creator.is_managed() + return self._storage_backend.is_managed() def creating_storage(self): """ Return True if the user requested us to create a device """ - if self.__no_storage(): - return False - - if self.__managed_storage(): - if self.vol_object or self._pool_object: - return False - return True - - if (not self.conn.is_remote() and - self.path and - os.path.exists(self.path)): - return False - - return True - - def __no_storage(self): - """ - Return True if no path or storage was specified - """ - if self.__managed_storage(): - return False - if self.path: - return False - return True + return bool(self._storage_creator) - def __validate_params(self): + def validate(self): """ function to validate all the complex interaction between the various disk parameters. """ - if not self._validate: - return - # No storage specified for a removable device type (CDROM, floppy) - if self.__no_storage(): + if self.path is None: if not self.can_be_empty(): raise ValueError(_("Device type '%s' requires a path") % self.device) @@ -1098,10 +727,13 @@ class VirtualDisk(VirtualDevice): managed_storage = self.__managed_storage() create_media = self.creating_storage() - self.__set_format() - # If not creating the storage, our job is easy if not create_media: + if not self._storage_backend.exists(): + raise ValueError( + _("Must specify storage creation parameters for " + "non-existent path '%s'.") % self.path) + # Make sure we have access to the local path if not managed_storage: if (os.path.isdir(self.path) and @@ -1111,27 +743,7 @@ class VirtualDisk(VirtualDevice): return True - - if (self.device == self.DEVICE_FLOPPY or - self.device == self.DEVICE_CDROM): - raise ValueError(_("Cannot create storage for %s device.") % - self.device) - - if not managed_storage: - if self.type is self.TYPE_BLOCK: - raise ValueError(_("Local block device path '%s' must " - "exist.") % self.path) - - # Path doesn't exist: make sure we have write access to dir - 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)) + self._storage_creator.validate(self.device, self.type) # Applicable for managed or local storage ret = self.is_size_conflict() @@ -1140,123 +752,6 @@ class VirtualDisk(VirtualDevice): elif ret[1]: logging.warn(ret[1]) - # Storage creation routines - def _do_create_storage(self, progresscb): - # 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)): - self._set_vol_object(self.vol_install.install(meter=progresscb), - validate=False) - return - - 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 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) def setup(self, meter=None): """ @@ -1265,15 +760,19 @@ class VirtualDisk(VirtualDevice): If storage doesn't exist (a non-existent file 'path', or 'vol_install' was specified), we create it. - @param conn: Optional connection to use if self.conn not specified @param meter: Progress meter to report file creation on @type meter: instanceof urlgrabber.BaseMeter """ if not meter: meter = progress.BaseMeter() + if not self._storage_creator: + return + + volobj = self._storage_creator.create(meter) + self._storage_creator = None + if volobj: + self._change_storage(vol_object=volobj) - if self.creating_storage() or self.clone_path: - self._do_create_storage(meter) def _get_xml_config(self, disknode=None): """ @@ -1290,14 +789,8 @@ class VirtualDisk(VirtualDevice): if self.target: disknode = self.target - if not disknode: - raise ValueError(_("'disknode' or self.target must be set!")) - path = None - if self.vol_object: - path = self.vol_object.path() - elif self.path: - path = self.path + path = self.path if path: path = util.xml_escape(path) @@ -1372,32 +865,9 @@ class VirtualDisk(VirtualDevice): Non fatal conflicts (sparse disk exceeds available space) will return (False, "description of collision") """ - - if self.vol_install: - return self.vol_install.is_size_conflict() - - if not self.creating_storage(): + if not self._storage_creator: return (False, None) - - 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) + return self._storage_creator.is_size_conflict() def is_conflict_disk(self, conn): """ @@ -1407,19 +877,13 @@ class VirtualDisk(VirtualDevice): @return: list of colliding VM names @rtype: C{list} """ - if self.vol_object: - path = self.vol_object.path() - else: - path = self.path - - if not path: + if not self.path: return False - if not conn: conn = self.conn check_conflict = self.shareable - ret = self.path_in_use_by(conn, path, + ret = self.path_in_use_by(conn, self.path, check_conflict=check_conflict) return ret diff --git a/virtinst/XMLBuilderDomain.py b/virtinst/XMLBuilderDomain.py index 92fc06df7..dd02f0097 100644 --- a/virtinst/XMLBuilderDomain.py +++ b/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 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() diff --git a/virtinst/cli.py b/virtinst/cli.py index caf743dd2..a2a7c863f 100644 --- a/virtinst/cli.py +++ b/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.vol_object or + (origdev.get_vol_install() 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: - dev.size = size + if size is not None and size != dev.get_size(): + dev.set_create_storage(size=size, sparse=origsparse) else: - dev = VirtualDisk(conn=conn, path=path, size=size, - sparse=origsparse) + dev = VirtualDisk(conn) + 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: - # Build a stub device that should always validate cleanly - dev = virtinst.VirtualDisk(conn=guest.conn, - path=abspath, - volObject=volobj, - volInstall=volinst, - size=size, - readOnly=ro, - sparse=sparse, - shareable=shared, - device=device, - format=fmt) + if volobj: + dev.set_vol_object(volobj) + else: + dev.path = abspath + dev.read_only = ro + dev.shareable = shared + dev.set_create_storage(size=size, fmt=fmt, sparse=sparse, + vol_install=volinst) set_param = _build_set_param(dev, opts) - set_param("path", "path", abspath) - 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("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"]: diff --git a/virtinst/diskbackend.py b/virtinst/diskbackend.py new file mode 100644 index 000000000..f12f0fdfb --- /dev/null +++ b/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) diff --git a/virtinst/osdict.py b/virtinst/osdict.py index ca4ca8bb5..bd6bab4bc 100644 --- a/virtinst/osdict.py +++ b/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": {