-
-
-
-
-
-
{% trans "To Enable/Disable Qemu Guest Agent. Status" %}:
- {% if instance.status == 1 %}
- {% if instance.guest_agent_ready %}
-
- {% else %}
-
- {% endif %}
- {% else %}
-
- {% endif %}
-
diff --git a/instances/tests/test_live_dataloss.py b/instances/tests/test_live_dataloss.py
index 6c8b7551..38f23329 100644
--- a/instances/tests/test_live_dataloss.py
+++ b/instances/tests/test_live_dataloss.py
@@ -544,12 +544,10 @@ class LiveDataLossTestCase(TestCase):
return (f"
"
"")
- agent = "
"
cases = (
("disk", lambda n: disk(livetest.create_volume(self.conn, P + n + "-pending")),
lambda vm, n: vm.attach_disk("sda", livetest.create_volume(self.conn, P + n + "-new"),
target_bus="scsi", format_type="qcow2")),
- ("agent", lambda n: agent, lambda vm, n: vm.add_guest_agent()),
)
both = (0, libvirt.VIR_DOMAIN_XML_INACTIVE)
for paused in (False, True):
@@ -566,6 +564,19 @@ class LiveDataLossTestCase(TestCase):
attach(inst.proxy, name)
self.assertEqual([dom.XMLDesc(flags) for flags in both], before)
+ def test_an_agent_channel_pending_the_next_start_is_added_to_the_running_vm_only(self):
+ from lxml import etree
+
+ agent = "
"
+ dom, inst = self.vm("agent-pending", [], extra_devices="
")
+ dom.create()
+ dom.attachDeviceFlags(agent, libvirt.VIR_DOMAIN_AFFECT_CONFIG) # pending: persistent only
+ channels = "devices/channel/target[@name='org.qemu.guest_agent.0']"
+ for _attempt in range(2): # saving Enabled again changes nothing
+ inst.proxy.add_guest_agent()
+ self.assertEqual([len(etree.fromstring(dom.XMLDesc(flags)).xpath(channels))
+ for flags in (0, libvirt.VIR_DOMAIN_XML_INACTIVE)], [1, 1])
+
def test_a_nic_without_a_mac_gets_the_same_one_in_both_definitions(self):
from lxml import etree
diff --git a/instances/tests/test_pending_edits.py b/instances/tests/test_pending_edits.py
index 0d2cc8de..d14f1c2a 100644
--- a/instances/tests/test_pending_edits.py
+++ b/instances/tests/test_pending_edits.py
@@ -132,16 +132,29 @@ class PendingEditsTestCase(TestCase):
self.assertEqual(doc.xpath("//select[@name='vcpu']/option[@selected]/@value"), ["4"])
self.assertEqual(doc.xpath("//select[@name='cur_vcpu']/option[@selected]/@value"), ["3"])
+ def test_the_agent_and_vcpu_hot_plug_forms_show_the_pending_definition(self):
+ # running without an agent channel and hot plug, both pending for the next start
+ hotplug = {"0": {"enabled": "yes", "hotpluggable": "no", "order": "1"},
+ "1": {"enabled": "yes", "hotpluggable": "yes", "order": "2"}}
+ doc = self.page(get_guest_agent=(None, object()), get_vcpus=({}, hotplug))
+ self.assertEqual(doc.xpath("//select[@name='guest_agent']/option[@selected]/@value"), ["True"])
+ self.assertEqual(doc.xpath("//select[@name='vcpu_hotplug']/option[@selected]/@value"), ["True"])
+ self.assertIn("No channel", doc.xpath("string(//form[.//select[@name='guest_agent']])"))
+
def test_buttons_shown_as_disabled_cannot_submit(self):
- # running: the CPU, options and video forms wait for a shutdown
+ # running: the CPU resize, vCPU hot plug, template and disk removal forms wait for a shutdown;
+ # the title, description and video model are of the next start, saved at once
doc = self.page(live=[disk("vda", "default", "")], config=[disk("vda", "default", "")])
- self.assertGreater(len(doc.xpath("//button[@disabled]")), 5)
+ self.assertGreater(len(doc.xpath("//button[@disabled]")), 4)
shown = doc.xpath("//button[contains(concat(' ', @class, ' '), ' disabled ')]")
self.assertEqual([b for b in shown if b.get("disabled") is None], [])
- for action in ("resizevm_cpu", "change_options", "set_video_model", "detach_vol", "delete_vol"):
+ for action, waits in (("resizevm_cpu", True), ("set_vcpu_hotplug", True), ("set_template", True),
+ ("detach_vol", True), ("delete_vol", True),
+ ("change_options", False), ("set_video_model", False)):
with self.subTest(action):
(form,) = doc.xpath("//form[@action=$a]", a=reverse(f"instances:{action}", args=[self.instance.id]))
- self.assertTrue(all(b.get("disabled") is not None for b in form.xpath(".//button")))
+ controls = form.xpath(".//button | .//input[@type='checkbox']")
+ self.assertEqual([c.get("disabled") is not None for c in controls], [waits] * len(controls))
def test_the_xml_editor_shows_the_definition_for_the_next_start(self):
# static/js/xml_editor.js opens the review dialog on Change, also while the VM runs
diff --git a/instances/tests/test_template_flag.py b/instances/tests/test_template_flag.py
index 6db74e31..5880e7ba 100644
--- a/instances/tests/test_template_flag.py
+++ b/instances/tests/test_template_flag.py
@@ -1,8 +1,6 @@
"""
-Only superusers and staff may mark or unmark a VM as a template.
-The UI only disables the checkbox for other users, and a disabled checkbox is
-not submitted, so the server must not derive is_template from its absence for
-them.
+Only superusers and staff may mark or unmark a VM as a template, on its own
+form: the title and description form leaves the flag alone.
"""
from unittest.mock import patch
@@ -13,6 +11,7 @@ from django.urls import reverse
from accounts.models import UserInstance
from computes.models import Compute
from instances.models import Instance
+from logs.models import Logs
class TemplateFlagTestCase(TestCase):
@@ -36,16 +35,15 @@ class TemplateFlagTestCase(TestCase):
for user in (self.owner, self.staff_owner):
UserInstance.objects.create(instance=self.instance, user=user, is_change=True)
- def _change_options(self, user, data, status=302):
+ def _post(self, user, action, data, status=302):
self.client.force_login(user)
with patch("instances.models.wvmInstance") as wvm:
+ wvm.return_value.get_status.return_value = 5
# unmarking a template checks the owner's quota with the VM's size
wvm.return_value.get_vcpu.return_value = 1
wvm.return_value.get_memory.return_value = 1024
wvm.return_value.get_disk_devices.return_value = []
- res = self.client.post(
- reverse("instances:change_options", args=[self.instance.id]), data
- )
+ res = self.client.post(reverse(f"instances:{action}", args=[self.instance.id]), data)
self.assertEqual(res.status_code, status)
self.instance.refresh_from_db()
@@ -54,23 +52,30 @@ class TemplateFlagTestCase(TestCase):
self.instance.save(update_fields=["is_template"])
def test_owner_cannot_mark_vm_as_template(self):
- self._change_options(self.owner, {"title": "t", "is_template": "True"})
+ self._post(self.owner, "set_template", {"is_template": "True"}, status=403)
self.assertFalse(self.instance.is_template)
def test_owner_cannot_change_a_template(self):
# a template is changed only by superusers and staff owners
self._make_template()
- self._change_options(self.owner, {"title": "new title", "description": ""}, status=403)
+ self._post(self.owner, "change_options", {"title": "new title", "description": ""}, status=403)
self.assertTrue(self.instance.is_template)
def test_staff_owner_can_mark_and_unmark_template(self):
- self._change_options(self.staff_owner, {"title": "t", "is_template": "True"})
+ self._post(self.staff_owner, "set_template", {"is_template": "True"})
self.assertTrue(self.instance.is_template)
- # An unchecked checkbox is not submitted: for staff that means "not a template".
- self._change_options(self.staff_owner, {"title": "t"})
+ # an unchecked switch is not submitted: "not a template"
+ self._post(self.staff_owner, "set_template", {})
self.assertFalse(self.instance.is_template)
+ self.assertEqual(list(Logs.objects.filter(message__startswith="VM").order_by("id").values_list("message", "success")),
+ [("VM tpl-vm: mark as template", True), ("VM tpl-vm: unmark as template", True)])
def test_superuser_can_unmark_template(self):
self._make_template()
- self._change_options(self.superuser, {"title": "t"})
+ self._post(self.superuser, "set_template", {})
self.assertFalse(self.instance.is_template)
+
+ def test_the_title_form_leaves_the_template_alone(self):
+ self._make_template()
+ self._post(self.superuser, "change_options", {"title": "t", "description": ""})
+ self.assertTrue(self.instance.is_template)
diff --git a/instances/tests/test_template_quota.py b/instances/tests/test_template_quota.py
index 0472e3be..39de4de3 100644
--- a/instances/tests/test_template_quota.py
+++ b/instances/tests/test_template_quota.py
@@ -1,6 +1,6 @@
"""Templates do not count towards the quota and cannot be started. Turning a
template back into a VM adds it to its owner's usage, so for users with a
-quota it is checked like any other increase."""
+quota it is checked like any other increase. Only a shut-off VM changes."""
from unittest.mock import patch
@@ -22,18 +22,17 @@ class TemplateFlagQuotaTests(TestCase):
self.vm = Instance.objects.create(compute=compute, name="tq-vm", uuid="12345678-0000-0000-0000-00000000aaaa")
UserInstance.objects.create(user=self.staff, instance=self.vm, is_change=True)
- def post(self, user, is_template, quota=""):
+ def post(self, user, is_template, quota="", status=5):
self.client.force_login(user)
- data = {"title": "", "description": ""}
- if is_template:
- data["is_template"] = "on"
+ data = {"is_template": "on"} if is_template else {}
with patch("instances.models.wvmInstance") as wvm, patch(
"instances.views.utils.check_user_quota", return_value=quota
) as check:
+ wvm.return_value.get_status.return_value = status
wvm.return_value.get_vcpu.return_value = 2
wvm.return_value.get_memory.return_value = 2048
wvm.return_value.get_disk_devices.return_value = [{"size": 10 << 30}]
- response = self.client.post(reverse("instances:change_options", args=[self.vm.id]), data)
+ response = self.client.post(reverse("instances:set_template", args=[self.vm.id]), data)
self.vm.refresh_from_db()
return check, [str(m) for m in get_messages(response.wsgi_request)]
@@ -60,6 +59,14 @@ class TemplateFlagQuotaTests(TestCase):
self.assertTrue(self.vm.is_template)
self.assertTrue(any("cannot be reached" in m for m in messages), messages)
+ def test_a_vm_that_is_not_shut_off_stays_as_it_is(self):
+ # a running VM made a template would leave the quota while it runs
+ for status in (1, 3):
+ with self.subTest(status=status):
+ check, messages = self.post(self.staff, is_template=True, status=status)
+ self.assertFalse(self.vm.is_template)
+ self.assertIn("Shut the VM down to change whether it is a template.", messages)
+
def test_superusers_have_no_quota(self):
Instance.objects.filter(pk=self.vm.pk).update(is_template=True)
admin = get_user_model().objects.create_superuser("tq-admin", "tq@example.com", "pw")
diff --git a/instances/urls.py b/instances/urls.py
index 1e2ad9c6..bd4064c3 100755
--- a/instances/urls.py
+++ b/instances/urls.py
@@ -73,6 +73,7 @@ urlpatterns = [
path("
/clone/", views.clone, name="clone"),
path("/update_console/", views.update_console, name="update_console"),
path("/change_options/", views.change_options, name="change_options"),
+ path("/set_template/", views.set_template, name="set_template"),
path(
"/getvvfile/", views.getvvfile, name="getvvfile"
), # no links to this one???
diff --git a/instances/views.py b/instances/views.py
index 94d38c46..e23bc85d 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -2165,45 +2165,47 @@ def update_console(request, pk):
@require_POST
-@serialize_user_quota
@serialize_instance_mutation
def change_options(request, pk):
+ """The title and description of the persistent definition: they apply
+ at the next start, so a running VM's are changed too."""
instance = get_instance(request.user, pk, perm_type="change")
- try:
- userinstance = instance.userinstance_set.get(user=request.user)
- except UserInstance.DoesNotExist:
- userinstance = UserInstance(is_change=False)
-
- if request.user.is_superuser or userinstance.is_change:
- # Only superusers and staff may (un)mark templates. The
- # checkbox is disabled for everyone else, and a disabled checkbox is
- # not submitted, so for them the flag must be left alone.
- if request.user.is_superuser or request.user.is_staff:
- is_template = bool(request.POST.get("is_template", False))
- # A template is not counted and cannot be started: making it a
- # VM again adds it to the owner's usage, so the quota applies.
- if instance.is_template and not is_template and not request.user.is_superuser:
- disk_sum = sum(int(disk.get("size") or 0) >> 30 for disk in instance.disks)
- quota_msg = utils.check_user_quota(request.user, 1, instance.vcpu, instance.memory, disk_sum)
- if quota_msg:
- messages.error(request, quota_refused(quota_msg, _(
- "User %(quota_msg)s quota reached, '%(instance_name)s' stays a template!"
- ) % {"quota_msg": quota_msg, "instance_name": instance.name}))
- is_template = True
- instance.is_template = is_template
- instance.save(update_fields=["is_template"])
-
- options = {}
- for post in request.POST:
- if post in ["title", "description"]:
- options[post] = request.POST.get(post, "")
- with audit_vm(request, instance, "edit options"):
- instance.proxy.set_options(options)
+ options = {key: request.POST.get(key, "") for key in ("title", "description") if key in request.POST}
+ with audit_vm(request, instance, "edit options"):
+ instance.proxy.set_options(options)
return get_safe_redirect(
request, default=reverse("instances:instance", args=[instance.id]) + "#options"
)
+@require_POST
+@serialize_user_quota
+@serialize_instance_mutation
+def set_template(request, pk):
+ """Marks a shut-off VM as a template, or a template as a VM again; only
+ superusers and staff. A template is not counted and cannot be started:
+ making it a VM again adds it to the owner's usage, so the quota applies."""
+ instance = get_instance(request.user, pk, perm_type="change")
+ if not (request.user.is_superuser or request.user.is_staff):
+ raise PermissionDenied
+ is_template = bool(request.POST.get("is_template"))
+ if instance.proxy.get_status() != 5:
+ messages.error(request, _("Shut the VM down to change whether it is a template."))
+ return _back(request, pk, "options")
+ if instance.is_template and not is_template and not request.user.is_superuser:
+ disk_sum = sum(int(disk.get("size") or 0) >> 30 for disk in instance.disks)
+ quota_msg = utils.check_user_quota(request.user, 1, instance.vcpu, instance.memory, disk_sum)
+ if quota_msg:
+ messages.error(request, quota_refused(quota_msg, _(
+ "User %(quota_msg)s quota reached, '%(instance_name)s' stays a template!"
+ ) % {"quota_msg": quota_msg, "instance_name": instance.name}))
+ return _back(request, pk, "options")
+ with audit_vm(request, instance, "mark as template" if is_template else "unmark as template"):
+ instance.is_template = is_template
+ instance.save(update_fields=["is_template"])
+ return _back(request, pk, "options")
+
+
def getvvfile(request, pk):
instance = get_instance(request.user, pk)
# The .vv file contains the VNC password: same rule as the console.
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index e1ca942a..67c555be 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -83,6 +83,7 @@ PERSISTENT_XML = VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE
USB_HOSTDEVS = "devices/hostdev[@mode='subsystem'][@type='usb']"
PCI_HOSTDEVS = "devices/hostdev[@mode='subsystem'][@type='pci']"
# what of the host a VM holds: host devices, and NICs that are host devices
+GUEST_AGENT_CHANNEL = "/domain/devices/channel[@type='unix']/target[@name='org.qemu.guest_agent.0']"
HOST_DEVICES = "devices/hostdev | devices/interface[@type='hostdev'] | devices/interface/actual[@type='hostdev']"
# External snapshot names become part of overlay and memory file names.
@@ -695,9 +696,9 @@ class wvmInstance(wvmConnect):
readonly = util.get_xml_path(xml, "/domain/os/loader/@readonly")
return {"loader": loader, "type": loader_type, "readonly": readonly}
- def get_vcpus(self):
+ def get_vcpus(self, config=False):
vcpus = OrderedDict()
- tree = etree.fromstring(self._XMLDesc(0))
+ tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML if config else 0))
for vcpu in tree.xpath("/domain/vcpus/vcpu"):
vcpu_id = vcpu.get("id")
enabled = vcpu.get("enabled")
@@ -1597,7 +1598,8 @@ class wvmInstance(wvmConnect):
return model.get("type")
def set_video_model(self, model):
- """Changes only primary video card"""
+ """Changes only the primary video card, which stays the primary one;
+ the old model's memory and head settings go with it."""
xml = self._XMLDesc(PERSISTENT_XML)
tree = etree.fromstring(xml)
video_models = tree.xpath("/domain/devices/video/model")
@@ -1605,7 +1607,9 @@ class wvmInstance(wvmConnect):
if video_model.get("primary") == "yes" or len(video_models) == 1:
parent = video_model.getparent()
parent.remove(video_model)
- etree.SubElement(parent, "model", type=model)
+ new_model = etree.SubElement(parent, "model", type=model)
+ if video_model.get("primary"):
+ new_model.set("primary", video_model.get("primary"))
self._defineXML(etree.tostring(tree).decode())
return
@@ -2923,23 +2927,23 @@ class wvmInstance(wvmConnect):
self._defineXML(etree.tostring(tree).decode())
def add_guest_agent(self):
- channel_xml = """
-
-
-
- """
- self._attach_device(channel_xml)
+ """Adds the agent channel to the definitions that do not have it."""
+ flags = 0
+ for xml_flags, affect in self._definitions():
+ if not etree.fromstring(self._XMLDesc(xml_flags)).xpath(GUEST_AGENT_CHANNEL):
+ flags |= affect
+ if flags:
+ channel_xml = ""
+ self.instance.attachDeviceFlags(channel_xml, flags)
def remove_guest_agent(self):
for xml_flags, affect in self._definitions():
tree = etree.fromstring(self._XMLDesc(xml_flags))
- for target in tree.xpath(
- "/domain/devices/channel[@type='unix']/target[@name='org.qemu.guest_agent.0']"
- ):
+ for target in tree.xpath(GUEST_AGENT_CHANNEL):
channel_xml = etree.tostring(target.getparent()).decode()
self.instance.detachDeviceFlags(channel_xml, affect)
- def get_guest_agent(self):
+ def get_guest_agent(self, config=False):
def _get_agent(doc):
"""
Return agent channel object if it is defined.
@@ -2952,7 +2956,7 @@ class wvmInstance(wvmConnect):
return channel
return None
- return util.get_xml_path(self._XMLDesc(0), func=_get_agent)
+ return util.get_xml_path(self._XMLDesc(PERSISTENT_XML if config else 0), func=_get_agent)
def set_user_password(self, user, crypted):
"""Set a user's password in the running guest through its agent;
diff --git a/vrtManager/tests/test_device_edits.py b/vrtManager/tests/test_device_edits.py
index 16e59761..1e61c504 100644
--- a/vrtManager/tests/test_device_edits.py
+++ b/vrtManager/tests/test_device_edits.py
@@ -116,6 +116,24 @@ class DeviceEditsTestCase(unittest.TestCase):
inbound = defined(p).find("devices/interface[@type='network']/bandwidth/inbound")
self.assertEqual(inbound.attrib, {"average": "10", "peak": "20", "burst": "30", "floor": "4"})
+ def test_a_new_video_model_stays_the_primary_one(self):
+ # the old model's memory sizes are its own: they go
+ p = proxy()
+ p._XMLDesc = lambda flags: PERSISTENT.replace("", "")
+ p.set_video_model("virtio")
+ self.assertEqual(defined(p).find("devices/video/model").attrib, {"type": "virtio", "primary": "yes"})
+
+ def test_the_agent_channel_and_vcpu_hot_plug_are_read_from_either_definition(self):
+ p = proxy()
+ pending = (""
+ "