diff --git a/instances/test_live_clone.py b/instances/test_live_clone.py
new file mode 100644
index 00000000..174f4f5f
--- /dev/null
+++ b/instances/test_live_clone.py
@@ -0,0 +1,92 @@
+"""Live checks for cloning (R-08). Runs only with TEST_LIBVIRT_HOST set."""
+
+import os
+import unittest
+
+import libvirt
+from django.test import SimpleTestCase
+from vrtManager.connection import connection_manager
+from vrtManager.instance import wvmInstance
+
+from . import livetest
+
+P = livetest.PREFIX
+
+
+class LiveCloneTestCase(SimpleTestCase):
+ @classmethod
+ def setUpClass(cls):
+ if not livetest.enabled():
+ raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests")
+ cls.args = (
+ os.environ["TEST_LIBVIRT_HOST"],
+ os.environ.get("TEST_LIBVIRT_LOGIN", ""),
+ os.environ.get("TEST_LIBVIRT_PASSWORD", ""),
+ int(os.environ.get("TEST_LIBVIRT_TYPE", 4)),
+ )
+ cls.conn = connection_manager.get_connection(*cls.args)
+ livetest.cleanup(cls.conn)
+ livetest.ensure_pool(cls.conn)
+ cls.inventory = livetest.inventory(cls.conn)
+ super().setUpClass()
+
+ @classmethod
+ def tearDownClass(cls):
+ try:
+ livetest.remove_pool(cls.conn)
+ finally:
+ super().tearDownClass()
+ if livetest.inventory(cls.conn) != cls.inventory:
+ raise AssertionError("Live tests changed objects outside the test namespace")
+
+ def tearDown(self):
+ livetest.cleanup(self.conn)
+
+ def pool_volumes(self):
+ pool = self.conn.storagePoolLookupByName(livetest.POOL)
+ pool.refresh(0)
+ return set(pool.listVolumes())
+
+ def source(self, name):
+ disks = [livetest.create_volume(self.conn, P + f"{name}-{d}") for d in ("a", "b")]
+ dom = livetest.define_vm(self.conn, P + name, disks)
+ return dom, wvmInstance(*self.args, None, uuid=dom.UUIDString())
+
+ def clone_data(self, name):
+ return {
+ "name": P + name,
+ "disk-vda": P + name + "-a.qcow2",
+ "disk-vdb": P + name + "-b.qcow2",
+ "disk_owner_uid": 0,
+ "disk_owner_gid": 0,
+ }
+
+ def test_failed_clone_removes_only_its_own_copies(self):
+ _, vm = self.source("clone-src")
+ taken = livetest.create_volume(self.conn, P + "clone-dst-b") # destination of vdb exists
+ before = self.pool_volumes()
+
+ with self.assertRaises(Exception):
+ vm.clone_instance(self.clone_data("clone-dst"))
+
+ self.assertEqual(self.pool_volumes(), before, "copies left behind or a volume deleted")
+ self.assertTrue(livetest.volume_exists(self.conn, taken))
+ with self.assertRaises(libvirt.libvirtError):
+ self.conn.lookupByName(P + "clone-dst")
+
+ def test_running_vm_is_not_cloned(self):
+ dom, vm = self.source("clone-run")
+ dom.create()
+ before = self.pool_volumes()
+
+ with self.assertRaises(Exception):
+ vm.clone_instance(self.clone_data("clone-run-copy"))
+
+ self.assertEqual(self.pool_volumes(), before)
+
+ def test_clone_of_a_stopped_vm(self):
+ _, vm = self.source("clone-ok")
+ uuid = vm.clone_instance(self.clone_data("clone-ok-copy"))
+ clone = self.conn.lookupByUUIDString(uuid)
+ self.assertEqual(clone.name(), P + "clone-ok-copy")
+ self.assertTrue({P + "clone-ok-copy-a.qcow2", P + "clone-ok-copy-b.qcow2"} <= self.pool_volumes())
diff --git a/instances/views.py b/instances/views.py
index bed07f02..449aac71 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -2100,9 +2100,9 @@ def create_instance(request, compute_id, arch, machine):
if conn:
if not storages:
- raise libvirtError(_("You haven't defined any storage pools"))
+ raise util.OperationError(_("You haven't defined any storage pools"))
if not networks:
- raise libvirtError(_("You haven't defined any network pools"))
+ raise util.OperationError(_("You haven't defined any network pools"))
if request.method == "POST":
if "create" in request.POST:
@@ -2117,11 +2117,11 @@ def create_instance(request, compute_id, arch, machine):
meta_prealloc = True
if instances:
if data["name"] in instances:
- raise libvirtError(
+ raise util.OperationError(
_("A virtual machine with this name already exists")
)
if Instance.objects.filter(name__exact=data["name"]):
- raise libvirtError(
+ raise util.OperationError(
_(
"There is an instance with same name. Remove it and try again!"
)
@@ -2129,7 +2129,7 @@ def create_instance(request, compute_id, arch, machine):
if data["hdd_size"]:
if not data["mac"]:
- raise libvirtError(
+ raise util.OperationError(
_("No Virtual Machine MAC has been entered")
)
else:
@@ -2163,7 +2163,7 @@ def create_instance(request, compute_id, arch, machine):
data["name"] + ".img", data["storage"]
)
if dest_vol:
- raise libvirtError(
+ raise util.OperationError(
_(
"Image has already exist. Please check volumes or change instance name"
)
@@ -2193,7 +2193,7 @@ def create_instance(request, compute_id, arch, machine):
is_disk_created = True
else:
if not data["images"]:
- raise libvirtError(
+ raise util.OperationError(
_("First you need to create or select an image")
)
else:
diff --git a/nwfilters/views.py b/nwfilters/views.py
index 1959cf1f..d82d30f6 100644
--- a/nwfilters/views.py
+++ b/nwfilters/views.py
@@ -185,7 +185,7 @@ def nwfilter(request, compute_id, nwfltr):
conn.create_nwfilter(new_xml)
except libvirtError as lib_err:
conn.create_nwfilter(xml)
- raise libvirtError(lib_err)
+ raise util.OperationError(lib_err)
if "del_nwfilter_rule" in request.POST:
action = request.POST.get("action", "")
@@ -198,7 +198,7 @@ def nwfilter(request, compute_id, nwfltr):
conn.create_nwfilter(new_xml)
except libvirtError as lib_err:
conn.create_nwfilter(xml)
- raise libvirtError(lib_err)
+ raise util.OperationError(lib_err)
if "del_nwfilter_ref" in request.POST:
ref_name = request.POST.get("ref")
@@ -208,7 +208,7 @@ def nwfilter(request, compute_id, nwfltr):
conn.create_nwfilter(new_xml)
except libvirtError as lib_err:
conn.create_nwfilter(xml)
- raise libvirtError(lib_err)
+ raise util.OperationError(lib_err)
if "add_nwfilter_rule" in request.POST:
rule_xml = request.POST.get("nwfilterrule_xml", "")
@@ -220,7 +220,7 @@ def nwfilter(request, compute_id, nwfltr):
conn.create_nwfilter(new_xml)
except libvirtError as lib_err:
conn.create_nwfilter(xml)
- raise libvirtError(lib_err)
+ raise util.OperationError(lib_err)
if "add_nwfilter_ref" in request.POST:
ref_name = request.POST.get("nwfilters_select", "")
@@ -232,7 +232,7 @@ def nwfilter(request, compute_id, nwfltr):
conn.create_nwfilter(new_xml)
except libvirtError as lib_err:
conn.create_nwfilter(xml)
- raise libvirtError(lib_err)
+ raise util.OperationError(lib_err)
return HttpResponseRedirect(request.get_full_path())
conn.close()
diff --git a/vrtManager/connection.py b/vrtManager/connection.py
index 093b5ef0..34ff87b9 100644
--- a/vrtManager/connection.py
+++ b/vrtManager/connection.py
@@ -301,7 +301,7 @@ class wvmConnectionManager(object):
return connection.connection
else:
# raise libvirt error
- raise libvirtError(connection.last_error)
+ raise util.OperationError(connection.last_error)
def host_is_up(self, conn_type, hostname):
"""
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index a0054f62..490f3c09 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -140,7 +140,7 @@ class wvmInstances(wvmConnect):
dom_emulator = conn.get_dom_emulator()
if dom_emulator != self.get_emulator(dom_arch):
- raise libvirtError(
+ raise util.OperationError(
"Destination host emulator is different. Cannot be migrated"
)
@@ -721,7 +721,7 @@ class wvmInstance(wvmConnect):
tree = etree.fromstring(self._XMLDesc(xml_flags))
found = tree.xpath("./devices/disk[@device='cdrom'][target/@dev=$dev]", dev=dev)
if not found:
- raise libvirtError(f"CD-ROM {dev} is not in the {label} definition")
+ raise util.OperationError(f"CD-ROM {dev} is not in the {label} definition")
devices.append(found[0])
for (label, _, affect), disk in zip(definitions, devices):
for source in disk.findall("source"):
@@ -732,7 +732,7 @@ class wvmInstance(wvmConnect):
self.instance.updateDeviceFlags(etree.tostring(disk).decode(), affect)
except libvirtError as err:
if label == "persistent" and len(definitions) == 2:
- raise libvirtError(f"Media of {dev} changed in the running VM only: {err}") from err
+ raise util.OperationError(f"Media of {dev} changed in the running VM only: {err}") from err
raise
def mount_iso(self, dev, image):
@@ -742,9 +742,9 @@ class wvmInstance(wvmConnect):
if image in stg.listVolumes():
paths.append(stg.storageVolLookupByName(image).path())
if not paths:
- raise libvirtError(f"ISO image {image} was not found")
+ raise util.OperationError(f"ISO image {image} was not found")
if len(paths) > 1:
- raise libvirtError(f"ISO image {image} is in more than one storage pool")
+ raise util.OperationError(f"ISO image {image} is in more than one storage pool")
self._set_cdrom_media(dev, paths[0])
def umount_iso(self, dev, image=None):
@@ -986,7 +986,7 @@ class wvmInstance(wvmConnect):
parent.remove(vcpu)
self._defineXML(etree.tostring(tree).decode())
else:
- raise libvirtError(
+ raise util.OperationError(
"Please shutdown the instance then try to enable vCPU hotplug"
)
@@ -1505,26 +1505,29 @@ class wvmInstance(wvmConnect):
return ":".join(mac_tuples)
def clone_instance(self, clone_data):
- clone_dev_path = []
-
+ """Copy a shut-off VM. Inputs are checked before anything is allocated,
+ and on failure only the volumes this call created are removed (R-08)."""
+ if self.get_status() != 5:
+ # A running guest keeps writing while its disks are copied.
+ raise util.OperationError("Shut the VM down before cloning it")
if clone_data["name"] in self.get_instances():
raise ValueError("An instance with the clone name already exists")
- xml = self._XMLDesc(VIR_DOMAIN_XML_SECURE)
- tree = etree.fromstring(xml)
- name = tree.find("name")
- name.text = clone_data["name"]
+ tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML))
+ tree.find("name").text = clone_data["name"]
uuid = tree.find("uuid")
if uuid is None:
uuid = etree.SubElement(tree, "uuid")
uuid.text = util.randomUUID()
+ self._set_options(
+ tree,
+ {
+ "title": clone_data.get("clone-title", ""),
+ "description": clone_data.get("clone-description", ""),
+ },
+ )
- options = {
- "title": clone_data.get("clone-title", ""),
- "description": clone_data.get("clone-description", ""),
- }
- self._set_options(tree, options)
-
+ disks = []
for disk in list(tree.findall("devices/disk")):
if disk.get("device") != "disk":
continue
@@ -1539,120 +1542,96 @@ class wvmInstance(wvmConnect):
dev = target.get("dev") if target is not None else None
if not supported or not dev or not clone_data.get("disk-" + dev):
raise ValueError("Cannot clone a disk without a supported source and destination")
-
- src_nvram_path = self.get_nvram()
- if src_nvram_path:
- # Change XML for nvram
- nvram = tree.find("os/nvram")
- nvram.getparent().remove(nvram)
-
- # NVRAM CLONE: create pool if nvram is not in a pool. then clone it
- src_nvram_name = os.path.basename(src_nvram_path)
- nvram_dir = os.path.dirname(src_nvram_path)
- nvram_pool_name = os.path.basename(nvram_dir)
- try:
- self.get_volume_by_path(src_nvram_path)
- except libvirtError:
- stg_conn = self.get_wvmStorages()
- stg_conn.create_storage("dir", nvram_pool_name, None, nvram_dir)
-
- new_nvram_name = f"{clone_data['name']}_VARS"
- nvram_stg = self.get_wvmStorage(nvram_pool_name)
- nvram_stg.clone_volume(src_nvram_name, new_nvram_name, file_suffix="fd")
+ disks.append((source, dev))
for num, net in enumerate(tree.findall("devices/interface")):
- elm = net.find("mac")
- mac_address = self.fix_mac(clone_data["clone-net-mac-" + str(num)])
- elm.set("address", mac_address)
+ mac = clone_data.get(f"clone-net-mac-{num}")
+ if not mac:
+ raise ValueError(f"No MAC address for network interface {num}")
+ net.find("mac").set("address", self.fix_mac(mac))
- for disk in tree.findall("devices/disk"):
- if disk.get("device") == "disk":
- elm = disk.find("target")
- device_name = elm.get("dev")
- if device_name:
- target_file = clone_data["disk-" + device_name]
- meta_prealloc = False
- with contextlib.suppress(Exception):
- meta_prealloc = clone_data["meta-" + device_name]
+ created = [] # paths of volumes this call created
+ try:
+ src_nvram_path = self.get_nvram()
+ if src_nvram_path:
+ nvram = tree.find("os/nvram")
+ nvram.getparent().remove(nvram)
- elm.set("dev", device_name)
+ # NVRAM CLONE: create pool if nvram is not in a pool. then clone it
+ src_nvram_name = os.path.basename(src_nvram_path)
+ nvram_dir = os.path.dirname(src_nvram_path)
+ nvram_pool_name = os.path.basename(nvram_dir)
+ try:
+ self.get_volume_by_path(src_nvram_path)
+ except libvirtError:
+ self.get_wvmStorages().create_storage("dir", nvram_pool_name, None, nvram_dir)
+ nvram_stg = self.get_wvmStorage(nvram_pool_name)
+ name = nvram_stg.clone_volume(src_nvram_name, f"{clone_data['name']}_VARS", file_suffix="fd")
+ created.append(os.path.join(nvram_dir, name))
- elm = disk.find("source")
- source_file = elm.get("file")
- if source_file:
- clone_dev_path.append(source_file)
- clone_path = os.path.join(os.path.dirname(source_file), target_file)
- elm.set("file", clone_path)
-
- vol = self.get_volume_by_path(source_file)
- vol_format = util.get_xml_path(
- vol.XMLDesc(0), "/volume/target/format/@type"
+ for source, dev in disks:
+ target_file = clone_data["disk-" + dev]
+ meta_prealloc = bool(clone_data.get("meta-" + dev))
+ if source.get("file"):
+ vol = self.get_volume_by_path(source.get("file"))
+ vol_format = util.get_xml_path(vol.XMLDesc(0), "/volume/target/format/@type")
+ new_vol = vol.storagePoolLookupByVolume().createXMLFrom(
+ f"""
+ {target_file}
+ 0
+ 0
+
+
+
+ {clone_data['disk_owner_uid']}
+ {clone_data['disk_owner_gid']}
+ 0644
+
+
+ 1.1
+
+
+ """,
+ vol,
+ meta_prealloc and vol_format == "qcow2",
)
-
- if vol_format == "qcow2" and meta_prealloc:
- meta_prealloc = True
-
- vol_clone_xml = f"""
-
- {target_file}
- 0
- 0
-
-
-
- {clone_data['disk_owner_uid']}
- {clone_data['disk_owner_gid']}
- 0644
-
-
- 1.1
-
-
-
-
- """
-
- stg = vol.storagePoolLookupByVolume()
- stg.createXMLFrom(vol_clone_xml, vol, meta_prealloc)
-
- source_protocol = elm.get("protocol")
- if source_protocol == "rbd":
- source_name = elm.get("name")
- clone_name = "%s/%s" % (os.path.dirname(source_name), target_file)
- elm.set("name", clone_name)
-
- vol = self.get_volume_by_path(source_name)
- vol_format = util.get_xml_path(
- vol.XMLDesc(0), "/volume/target/format/@type"
+ created.append(new_vol.path())
+ source.set("file", new_vol.path())
+ elif source.get("protocol") == "rbd":
+ vol = self.get_volume_by_path(source.get("name"))
+ vol_format = util.get_xml_path(vol.XMLDesc(0), "/volume/target/format/@type")
+ new_vol = vol.storagePoolLookupByVolume().createXMLFrom(
+ f"""
+ {target_file}
+ 0
+ 0
+
+ """,
+ vol,
+ meta_prealloc,
)
-
- vol_clone_xml = f"""
-
- {target_file}
- 0
- 0
-
-
-
- """
- stg = vol.storagePoolLookupByVolume()
- stg.createXMLFrom(vol_clone_xml, vol, meta_prealloc)
-
- source_dev = elm.get("dev")
- if source_dev:
- clone_path = os.path.join(os.path.dirname(source_dev), target_file)
- elm.set("dev", clone_path)
-
- vol = self.get_volume_by_path(source_dev)
- stg = vol.storagePoolLookupByVolume()
-
- vol_name = util.get_xml_path(vol.XMLDesc(0), "/volume/name")
- pool_name = util.get_xml_path(stg.XMLDesc(0), "/pool/name")
-
+ created.append(new_vol.path())
+ source.set("name", f"{os.path.dirname(source.get('name'))}/{target_file}")
+ else:
+ vol = self.get_volume_by_path(source.get("dev"))
+ pool_name = util.get_xml_path(vol.storagePoolLookupByVolume().XMLDesc(0), "/pool/name")
storage = self.get_wvmStorage(pool_name)
- storage.clone_volume(vol_name, target_file)
+ name = storage.clone_volume(vol.name(), target_file)
+ new_path = storage.get_volume(name).path()
+ created.append(new_path)
+ source.set("dev", new_path)
- self._defineXML(ElementTree.tostring(tree).decode())
+ self._defineXML(etree.tostring(tree).decode())
+ except Exception as err:
+ leftovers = []
+ for path in reversed(created):
+ try:
+ self.get_volume_by_path(path).delete(0)
+ except libvirtError:
+ leftovers.append(path)
+ if leftovers:
+ raise util.OperationError(f"Clone failed ({err}); remove these copies by hand: {', '.join(leftovers)}") from err
+ raise
return self.get_instance(clone_data["name"]).UUIDString()
@@ -1720,7 +1699,7 @@ class wvmInstance(wvmConnect):
"""Replace the NIC with old_mac; put it back if the new one cannot be added."""
old_xml = self.delete_network(old_mac)
if old_xml is None:
- raise libvirtError(f"No network interface with MAC {old_mac}")
+ raise util.OperationError(f"No network interface with MAC {old_mac}")
try:
self.add_network(mac, source, source_type, model, nwfilter)
except libvirtError:
@@ -1757,11 +1736,11 @@ class wvmInstance(wvmConnect):
elif net_source_type == "iface":
source.set("dev", net_source)
else:
- raise libvirtError(
+ raise util.OperationError(
"Unknown network type: {}".format(net_source_type)
)
else:
- raise libvirtError(
+ raise util.OperationError(
"Unknown network type: {}".format(interface.get("type"))
)
diff --git a/vrtManager/test_util.py b/vrtManager/test_util.py
index 13abd6a6..fb174231 100644
--- a/vrtManager/test_util.py
+++ b/vrtManager/test_util.py
@@ -128,5 +128,21 @@ class TestVrtManagerUtil(unittest.TestCase):
self.assertIsNone(vol_dev_type("unknown_bus"))
+
+class OperationErrorTestCase(unittest.TestCase):
+ def test_keeps_its_own_message_after_a_libvirt_failure(self):
+ import libvirt
+ from vrtManager.util import OperationError
+
+ conn = libvirt.open("test:///default")
+ self.addCleanup(conn.close)
+ with self.assertRaises(libvirt.libvirtError):
+ conn.lookupByName("no-such-domain") # leaves a thread-local last error
+ self.assertNotEqual(str(libvirt.libvirtError("own message")), "own message")
+ error = OperationError("own message")
+ self.assertEqual(str(error), "own message")
+ self.assertIsInstance(error, libvirt.libvirtError)
+
+
if __name__ == "__main__":
unittest.main()
diff --git a/vrtManager/test_xml_safety.py b/vrtManager/test_xml_safety.py
index d7d61bc3..84679803 100644
--- a/vrtManager/test_xml_safety.py
+++ b/vrtManager/test_xml_safety.py
@@ -185,6 +185,47 @@ class TestXmlSafety(unittest.TestCase):
self.assertIn("type='pflash'", inst._snapshotCreateXML.call_args[0][0])
+
+class TestCloneRollback(unittest.TestCase):
+ """R-08: a failed clone removes only the volumes it created."""
+
+ def test_copies_are_removed_when_the_definition_fails(self):
+ import libvirt
+
+ inst = wvmInstance.__new__(wvmInstance)
+ inst.wvm = MagicMock()
+ inst.get_status = MagicMock(return_value=5)
+ inst.get_instances = MagicMock(return_value=[])
+ inst.get_nvram = MagicMock(return_value=None)
+ inst._XMLDesc = MagicMock(return_value=(
+ "src"
+ ""
+ ""
+ ))
+ source_vol = MagicMock()
+ source_vol.XMLDesc.return_value = ""
+ copy = MagicMock()
+ copy.path.return_value = "/pool/copy.qcow2"
+ source_vol.storagePoolLookupByVolume.return_value.createXMLFrom.return_value = copy
+ inst.get_volume_by_path = MagicMock(side_effect=lambda path: copy if path == "/pool/copy.qcow2" else source_vol)
+ inst._defineXML = MagicMock(side_effect=libvirt.libvirtError("bad definition"))
+
+ with self.assertRaises(libvirt.libvirtError):
+ inst.clone_instance({"name": "copy", "disk-vda": "copy.qcow2", "disk_owner_uid": 0, "disk_owner_gid": 0})
+ copy.delete.assert_called_once_with(0)
+ source_vol.delete.assert_not_called()
+
+ def test_running_vm_is_refused_before_anything_is_read(self):
+ import libvirt
+
+ inst = wvmInstance.__new__(wvmInstance)
+ inst.get_status = MagicMock(return_value=1)
+ inst._XMLDesc = MagicMock()
+ with self.assertRaisesRegex(libvirt.libvirtError, "Shut the VM down"):
+ inst.clone_instance({"name": "copy"})
+ inst._XMLDesc.assert_not_called()
+
+
if __name__ == "__main__":
unittest.main()
@@ -379,6 +420,7 @@ class TestEmptyDiskClone(unittest.TestCase):
# Exercise defensive handling of incomplete disk metadata, while the
# clone is still defined and looked up through the real test driver.
inst._XMLDesc = MagicMock(return_value=etree.tostring(config).decode())
+ inst.get_status = MagicMock(return_value=5) # only shut-off VMs are cloned
clone_uuid = inst.clone_instance({"name": "empty-disk-clone"})
clone = conn.lookupByUUIDString(clone_uuid)
self.addCleanup(clone.undefine)
@@ -396,6 +438,7 @@ class TestEmptyDiskClone(unittest.TestCase):
inst.wvm = conn
inst.instance = conn.lookupByName("test")
inst._XMLDesc = MagicMock()
+ inst.get_status = MagicMock(return_value=5)
with self.assertRaisesRegex(ValueError, "already exists"):
inst.clone_instance({"name": "test"})
inst._XMLDesc.assert_not_called()
diff --git a/vrtManager/util.py b/vrtManager/util.py
index 7ec27f80..26ca01f9 100644
--- a/vrtManager/util.py
+++ b/vrtManager/util.py
@@ -9,6 +9,19 @@ import lxml.etree as etree
from django.conf import settings
+
+class OperationError(libvirt.libvirtError):
+ """An error with WebVirtCloud's own message.
+
+ libvirtError(msg) reports the thread's last libvirt error instead of msg
+ when there is one, so an earlier, unrelated failure would be shown.
+ """
+
+ def __init__(self, msg):
+ Exception.__init__(self, msg)
+ self.err = None
+
+
def is_kvm_available(xml):
kvm_domains = get_xml_path(xml, "//domain/@type='kvm'")
return kvm_domains > 0