+ data-failed="{% trans "The request did not go through; your edit is still in the editor. Try again, or copy it and reload the page." %}">
diff --git a/instances/tests/test_change_xml.py b/instances/tests/test_change_xml.py
index 058d0d82..88515de7 100644
--- a/instances/tests/test_change_xml.py
+++ b/instances/tests/test_change_xml.py
@@ -9,10 +9,12 @@ from instances.models import Instance
from libvirt import libvirtError
from vrtManager.instance import PERSISTENT_XML
from logs.models import Logs
+from operations import claims
-CURRENT = "\n xml-vm\n 1024\n"
-LIVE = "\n xml-vm\n 512\n"
-EDITED = "\n xml-vm\n 2048\n"
+UUID = "11111111-2222-3333-4444-888888888888"
+CURRENT = f"\n xml-vm\n {UUID}\n 1024\n"
+LIVE = f"\n xml-vm\n {UUID}\n 512\n"
+EDITED = CURRENT.replace("1024", "2048")
class ChangeXmlTestCase(TestCase):
@@ -23,7 +25,7 @@ class ChangeXmlTestCase(TestCase):
def setUp(self):
self.client.force_login(get_user_model().objects.create_superuser("xml_admin", "x@example.com", "pw"))
compute = Compute.objects.create(name="xml-compute", hostname="127.0.0.1", login="", password="", type=4)
- self.instance = Instance.objects.create(compute=compute, name="xml-vm", uuid="11111111-2222-3333-4444-888888888888")
+ self.instance = Instance.objects.create(compute=compute, name="xml-vm", uuid=UUID)
def post(self, status=5, xml=EDITED, base=CURRENT, confirm=False, define_error=None):
data = {"inst_xml": xml, "base": base}
@@ -85,3 +87,38 @@ class ChangeXmlTestCase(TestCase):
response, define = self.post(xml=" ", confirm=True)
define.assert_not_called()
self.assertEqual(response.status_code, 400)
+
+ def test_a_busy_vm_answers_the_dialog_so_the_edit_stays(self):
+ # the dialog's script sends X-Requested-With; a page form gets the redirect
+ with patch("instances.views.libvirt_instance_lock", side_effect=claims.Busy("instance:xml-vm")), \
+ patch("instances.models.wvmInstance") as wvm:
+ url = reverse("instances:change_xml", args=[self.instance.id])
+ data = {"inst_xml": EDITED, "base": CURRENT, "confirm": "1"}
+ response = self.client.post(url, data, HTTP_X_REQUESTED_WITH="XMLHttpRequest")
+ self.assertEqual(response.status_code, 409)
+ self.assertIn("try again", response.json()["detail"])
+ self.assertEqual(list(get_messages(response.wsgi_request)), [])
+ self.assertEqual(self.client.post(url, data).status_code, 302)
+ wvm.return_value._defineXML.assert_not_called()
+
+ def test_the_xml_must_keep_the_vms_uuid(self):
+ # libvirt finds the VM to define by the XML's UUID: another VM's would
+ # overwrite that VM, a new or missing one would add a VM
+ other = CURRENT.replace(UUID, "99999999-2222-3333-4444-888888888888")
+ for xml in (other, CURRENT.replace(f" {UUID}\n", "")):
+ for confirm in (False, True):
+ with self.subTest(xml=xml, confirm=confirm):
+ response, define = self.post(xml=xml, confirm=confirm)
+ define.assert_not_called()
+ self.assertEqual(response.status_code, 400)
+ self.assertIn(UUID, response.json()["error"])
+
+ def test_a_malformed_xml_is_refused_with_the_parser_error(self):
+ response, define = self.post(xml="xml-vm", confirm=True)
+ define.assert_not_called()
+ self.assertEqual(response.status_code, 400)
+ self.assertIn("not well-formed", response.json()["error"])
+
+ def test_the_edited_xml_is_defined_with_the_line_breaks_libvirt_gave(self):
+ _, define = self.post(xml=EDITED.replace("\n", "\r\n"), confirm=True)
+ define.assert_called_once_with(EDITED)
diff --git a/instances/views.py b/instances/views.py
index f27008d3..39187e3a 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -431,8 +431,14 @@ def get_instance(user, pk, perm_type="view"):
return instance
+def from_script(request):
+ """A form a page's script sends and answers itself: the create page's
+ flavor forms (it swaps in the reply), the XML editor's dialog (JSON)."""
+ return request.headers.get("X-Requested-With") == "XMLHttpRequest"
+
+
def _busy(request, pk, busy):
- if hasattr(request, "accepted_renderer"):
+ if hasattr(request, "accepted_renderer") or from_script(request):
blocker = busy.blocker
return JsonResponse({"detail": busy_message(busy, request.user),
"operation": summary(blocker, request.user) if blocker else None}, status=409)
@@ -441,7 +447,7 @@ def _busy(request, pk, busy):
def _moved(request, pk):
- if hasattr(request, "accepted_renderer"):
+ if hasattr(request, "accepted_renderer") or from_script(request):
return JsonResponse({"detail": "The VM was moved meanwhile; try again."}, status=409)
messages.error(request, _("The VM was moved meanwhile; try again."))
return get_safe_redirect(request, default=reverse("instances:instance", args=[pk]))
@@ -1460,13 +1466,21 @@ def change_xml(request, pk):
in the persistent definition, to review; with confirm, the edited XML is
defined. A running VM uses it at its next start.
Only over the definition the editor was opened with (base), so a change
- made since is not overwritten; a form posts its line breaks as CRLF. A refusal comes back to the dialog and the
- edited XML stays in the editor.
+ made since is not overwritten, and only with the VM's UUID, by which
+ libvirt finds the VM to define. A form posts its line breaks as CRLF. A
+ refusal comes back to the dialog and the edited XML stays in the editor.
"""
instance = get_instance(request.user, pk)
- new_xml = request.POST.get("inst_xml", "")
+ new_xml = request.POST.get("inst_xml", "").replace("\r\n", "\n")
if not new_xml.strip():
return JsonResponse({"error": _("The XML is empty")}, status=400)
+ try:
+ uuid = util.get_xml_path(new_xml, "/domain/uuid")
+ except util.etree.Error as err:
+ return JsonResponse({"error": _("The XML is not well-formed: %(error)s") % {"error": err}}, status=400)
+ if (uuid or "").strip().lower() != instance.uuid:
+ return JsonResponse({"error": _("The XML must keep the VM's UUID %(uuid)s.") % {"uuid": instance.uuid}},
+ status=400)
current = instance.inst_xml
if request.POST.get("base", "").replace("\r\n", "\n") != current:
return JsonResponse({"error": _("The definition has changed since the editor was opened. "
@@ -1474,18 +1488,18 @@ def change_xml(request, pk):
if "confirm" not in request.POST:
diff = difflib.unified_diff(current.splitlines(), new_xml.splitlines(), "current", "new", lineterm="")
return JsonResponse({"diff": list(diff)})
+ shut_off = instance.proxy.get_status() == 5
try:
with audit_vm(request, instance, "change XML"):
instance.proxy._defineXML(new_xml)
except libvirtError as err:
return JsonResponse({"error": error_text(request, err)}, status=400)
- if instance.proxy.get_status() == 5:
+ if shut_off:
messages.success(request, _("The XML definition was saved."))
else:
messages.success(request, _("The XML definition was saved; the VM uses it from its next start."))
return JsonResponse({"saved": True})
-
@require_POST
@superuser_only
@serialize_instance_mutation
@@ -2101,11 +2115,6 @@ def create_instance(request, compute_id, arch, machine):
return render(request, "create_instance_w2.html", locals())
-def from_create_page(request):
- """A flavor form sent from the create page's script, which swaps in the reply."""
- return request.headers.get("X-Requested-With") == "XMLHttpRequest"
-
-
def flavors_reply(request, flavor_form):
return render(request, "create_flavors_reply.html",
{"flavors": Flavor.objects.order_by("id"), "flavor_form": flavor_form})
@@ -2117,11 +2126,11 @@ def flavor_create(request):
if form.is_valid():
with audit(request, None, f"Flavor {form.cleaned_data['label']}", "add"):
form.save()
- if from_create_page(request):
+ if from_script(request):
return flavors_reply(request, FlavorForm())
messages.success(request, _("Flavor Created"))
return get_safe_redirect(request, default=reverse("instances:index"))
- if from_create_page(request):
+ if from_script(request):
return flavors_reply(request, form)
return render(
@@ -2158,7 +2167,7 @@ def flavor_delete(request, pk):
if request.method == "POST":
with audit(request, None, f"Flavor {flavor.label}", "delete"):
flavor.delete()
- if from_create_page(request):
+ if from_script(request):
return flavors_reply(request, FlavorForm())
messages.success(request, _("Flavor Deleted"))
return get_safe_redirect(request, default=reverse("instances:index"))
diff --git a/static/js/xml_editor.js b/static/js/xml_editor.js
index fa7b6aa7..f2527856 100644
--- a/static/js/xml_editor.js
+++ b/static/js/xml_editor.js
@@ -17,7 +17,8 @@
var save = document.getElementById("xml-review-save");
var reviewed = ""; // Save defines the XML the dialog shows, not later edits
- // null after a redirect (the VM is busy or moved): the page says why
+ // The answer: diff, saved, or error (detail when the VM is busy or moved).
+ // Any other failure, such as a redirect, keeps the edit and says so.
function post(xml, confirm) {
var data = new FormData(form);
data.set("inst_xml", xml);
@@ -25,12 +26,12 @@
if (confirm) {
data.set("confirm", "1");
}
- return fetch(form.action, {method: "POST", body: data, redirect: "manual"}).then(function (response) {
- if (response.type === "opaqueredirect") {
- location.reload();
- return null;
- }
+ return fetch(form.action, {
+ method: "POST", body: data, redirect: "manual", headers: {"X-Requested-With": "XMLHttpRequest"},
+ }).then(function (response) {
return response.json();
+ }).then(function (answer) {
+ return {diff: answer.diff, saved: answer.saved, error: answer.error || answer.detail};
}).catch(function () {
return {error: dialog.dataset.failed};
});
@@ -56,23 +57,26 @@
form.addEventListener("submit", function (event) {
event.preventDefault();
- reviewed = editor.getValue();
- post(reviewed, false).then(function (answer) {
- if (answer) {
- show(answer);
- modal.show();
- }
+ var xml = editor.getValue();
+ // the last answer shown is the one Save sends, whatever order answers come in
+ post(xml, false).then(function (answer) {
+ reviewed = xml;
+ show(answer);
+ modal.show();
});
});
save.addEventListener("click", function () {
save.disabled = true;
post(reviewed, true).then(function (answer) {
- if (answer && answer.saved) {
+ if (answer.saved) {
location.hash = "xmledit";
location.reload();
- } else if (answer) {
- show({error: answer.error, diff: []});
+ } else {
+ // the diff stays, and Save can be tried again
+ error.textContent = answer.error;
+ error.classList.remove("d-none");
+ save.disabled = false;
}
});
});