guest: Move VM replace helper to cloner

It's the only user. Rework it a bit and add full coverage

Signed-off-by: Cole Robinson <crobinso@redhat.com>

3 files changed, 27 insertions, 36 deletions

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 04f196cd..e5436a20 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1319,7 +1319,7 @@ c.add_invalid("--connect %(URI-TEST-REMOTE)s -o test-clone-simple --auto-clone -
 
 
 c = vclon.add_category("general", "-n clonetest")
-c.add_valid("-o test --auto-clone")  # Auto flag, no storage
+c.add_valid("-o test --auto-clone --replace")  # Auto flag, no storage, --replace is redundant
 c.add_valid("-o test --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s")  # Nodisk, but with spurious files passed
 c.add_valid("-o test --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s --prompt")  # Working scenario w/ prompt shouldn't ask anything
 c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s")  # XML File with 2 disks
@@ -1330,7 +1330,7 @@ c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --file %(NEWCLONEIMG1)s --f
 c.add_valid("--original-xml " + _CLONE_MANAGED + " --file %(NEWIMG1)s")  # XML w/ managed storage, specify managed path
 c.add_valid("--original-xml " + _CLONE_MANAGED + " --file %(NEWIMG1)s --reflink")  # XML w/ managed storage, specify managed path
 c.add_valid("--original-xml " + _CLONE_NOEXIST + " --file %(EXISTIMG1)s --preserve")  # XML w/ managed storage, specify managed path across pools# Libvirt test driver doesn't support cloning across pools# XML w/ non-existent storage, with --preserve
-c.add_valid("--connect %(URI-TEST-FULL)s -o test -n test-clone --auto-clone --replace")  # Overwriting existing VM
+c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone -n test --auto-clone --replace")  # Overwriting existing running VM
 c.add_invalid("-o test foobar")  # Positional arguments error
 c.add_invalid("-o idontexist")  # Non-existent vm name
 c.add_invalid("-o idontexist --auto-clone")  # Non-existent vm name with auto flag,
diff --git a/virtinst/cloner.py b/virtinst/cloner.py
index ac6987b0..97712837 100644
--- a/virtinst/cloner.py
+++ b/virtinst/cloner.py
@@ -23,6 +23,29 @@ from .storage import StorageVolume
 from .devices import DeviceChannel
 
 
+def _replace_vm(conn, name):
+    """
+    Remove the existing VM with the same name if requested
+    """
+    try:
+        vm = conn.lookupByName(name)
+    except libvirt.libvirtError:
+        return
+
+    try:
+
+        log.debug("Explicitly replacing guest '%s'", name)
+        if vm.ID() != -1:
+            log.debug("Destroying guest '%s'", name)
+            vm.destroy()
+
+        log.debug("Undefining guest '%s'", name)
+        vm.undefine()
+    except libvirt.libvirtError as e:  # pragma: no cover
+        raise RuntimeError(_("Could not remove old vm '%s': %s") %
+                           (str(e)))
+
+
 class Cloner(object):
 
     # Reasons why we don't default to cloning.
@@ -447,8 +470,8 @@ class Cloner(object):
         dom = None
         try:
             # Replace orig VM if required
-            Guest.check_vm_collision(self.conn, self.clone_name,
-                                     do_remove=self.replace)
+            if self.replace:
+                _replace_vm(self.conn, self.clone_name)
 
             # Define domain early to catch any xml errors before duping storage
             dom = self.conn.defineXML(self.clone_xml)
diff --git a/virtinst/guest.py b/virtinst/guest.py
index 74f4fa85..7ab35420 100644
--- a/virtinst/guest.py
+++ b/virtinst/guest.py
@@ -8,8 +8,6 @@
 
 import random
 
-import libvirt
-
 from . import generatename
 from . import xmlutil
 from .buildconfig import BuildConfig
@@ -73,36 +71,6 @@ class _IOThreadID(XMLBuilder):
 
 class Guest(XMLBuilder):
     @staticmethod
-    def check_vm_collision(conn, name, do_remove):
-        """
-        Remove the existing VM with the same name if requested, or error
-        if there is a collision.
-        """
-        vm = None
-        try:
-            vm = conn.lookupByName(name)
-        except libvirt.libvirtError:
-            pass
-
-        if vm is None:
-            return
-
-        if not do_remove:
-            raise RuntimeError(_("Domain named %s already exists!") % name)
-
-        try:
-            log.debug("Explicitly replacing guest '%s'", name)
-            if vm.ID() != -1:
-                log.debug("Destroying guest '%s'", name)
-                vm.destroy()
-
-            log.debug("Undefining guest '%s'", name)
-            vm.undefine()
-        except libvirt.libvirtError as e:
-            raise RuntimeError(_("Could not remove old vm '%s': %s") %
-                               (str(e)))
-
-    @staticmethod
     def validate_name(conn, name, check_collision=True, validate=True):
         if validate:
             XMLBuilder.validate_generic_name(_("Guest"), name)