From 400ba1614f29b38f2bd4f29fa132c7f8f2fd21e9 Mon Sep 17 00:00:00 2001
From: catborise <catborise@gmail.com>
Date: Fri, 30 Oct 2026 13:32:28 +0300
Subject: [PATCH] feat(instances): a VM's clone form in device cards, with
 fixes to what it clones

Settings > Clone was a horizontal form with unlabelled checkboxes. The
owner chose this layout from three rendered options: the clone's name
on top, a "Network" card with a new MAC address per NIC, a "Storage"
card with a new volume name per disk, then the optional title and
description and a footer with Clone.

Codex reviewed cloning end to end; these are fixed:

- Every NIC of the clone gets a new random MAC when the form opens.
  Only the first did, so a VM with two NICs gave its clone the second
  NIC's MAC. Guess MAC is offered for the first NIC only: a name's DHCP
  host entry holds one address.
- A VM without a NIC can be cloned: the first NIC's MAC was required.
- The disk name script uses the persistent disks, as the form does; a
  disk added pending the next start kept the source's volume name.
- A running or paused VM is not cloned while a copied disk changes at
  the next start: the backup copies the running disk, so the clone
  would get the old contents under the new definition.
- A derived volume name that does not start with the source's name now
  starts with the clone's name and the disk's device, so a second clone
  or a second disk does not take the same name.
- Guess for the clone name says when the DHCP file has no free name; it
  stopped with a script error.
- The qcow2 metadata preallocation box shows only for a shut-off VM: a
  live clone's backup ignores it.

Other users no longer get hidden MAC and disk inputs or the scripts that
filled them: the server derives those values and ignored the posted ones.
---
 instances/jobs.py                             |   5 +-
 instances/templates/instance.html             |  26 ++-
 .../templates/instances/settings_tab.html     | 154 +++++++++---------
 instances/tests/test_clone_permission.py      |  25 ++-
 instances/tests/test_pending_edits.py         |  17 ++
 instances/utils.py                            |  11 +-
 vrtManager/instance.py                        |   5 +
 vrtManager/tests/test_live_clone.py           |  17 +-
 8 files changed, 164 insertions(+), 96 deletions(-)

diff --git a/instances/jobs.py b/instances/jobs.py
index bf467460..2cc43e3c 100644
--- a/instances/jobs.py
+++ b/instances/jobs.py
@@ -141,14 +141,13 @@ def clone_data(user, instance, fields):
     if app_settings.CLONE_INSTANCE_AUTO_NAME == "True" and not data["name"]:
         auto_vname = utils.get_clone_free_names()[0]
         data["name"] = auto_vname
-        data["clone-net-mac-0"] = utils.get_dhcp_mac_address(auto_vname)
         for disk in instance.config_disks:
             data[f"disk-{disk['dev']}"] = utils.get_clone_disk_name(disk, instance.name, auto_vname)
 
     if not user.is_superuser:
         for disk in instance.config_disks:
             data[f"disk-{disk['dev']}"] = utils.get_clone_disk_name(disk, instance.name, data["name"])
-        for num in range(max(1, len(instance.config_networks))):
+        for num in range(len(instance.config_networks)):
             key = f"clone-net-mac-{num}"
             if not data.get(key):
                 data[key] = (utils.get_dhcp_mac_address(data["name"]) if num == 0 else "") or utils.get_random_mac_address()
@@ -173,7 +172,7 @@ def clone_data(user, instance, fields):
     if not re.match(r"^[a-zA-Z0-9-]+$", data["name"]):
         raise util.OperationError(
             _("Instance name '%(clone_name)s' contains invalid characters!") % {"clone_name": data["name"]})
