From 387614c641b620954f9c987e957a843dbd54f537 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Tue, 11 Jun 2019 10:05:15 -0400 Subject: [PATCH] generatename: Move libvirt collision handling to callers Make every caller pass an explicit cb that handles libvirt collision processing. Makes it easier to see exactly what is going on at the call impls --- virtManager/create.py | 6 ++++-- virtManager/createnet.py | 6 ++++-- virtManager/snapshots.py | 4 +++- virtinst/cloner.py | 14 +++++++------- virtinst/generatename.py | 14 +++----------- virtinst/guest.py | 3 ++- virtinst/storage.py | 5 ++--- 7 files changed, 25 insertions(+), 27 deletions(-) diff --git a/virtManager/create.py b/virtManager/create.py index 63738329..6b4dcee1 100644 --- a/virtManager/create.py +++ b/virtManager/create.py @@ -1494,8 +1494,10 @@ class vmmCreate(vmmGObjectUI): basename += "-%s" % _pretty_arch(self._guest.os.arch) force_num = False - return virtinst.generatename.generate_name(basename, - self.conn.get_backend().lookupByName, + def cb(n): + return virtinst.generatename.check_libvirt_collision( + self.conn.get_backend().lookupByName, n) + return virtinst.generatename.generate_name(basename, cb, start_num=force_num and 1 or 2, force_num=force_num, sep=not force_num and "-" or "") diff --git a/virtManager/createnet.py b/virtManager/createnet.py index 2dd751aa..1220668e 100644 --- a/virtManager/createnet.py +++ b/virtManager/createnet.py @@ -114,8 +114,10 @@ class vmmCreateNetwork(vmmGObjectUI): def reset_state(self): basename = "network" - default_name = generatename.generate_name( - basename, self.conn.get_backend().networkLookupByName) + def cb(n): + return generatename.check_libvirt_collision( + self.conn.get_backend().networkLookupByName, n) + default_name = generatename.generate_name(basename, cb) self.widget("net-name").set_text(default_name) self.widget("net-dns-use-netname").set_active(True) diff --git a/virtManager/snapshots.py b/virtManager/snapshots.py index df1881ef..8f46b2f6 100644 --- a/virtManager/snapshots.py +++ b/virtManager/snapshots.py @@ -118,7 +118,9 @@ class vmmSnapshotNew(vmmGObjectUI): def _reset_state(self): basename = "snapshot" - cb = self.vm.get_backend().snapshotLookupByName + def cb(n): + return generatename.check_libvirt_collision( + self.vm.get_backend().snapshotLookupByName, n) default_name = generatename.generate_name( basename, cb, sep="", start_num=1, force_num=True) diff --git a/virtinst/cloner.py b/virtinst/cloner.py index 082dfe98..8bf8a22b 100644 --- a/virtinst/cloner.py +++ b/virtinst/cloner.py @@ -493,11 +493,9 @@ class Cloner(object): clonebase = newname clonebase = os.path.join(dirname, clonebase) - return generatename.generate_name( - clonebase, - lambda p: DeviceDisk.path_definitely_exists(self.conn, p), - suffix, - lib_collision=False) + def cb(p): + return DeviceDisk.path_definitely_exists(self.conn, p) + return generatename.generate_name(clonebase, cb, suffix=suffix) def generate_clone_name(self): # If the orig name is "foo-clone", we don't want the clone to be @@ -512,9 +510,11 @@ class Cloner(object): start_num = int(str(num_match.group())) basename = basename.replace(match.group(), "") + def cb(n): + return generatename.check_libvirt_collision( + self.conn.lookupByName, n) basename = basename + "-clone" - return generatename.generate_name(basename, - self.conn.lookupByName, + return generatename.generate_name(basename, cb, sep="", start_num=start_num) diff --git a/virtinst/generatename.py b/virtinst/generatename.py index 997d6e33..f0ca2f4e 100644 --- a/virtinst/generatename.py +++ b/virtinst/generatename.py @@ -8,7 +8,7 @@ import libvirt -def libvirt_collision(collision_cb, val): +def check_libvirt_collision(collision_cb, val): """ Run the passed collision function with val as the only argument: If libvirtError is raised, return False @@ -24,7 +24,7 @@ def libvirt_collision(collision_cb, val): return check -def generate_name(base, collision_cb, suffix="", lib_collision=True, +def generate_name(base, collision_cb, suffix="", start_num=1, sep="-", force_num=False): """ Generate a new name from the passed base string, verifying it doesn't @@ -42,8 +42,6 @@ def generate_name(base, collision_cb, suffix="", lib_collision=True, :param base: The base string to use for the name (e.g. "my-orig-vm-clone") :param collision_cb: A callback function to check for collision, receives the generated name as its only arg - :param lib_collision: If true, the collision_cb is not a boolean function, - and instead throws a libvirt error on failure :param start_num: The number to start at for generating non colliding names :param sep: The separator to use between the basename and the generated number (default is "-") @@ -51,12 +49,6 @@ def generate_name(base, collision_cb, suffix="", lib_collision=True, """ base = str(base) - def collide(n): - if lib_collision: - return libvirt_collision(collision_cb, n) - else: - return collision_cb(n) - numrange = list(range(start_num, start_num + 100000)) if not force_num: numrange = [None] + numrange @@ -68,7 +60,7 @@ def generate_name(base, collision_cb, suffix="", lib_collision=True, tryname += ("%s%d" % (sep, i)) tryname += suffix - if not collide(tryname): + if not collision_cb(tryname): ret = tryname break diff --git a/virtinst/guest.py b/virtinst/guest.py index eb6a9202..207ca8b7 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -131,7 +131,8 @@ class Guest(XMLBuilder): for ignore in range(256): uuid = _randomUUID() - if not generatename.libvirt_collision(conn.lookupByUUID, uuid): + if not generatename.check_libvirt_collision( + conn.lookupByUUID, uuid): return uuid logging.error("Failed to generate non-conflicting UUID") diff --git a/virtinst/storage.py b/virtinst/storage.py index 33aadd58..d3741e36 100644 --- a/virtinst/storage.py +++ b/virtinst/storage.py @@ -248,8 +248,6 @@ class StoragePool(_StorageObject): if pool.name == name: return True return False - - kwargs["lib_collision"] = False return generatename.generate_name(basename, cb, **kwargs) @staticmethod @@ -560,7 +558,8 @@ class StorageVolume(_StorageObject): def cb(tryname): if tryname in collidelist: return True - return pool_object.storageVolLookupByName(tryname) + return generatename.check_libvirt_collision( + pool_object.storageVolLookupByName, tryname) StoragePool.ensure_pool_is_running(pool_object, refresh=True) return generatename.generate_name(basename, cb, **kwargs)