diff --git a/tests/data/cli/compare/virt-xml-build-disk-domain.xml b/tests/data/cli/compare/virt-xml-build-disk-domain.xml index 76af6280f..e59eb93f8 100644 --- a/tests/data/cli/compare/virt-xml-build-disk-domain.xml +++ b/tests/data/cli/compare/virt-xml-build-disk-domain.xml @@ -1,5 +1,5 @@ - + diff --git a/tests/test_cli.py b/tests/test_cli.py index e5436a209..f7c5ef1d7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -765,7 +765,7 @@ c.add_valid("--disk %(EXISTIMG1)s") # Not specifying path= c.add_valid("--disk %(NEWIMG1)s,format=raw,size=.0000001") # Not specifying path= but creating storage c.add_valid("--disk %(COLLIDE)s --check path_in_use=off") # Colliding storage with --check c.add_valid("--disk %(COLLIDE)s --force") # Colliding storage with --force -c.add_valid("--disk /dev/default-pool/sharevol.img,perms=sh") # Colliding shareable storage +c.add_valid("--connect %(URI-KVM)s --disk /dev/default-pool/sharevol.img,perms=sh") # Colliding shareable storage c.add_valid("--disk path=%(EXISTIMG1)s,device=cdrom --disk path=%(EXISTIMG1)s,device=cdrom") # Two IDE cds c.add_valid("--disk %(EXISTIMG1)s,driver_name=qemu,driver_type=qcow2") # Driver name and type options c.add_valid("--disk /dev/zero") # Referencing a local unmanaged /dev node @@ -777,6 +777,8 @@ c.add_invalid("--file %(NEWIMG1)s --file-size 100000 --nonsparse") # Nonexistin c.add_invalid("--file %(NEWIMG1)s --file-size 100000") # Huge file, sparse, but no prompting c.add_invalid("--file %(NEWIMG1)s") # Nonexisting file, no size c.add_invalid("--file %(EXISTIMG1)s --file %(EXISTIMG1)s --file %(EXISTIMG1)s --file %(EXISTIMG1)s --file %(EXISTIMG1)s") # Too many IDE +c.add_invalid("--disk device=disk", grep="requires a path") # --disk device=disk, but no path +c.add_invalid("--disk pool=disk-pool,size=1,format=qcow2", grep="Format attribute not supported") # format= invalid for disk pool c.add_invalid("--disk pool=foopool,size=.0001") # Specify a nonexistent pool c.add_invalid("--disk vol=default-pool/foovol") # Specify a nonexistent volume c.add_invalid("--disk vol=default-pool-no-slash") # Wrong vol= format diff --git a/tests/test_xmlconfig.py b/tests/test_xmlconfig.py index 5d7060eaf..7ff55883f 100644 --- a/tests/test_xmlconfig.py +++ b/tests/test_xmlconfig.py @@ -202,19 +202,23 @@ class TestXMLMisc(unittest.TestCase): # Normally the dir searchable test is skipped in the unittest, # but let's contrive an example that should trigger all the code # to ensure it isn't horribly broken - from virtinst import diskbackend - uid = -1 - username = "fakeuser-zzzz" + conn = utils.URIs.open_kvm() + with tempfile.TemporaryDirectory() as tmpdir: - fixlist = diskbackend.is_path_searchable(tmpdir, uid, username) - self.assertTrue(bool(fixlist)) - errdict = diskbackend.set_dirs_searchable(fixlist, username) + searchdata = virtinst.DeviceDisk.check_path_search(conn, tmpdir) + self.assertTrue(bool(searchdata.fixlist)) + self.assertEqual(searchdata.user, "qemu") + errdict = virtinst.DeviceDisk.fix_path_search(searchdata) self.assertTrue(not bool(errdict)) - import getpass - fixlist = diskbackend.is_path_searchable( - os.getcwd(), os.getuid(), getpass.getuser()) - self.assertTrue(not bool(fixlist)) + def test_path_in_use(self): + # Extra tests for DeviceDisk.path_in_use + conn = utils.URIs.open_kvm() + + # Comparing against kernel + vms = virtinst.DeviceDisk.path_in_use_by( + conn, "/dev/default-pool/test-arm-kernel") + assert vms == ["test-arm-kernel"] def test_nonpredicatble_generate(self): kvm_uri = utils.URIs.kvm.replace(",predictable", "") diff --git a/tests/testdriver.xml b/tests/testdriver.xml index 4ca85db17..040b93019 100644 --- a/tests/testdriver.xml +++ b/tests/testdriver.xml @@ -227,7 +227,9 @@ Foo bar baz & yeah boii < > yeahfoo +
+ @@ -302,6 +304,13 @@ Foo bar baz & yeah boii < > yeahfoo + + + + + + + diff --git a/tests/testsuite.xml b/tests/testsuite.xml index e8bba338c..b9a969b75 100644 --- a/tests/testsuite.xml +++ b/tests/testsuite.xml @@ -645,6 +645,50 @@ + + disk-pool + 35bb2ad9-388a-cdfe-461a-b8907f6e5aaa + 107374182400 + 0 + 107374182400 + + + + /dev/disk-pool + + 0700 + 10736 + 10736 + + + + + diskvol1 + 1000000 + 50000 + + + 0700 + 10736 + 10736 + + + + + diskvol7 + 1000000 + 50000 + + + 0700 + 10736 + 10736 + + + + + + diff --git a/virtManager/device/addstorage.py b/virtManager/device/addstorage.py index 9fc8ffc3b..55b1a13b5 100644 --- a/virtManager/device/addstorage.py +++ b/virtManager/device/addstorage.py @@ -100,8 +100,7 @@ class vmmAddStorage(vmmGObjectUI): return log.debug("Attempting to correct permission issues.") - errors = virtinst.DeviceDisk.fix_path_search( - conn.get_backend(), searchdata) + errors = virtinst.DeviceDisk.fix_path_search(searchdata) if not errors: return diff --git a/virtinst/devices/disk.py b/virtinst/devices/disk.py index 26a9c3cb2..c9b08b77b 100644 --- a/virtinst/devices/disk.py +++ b/virtinst/devices/disk.py @@ -12,6 +12,7 @@ from ..logger import log from .. import diskbackend from .. import progress +from .. import xmlutil from .device import Device, DeviceSeclabel from ..xmlbuilder import XMLBuilder, XMLChildProperty, XMLProperty @@ -115,7 +116,6 @@ class DeviceDisk(Device): if path is None: return searchdata - path = os.path.abspath(path) if conn.is_remote(): return searchdata if not conn.is_qemu_system(): @@ -124,10 +124,11 @@ class DeviceDisk(Device): return searchdata if diskbackend.path_is_network_vol(conn, path): return searchdata + path = os.path.abspath(path) user, uid = conn.caps.host.get_qemu_baselabel() if not user: - return searchdata + return searchdata # pragma: no cover if uid == 0: return searchdata @@ -138,13 +139,12 @@ class DeviceDisk(Device): return searchdata @staticmethod - def fix_path_search(conn, searchdata): + def fix_path_search(searchdata): """ Try to fix any permission problems found by check_path_search :returns: Return a dictionary of entries {broken path : error msg} """ - ignore = conn errdict = diskbackend.set_dirs_searchable( searchdata.fixlist, searchdata.user) return errdict @@ -172,7 +172,7 @@ class DeviceDisk(Device): while backpath in volmap: vol = volmap[backpath] if vol in vols: - break + break # pragma: no cover backpath = vol.target_path vols.append(backpath) @@ -326,8 +326,9 @@ class DeviceDisk(Device): def _set_path(self, newpath): if (self._storage_backend and self._storage_backend.will_create_storage()): - raise ValueError(_("Can't change disk path if storage creation info " - "has been set.")) + xmlutil.raise_programming_error(None, + "Can't change disk path if storage creation info " + "has been set.") # User explicitly changed 'path', so try to lookup its storage # object since we may need it @@ -788,19 +789,14 @@ class DeviceDisk(Device): return (False, None) return self._storage_backend.is_size_conflict() - def is_conflict_disk(self, conn=None): + def is_conflict_disk(self): """ check if specified storage is in use by any other VMs on passed connection. :returns: list of colliding VM names """ - if not self.path: - return False - if not conn: - conn = self.conn - - ret = self.path_in_use_by(conn, self.path, + ret = self.path_in_use_by(self.conn, self.path, shareable=self.shareable, read_only=self.read_only) return ret diff --git a/virtinst/diskbackend.py b/virtinst/diskbackend.py index 5c4d56071..fd3cf75b0 100644 --- a/virtinst/diskbackend.py +++ b/virtinst/diskbackend.py @@ -34,7 +34,7 @@ def _lookup_vol_by_path(conn, path): # an empty error code if (e.get_error_code() and e.get_error_code() != libvirt.VIR_ERR_NO_STORAGE_VOL): - raise + raise # pragma: no cover return None, e @@ -129,7 +129,7 @@ def manage_path(conn, path): If path is not managed, try to create a storage pool to probe the path """ if not conn.support.conn_storage(): - return None, None + return None, None # pragma: no cover if not path: return None, None @@ -160,20 +160,15 @@ def path_is_url(path): """ Detect if path is a URL """ - if not path: - return False - return bool(re.match(r"[a-zA-Z]+(\+[a-zA-Z]+)?://.*", path)) + return bool(re.match(r"[a-zA-Z]+(\+[a-zA-Z]+)?://.*", path or "")) def path_is_network_vol(conn, path): """ Detect if path is a network volume such as rbd, gluster, etc """ - if not path: - return False - for volxml in conn.fetch_all_vols(): - if volxml.target_path == path: + if path and volxml.target_path == path: return volxml.type == "network" return False @@ -240,7 +235,7 @@ def path_definitely_exists(conn, path): if not conn.is_remote(): return os.path.exists(path) - except Exception: + except Exception: # pragma: no cover pass return False