-    if "clone-net-mac-0" not in data or invalid_macs:
+    if invalid_macs:
         raise util.OperationError(_("Instance MAC '%(clone_mac)s' invalid format!") % {"clone_mac": ", ".join(invalid_macs)})
     if invalid_disks:
         raise util.OperationError(
diff --git a/instances/templates/instance.html b/instances/templates/instance.html
index 9a37acb6..955df7eb 100644
--- a/instances/templates/instance.html
+++ b/instances/templates/instance.html
@@ -152,6 +152,12 @@
 <script>
     function guess_clone_name() {
         $.getJSON("{% url 'instances:guess_clone_name' %}", function(data) {
+            // a free name of the DHCP file, if there is one
+            if (!data.name) {
+                $('#clone-name-help').text("{{ _("The DHCP file has no free name to suggest.")|escapejs }}");
+                return;
+            }
+            $('#clone-name-help').text('');
             guessed_name = data['name'].split(".")[0];
             $('#clone_name').val(guessed_name);
             update_clone_disk_name(guessed_name);
@@ -161,9 +167,9 @@
 </script>
 <script>
     function update_clone_disk_name(new_vname) {
-        vname = '{{ instance.name }}';
-        {% for disk in instance.disks %}
-            disk_name = '{{ disk.image }}';
+        vname = '{{ instance.name|escapejs }}';
+        {% for disk in instance.config_disks %}
+            disk_name = '{{ disk.image|escapejs }}';
             disk_dot = disk_name.split('.');
             disk_dot_suffix = disk_dot[disk_dot.length-1];
             if (disk_name.startsWith(vname)) {
@@ -177,7 +183,7 @@
             } else {
                 image = new_vname + '-clone';
             }
-            $('#disk_name-{{ disk.dev }}').val(image);
+            $('#disk_name-{{ disk.dev|escapejs }}').val(image);
         {% endfor %}
     }
 </script>
@@ -203,19 +209,11 @@
 {% endif %}
 {% if request.user.is_superuser %}
     $(document).ready(function () {
-        random_mac('clone-net-mac-0');
+        // every NIC of the clone a new address: one the VM has would clash with it
+        $('input[name^="clone-net-mac-"]').each(function () { random_mac(this.name); });
         random_mac('add-net-mac');
         update_clone_disk_name($('#clone_name').val());
     });
-{% else %}
-    $('#select_clone_name').on('change', function () {
-        update_clone_disk_name($(this).val());
-        guess_mac_address('#select_clone_name', 0);
-    });
-    $(document).ready(function () {
-        update_clone_disk_name($('#select_clone_name').val());
-        guess_mac_address('#select_clone_name', 0);
-    });
 {% endif %}
 </script>
 <script>
diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html
index 08753a2e..d00e75d9 100644
--- a/instances/templates/instances/settings_tab.html
+++ b/instances/templates/instances/settings_tab.html
@@ -692,97 +692,105 @@
         {% endif %}
         {% if can_clone %}
         <div role="tabpanel" class="tab-pane" id="clone">
-            <p class="fw-bold">{% trans "Create a clone" %}</p>
-            {% if usb_hostdevs or pci_hostdevs or other_hostdevs %}<p class="small text-body-secondary">{% trans "The clone gets none of the VM's host devices: one VM holds a device of the host." %}</p>{% endif %}
-            <form class="form" action="{% url 'instances:clone' instance.id %}" method="post" role="form" aria-label="Create clone form">{% csrf_token %}{% submission_key %}
-                <div class="row">
-                    <label class="col-sm-3 col-form-label fw-normal">{% trans "Clone Name" %}</label>
+            <form action="{% url 'instances:clone' instance.id %}" method="post" aria-labelledby="clone-heading">{% csrf_token %}{% submission_key %}
+                <header class="mb-4">
+                    <h2 class="h6 mb-1" id="clone-heading">{% trans "Create a clone" %}</h2>
+                    <p class="small text-body-secondary mb-0">{% blocktrans with name=instance.name %}Source VM: {{ name }}{% endblocktrans %}</p>
+                </header>
+                <div class="mb-4">
                     {% if request.user.is_superuser %}
-                    <div class="col-sm-6">
-                        <div class="input-group">
-                            <input id="clone_name" type="text" class="form-control" name="name" value="{{ instance.name }}-clone">
-                            <button type="button" class="btn btn-primary" name="guess-clone-name"
-                                        onclick="guess_clone_name()">{% trans "Guess" %}</button>
+                    <label class="form-label" for="clone_name">{% trans "Clone name" %}</label>
+                    <div class="row g-2">
+                        <div class="col-12 col-sm">
+                            <input class="form-control" id="clone_name" name="name" type="text" value="{{ instance.name }}-clone" aria-describedby="clone-name-help">
+                        </div>
+                        <div class="col-12 col-sm-auto">
+                            <button class="btn btn-secondary" type="button" onclick="guess_clone_name()">{% trans "Guess" %}</button>
                         </div>
                     </div>
-                    {% elif app_settings.CLONE_INSTANCE_AUTO_NAME == 'True'%}
-                    <div class="col-sm-4">
-                        <input id="clone_instance_auto_name" type="text" class="form-control" name="auto_name" value="Automatic" disabled/>
-                    </div>
+                    <p class="small text-body-secondary mt-1 mb-0" id="clone-name-help"></p>
+                    {% elif app_settings.CLONE_INSTANCE_AUTO_NAME == 'True' %}
+                    <label class="form-label" for="clone_instance_auto_name">{% trans "Clone name" %}</label>
+                    <input class="form-control" id="clone_instance_auto_name" type="text" value="{% trans "Automatic" %}" disabled>
                     {% else %}
-                    <div class="col-sm-4">
-                        <select id="select_clone_name" class="form-control" name="name" size="1">
+                    <label class="form-label" for="select_clone_name">{% trans "Clone name" %}</label>
+                    <select class="form-select" id="select_clone_name" name="name">
                         {% for name in clone_free_names %}
-                            <option value="{{ name }}">{{ name }}</option>
+                        <option value="{{ name }}">{{ name }}</option>
                         {% endfor %}
-                        </select>
-                    </div>
+                    </select>
                     {% endif %}
                 </div>
                 {% if request.user.is_superuser %}
-                    <label>{% trans "Network devices" %}:</label>
-                    {% for network in instance.config_networks %}
-                    <div class="mb-3">
-                        <div class="row">
-                            <label class="col-sm-2 col-form-label offset-sm-1">eth{{ forloop.counter0 }} ({{ network.nic }})</label>
-                            <div class="col-sm-6">
-                                <div class="input-group">
-                                    <input type="text" class="form-control" name="clone-net-mac-{{ forloop.counter0 }}" value="{{ network.mac }}"/>
-                                    <button type="button" class="btn btn-primary" name="random-mac-{{ forloop.counter0 }}"
-                                            onclick="random_mac('clone-net-mac-{{ forloop.counter0 }}')">{% trans "Random" %}</button>
-                                    <button type="button" class="btn btn-primary" name="guess-mac-{{ forloop.counter0 }}"
-                                            onclick="guess_mac_address('#clone_name', {{ forloop.counter0 }})">{% trans "Guess" %}</button>
+                <div class="row g-3 mb-4">
+                    {% if instance.config_networks %}
+                    <div class="col-12 col-lg-6">
+                        <section class="card h-100" aria-labelledby="clone-network-heading">
+                            <div class="card-body">
+                                <h3 class="h6 mb-3" id="clone-network-heading">{% bs_icon 'ethernet' %} {% trans "Network" %}</h3>
+                                {% for network in instance.config_networks %}
+                                <div{% if not forloop.first %} class="border-top pt-3 mt-3"{% endif %}>
+                                    <p class="small mb-2"><strong>eth{{ forloop.counter0 }}</strong> <span class="text-body-secondary">· {{ network.nic }}</span></p>
+                                    <label class="form-label small" for="clone-net-mac-{{ forloop.counter0 }}">{% trans "New MAC address" %}</label>
+                                    <input class="form-control" id="clone-net-mac-{{ forloop.counter0 }}" name="clone-net-mac-{{ forloop.counter0 }}" type="text" value="{{ network.mac }}">
+                                    <div class="d-flex flex-wrap gap-2 mt-2">
+                                        <button class="btn btn-sm btn-secondary" type="button" onclick="random_mac('clone-net-mac-{{ forloop.counter0 }}')">{% trans "Random" %}</button>
+                                        {% if forloop.first %}
+                                        {# the DHCP host entry of the clone's name holds one address: the first NIC's #}
+                                        <button class="btn btn-sm btn-secondary" type="button" onclick="guess_mac_address('#clone_name', 0)">{% trans "Guess" %}</button>
+                                        {% endif %}
+                                    </div>
                                 </div>
+                                {% endfor %}
                             </div>
-                        </div>
+                        </section>
                     </div>
-                    {% endfor %}
-                {% else %}
-                    {% for network in instance.config_networks %}
-                        <input type="hidden" class="form-control" name="clone-net-mac-{{ forloop.counter0 }}" value="{{ network.mac }}"/>
-                    {% endfor %}
-                {% endif %}
-                {% if request.user.is_superuser %}
-                    <label>{% trans "Storage devices" %}:</label>
-                    {% for disk in instance.config_disks %}
-                        <div class="row">
-                            <label class="col-sm-2 col-form-label offset-sm-1">{{ disk.dev }} ({{ disk.storage }})</label>
-                            <div class="col-sm-6">
-                            <div class="input-group mb-3">
-                                    <input id="disk_name-{{ disk.dev }}" type="text" class="form-control" name="disk-{{ disk.dev }}" value="{{ disk.image }}"/>
-                                    {% if disk.format == 'qcow2' %}
-                                            <span class="input-group-text" >{% trans 'Metadata' %}</span>
-                                            <div class="input-group-text">
-                                                <input type="checkbox" name="meta-{{ disk.dev }}" value="true">
-                                            </div>
-                                        
+                    {% endif %}
+                    <div class="col-12{% if instance.config_networks %} col-lg-6{% endif %}">
+                        <section class="card h-100" aria-labelledby="clone-storage-heading">
+                            <div class="card-body">
+                                <h3 class="h6 mb-3" id="clone-storage-heading">{% bs_icon 'device-hdd' %} {% trans "Storage" %}</h3>
+                                {% for disk in instance.config_disks %}
+                                <div{% if not forloop.first %} class="border-top pt-3 mt-3"{% endif %}>
+                                    <p class="small mb-2"><strong>{{ disk.dev }}</strong> <span class="text-body-secondary">· {% blocktrans with pool=disk.storage %}Pool: {{ pool }}{% endblocktrans %} · {{ disk.format }}</span></p>
+                                    <label class="form-label small" for="disk_name-{{ disk.dev }}">{% trans "New volume name" %}</label>
+                                    <input class="form-control" id="disk_name-{{ disk.dev }}" name="disk-{{ disk.dev }}" type="text" value="{{ disk.image }}">
+                                    {# a running or paused VM's disks are copied by a backup job, which does not preallocate #}
+                                    {% if disk.format == 'qcow2' and instance.status == 5 %}
+                                    <div class="form-check mt-2">
+                                        <input class="form-check-input" id="meta-{{ disk.dev }}" name="meta-{{ disk.dev }}" type="checkbox" value="true">
+                                        <label class="form-check-label small" for="meta-{{ disk.dev }}">{% trans "Preallocate qcow2 metadata" %}</label>
+                                    </div>
                                     {% endif %}
                                 </div>
+                                {% empty %}
+                                <p class="small text-body-secondary mb-0">{% trans "The VM has no disk to copy." %}</p>
+                                {% endfor %}
                             </div>
-                        </div>
-                    {% endfor %}
-                {% else %}
-                    {% for disk in instance.disks %}
-                        <input id="disk_name-{{ disk.dev }}" type="hidden" class="form-control" name="disk-{{ disk.dev }}" value="{{ disk.image }}"/>
-                    {% endfor %}
-                {% endif %}
-                <div class="row mb-3">
-                    <label class="col-sm-3 col-form-label">{% trans "Title" %}</label>
-                    <div class="col-sm-6">
-                        <input type="text" name="clone-title" class="form-control">
+                        </section>
                     </div>
                 </div>
-                <div class="row">
-                    <label class="col-sm-3 col-form-label">{% trans "Description" %}</label>
-                    <div class="col-sm-6">
-                        <textarea name="clone-description" class="form-control"></textarea>
-                    </div>
-                </div>
-                {% if instance.status == 1 or instance.status == 3 or instance.status == 5 %}
-                    <button type="submit" class="btn btn-primary float-end" name="clone">{% trans "Clone" %}</button>
-                {% else %}
-                    <button class="btn btn-primary float-end disabled" disabled name="clone">{% trans "Clone" %}</button>
                 {% endif %}
+                <fieldset class="mb-4">
+                    <legend class="h6">{% trans "Optional details" %}</legend>
+                    <label class="form-label" for="clone-title">{% trans "Title" %}</label>
+                    <input class="form-control mb-3" id="clone-title" name="clone-title" type="text">
+                    <label class="form-label" for="clone-description">{% trans "Description" %}</label>
+                    <textarea class="form-control" id="clone-description" name="clone-description" rows="3"></textarea>
+                </fieldset>
+                <footer class="border-top pt-3">
+                    {% if usb_hostdevs or pci_hostdevs or other_hostdevs %}
+                    <p class="small text-body-secondary mb-2">{% trans "The clone gets none of the VM's host devices: one VM holds a device of the host." %}</p>
+                    {% endif %}
+                    <div class="d-flex flex-wrap align-items-center justify-content-between gap-3">
+                        <p class="small text-body-secondary mb-0">{% trans "Cloning runs as a background job." %}</p>
+                        {% if instance.status == 1 or instance.status == 3 or instance.status == 5 %}
+                        <button type="submit" class="btn btn-primary ms-auto" name="clone">{% trans "Clone" %}</button>
+                        {% else %}
+                        <button type="button" class="btn btn-primary ms-auto" disabled name="clone">{% trans "Clone" %}</button>
+                        {% endif %}
+                    </div>
+                </footer>
             </form>
             <div class="clearfix"></div>
         </div>
diff --git a/instances/tests/test_clone_permission.py b/instances/tests/test_clone_permission.py
index 59a802bb..4933b72d 100644
--- a/instances/tests/test_clone_permission.py
+++ b/instances/tests/test_clone_permission.py
@@ -7,6 +7,7 @@ from contextlib import contextmanager
 from pathlib import Path
 from unittest.mock import patch
 
+from appsettings.models import AppSettings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
 from django.test import TestCase, override_settings
@@ -170,6 +171,19 @@ class ClonePermissionTestCase(TestCase):
             )
         self.assertFalse(Operation.objects.exists())
 
+    def test_a_vm_without_a_nic_is_cloned(self):
+        for user, auto_name in ((self.superuser, "False"), (self.owner_change, "False"), (self.owner_change, "True")):
+            with self.subTest(user.username, auto_name=auto_name), self.mocked_libvirt() as proxy, patch(
+                "instances.views.utils.get_clone_free_names", return_value=["auto-clone"]
+            ):
+                AppSettings.objects.filter(key="CLONE_INSTANCE_AUTO_NAME").update(value=auto_name)
+                proxy.get_net_devices.return_value = []
+                Operation.objects.all().delete()
+                data = self._valid_post(name="" if auto_name == "True" else "clone-vm")
+                del data["clone-net-mac-0"]
+                self._clone(user, data)
+                self.assertNotIn("clone-net-mac-0", Operation.objects.get().intent["clone"])
+
     def test_superuser_valid_clone_passes_posted_values(self):
         with self.mocked_libvirt():
             self._clone(
@@ -191,14 +205,23 @@ class ClonePermissionTestCase(TestCase):
         self.assertEqual(clone_data["disk-vda"], "clone-vm.qcow2")
         self.assertIsNone(clone_data["disk-vdb"])
 
+    def test_derived_disk_names_differ(self):
+        disks = [dict(SRC_DISK, dev="vdb", image="src-vm-data.qcow2"), dict(SRC_DISK, dev="vdc", image="data.qcow2")]
+        with self.mocked_libvirt() as proxy:
+            proxy.get_disk_devices.return_value = disks
+            self._clone(self.owner_change, self._valid_post())
+        clone_data = Operation.objects.get().intent["clone"]
+        self.assertEqual((clone_data["disk-vdb"], clone_data["disk-vdc"]), ("clone-vm-data.qcow2", "clone-vm-vdc-data.qcow2"))
+
     def test_derived_disk_names_are_sanitized_instead_of_rejected(self):
         odd_disk = dict(SRC_DISK, image="my disk@2.qcow2")
         with self.mocked_libvirt() as proxy:
             proxy.get_disk_devices.return_value = [odd_disk]
             self._clone(self.owner_change, self._valid_post())
         self.assertEqual(Operation.objects.count(), 1)
+        # the clone's name leads, so a second clone's disk does not take the first one's name
         self.assertEqual(
-            Operation.objects.get().intent["clone"]["disk-vda"], "my-disk-2-clone.qcow2"
+            Operation.objects.get().intent["clone"]["disk-vda"], "clone-vm-vda-my-disk-2.qcow2"
         )
 
 
diff --git a/instances/tests/test_pending_edits.py b/instances/tests/test_pending_edits.py
index 21632ec3..0d2cc8de 100644
--- a/instances/tests/test_pending_edits.py
+++ b/instances/tests/test_pending_edits.py
@@ -97,6 +97,23 @@ class PendingEditsTestCase(TestCase):
                 self.assertEqual(doc.xpath("//div[@id='disks']//div[article[starts-with(@aria-labelledby, 'disk-')]]/@class"),
                                  wide)
 
+    def test_the_clone_form_lists_the_persistent_devices(self):
+        nic = {"type": "network", "mac": "52:54:10:00:00:01", "nic": "default", "state": "up", "model": "virtio",
+               "ipv4": None, "ipv6": None, "filterref": "", "target": "", "inbound": [], "outbound": []}
+        nics = [nic, {**nic, "mac": "52:54:10:00:00:02"}]
+        for status, metadata in ((1, []), (5, ["meta-vda", "meta-vdb"])):
+            with self.subTest(status=status):
+                # running: vdb is pending the next start
+                doc = self.page(live=[disk("vda", "default", "")], config=[disk("vda", "default", ""), disk("vdb", "default", "")],
+                                nics=([nic], nics), status=status)
+                (form,) = doc.xpath("//form[@action=$a]", a=reverse("instances:clone", args=[self.instance.id]))
+                self.assertEqual(form.xpath(".//input[@type='text']/@name"),
+                                 ["name", "clone-net-mac-0", "clone-net-mac-1", "disk-vda", "disk-vdb", "clone-title"])
+                # the DHCP entry of the clone's name holds one address: only the first NIC guesses it
+                self.assertEqual(len(form.xpath(".//button[contains(@onclick, 'guess_mac_address')]")), 1)
+                # a running or paused VM is copied by a backup, which does not preallocate
+                self.assertEqual(form.xpath(".//input[@type='checkbox']/@name"), metadata)
+
     def test_a_disk_with_a_pending_source_is_not_deleted(self):
         # running: vda is A, its definition for the next start uses B
         with patch("instances.models.wvmInstance") as wvm, patch("instances.views.wvmStorage") as storage:
diff --git a/instances/utils.py b/instances/utils.py
index 9a7489b6..6abbf570 100644
--- a/instances/utils.py
+++ b/instances/utils.py
@@ -358,17 +358,20 @@ def get_random_mac_address():
     return mac
 
 
-def get_clone_disk_name(disk, prefix, clone_name=""):
+def get_clone_disk_name(disk, prefix, clone_name):
+    """A volume name for the clone's copy of disk: the clone's name in place of
+    the source's (prefix), else before the disk's device and name, so two disks
+    of one clone do not get the same name."""
     if not disk["image"]:
         return None
-    if disk["image"].startswith(prefix) and clone_name:
+    if disk["image"].startswith(prefix):
         suffix = disk["image"][len(prefix) :]
         image = f"{clone_name}{suffix}"
     elif "." in disk["image"] and len(disk["image"].rsplit(".", 1)[1]) <= 7:
         name, suffix = disk["image"].rsplit(".", 1)
-        image = f"{name}-clone.{suffix}"
+        image = f"{clone_name}-{disk['dev']}-{name}.{suffix}"
     else:
-        image = f"{disk['image']}-clone"
+        image = f"{clone_name}-{disk['dev']}-{disk['image']}"
     # Derived names go into volume XML and paths: keep them to a safe charset.
     image = re.sub(r"[^A-Za-z0-9_.+-]+", "-", image).lstrip("_.+-")
     return image or None
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index 046f4c07..e1ca942a 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -2467,6 +2467,11 @@ class wvmInstance(wvmConnect):
                 raise util.OperationError(f"Disk {dev} is not a local file; shut the VM down to clone it")
             if source.getparent().find("shareable") is not None:
                 raise util.OperationError(f"Disk {dev} is shareable; shut the VM down to clone it")
+        # the backup copies the running disks: one the next start changes would give the clone the wrong data
+        live = etree.fromstring(self._XMLDesc(0))
+        for source, dev in disks:
+            if live.xpath("devices/disk[target/@dev=$dev]/source/@file", dev=dev) != [source.get("file")]:
+                raise util.OperationError(f"Disk {dev} changes at the next start; shut the VM down to clone it")
         if self.instance.jobInfo()[0] != VIR_DOMAIN_JOB_NONE:
             raise util.OperationError("Another job is running on the VM; clone it when that ends")
 
diff --git a/vrtManager/tests/test_live_clone.py b/vrtManager/tests/test_live_clone.py
index 4648bd12..f1f19434 100644
--- a/vrtManager/tests/test_live_clone.py
+++ b/vrtManager/tests/test_live_clone.py
@@ -9,7 +9,12 @@ from lxml import etree
 
 from vrtManager import instance as instance_module
 from vrtManager import util
-from vrtManager.instance import LIVE_CLONE_MIN_LIBVIRT, VIR_DOMAIN_BACKUP_BEGIN_PRESERVE_SHUTDOWN_DOMAIN, wvmInstance
+from vrtManager.instance import (
+    LIVE_CLONE_MIN_LIBVIRT,
+    PERSISTENT_XML,
+    VIR_DOMAIN_BACKUP_BEGIN_PRESERVE_SHUTDOWN_DOMAIN,
+    wvmInstance,
+)
 
 GIB = 1024**3
 UUID = "abababab-0000-1111-2222-333333333333"
@@ -98,6 +103,16 @@ class LiveCloneTargetsTestCase(unittest.TestCase):
     def test_a_tpm_is_refused(self):
         self.refused(proxy(domain(extra="<tpm model='tpm-crb'/>")), "TPM")
 
+    def test_a_disk_the_next_start_changes_is_refused(self):
+        # the backup copies the running disk: a pending new source, or a disk only the next start has
+        disk = "<disk type='file' device='disk'><source file='/pool/{}.qcow2'/><target dev='{}'/></disk>"
+        for persistent in (domain(disk.format("new", "vda")), domain(disk.format("src", "vda") + disk.format("add", "vdb"))):
+            with self.subTest(persistent):
+                p = proxy()
+                p._XMLDesc = lambda flags, xml=persistent: xml if flags == PERSISTENT_XML else domain()
+                with self.assertRaisesRegex(util.OperationError, "changes at the next start"):
+                    p.live_clone_targets({**FORM, "disk-vdb": "copy-vdb.qcow2"}, 10, 5)
+
     def test_a_running_libvirt_job_is_refused(self):
         p = proxy()
         p.instance.jobInfo.return_value = [libvirt.VIR_DOMAIN_JOB_UNBOUNDED] + [0] * 11