1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 19:26:48 +00:00

fix(storages): the pool page measures its pool fresh, shares one clone dialog and says what it cannot show

- The page lists the volumes first: that refreshes the pool, and libvirt
  measures a pool's space only when it is refreshed, so the size and
  usage shown were as of the previous refresh.
- A space that cannot be read says so instead of 0 bytes, and the
  pool's status, read from the same pool info, no longer fails the
  page after it; a stopped pool says its space and volumes are
  unavailable until it is started, instead of 0 bytes and nothing.
- The API's pool is measured after the same refresh, and a stopped
  pool lists no volumes there instead of failing.
- A clone the job refuses (a name in use, a volume gone) is a message
  on the page, not a server error.
- An ISO image is recognised by its name (storages.jobs.is_iso):
  libvirt calls its format raw, so the template's "iso" check never
  disabled its Clone button. The clone job refuses one too; the upload
  uses the same rule.
- One clone dialog serves every volume, filled and reset by the button
  that opens it, instead of one dialog per volume in the page; its
  fields have labels and its title names the dialog.
- Volume rows are searched by name only (data-search), with a message
  when none matches, in a live region for screen readers; names show
  in full. The search script loads only
  with the volume table: on a stopped pool's page it failed for want
  of a search box. An empty pool says "This pool has no volumes".
- The unused wvmStorage.get_volumes_details goes: update_volumes is
  the same.
This commit is contained in:
catborise 2026-10-28 15:22:57 +03:00
parent 3aac8f51f2
commit fa5a8c4d6e
7 changed files with 239 additions and 139 deletions

View file

@ -49,15 +49,20 @@ class StorageViewSet(viewsets.ViewSet):
compute.hostname, compute.login, compute.password, compute.type, pk
)
state = conn.is_active()
# first: it refreshes the pool, and libvirt measures a pool's space only then;
# a stopped pool lists none
volumes = conn.update_volumes() if state else None
size, free = conn.get_size()
infoset = {
"state": conn.is_active(),
"size": conn.get_size()[0],
"free": conn.get_size()[1],
"state": state,
"size": size,
"free": free,
"status": conn.get_status(),
"path": conn.get_target_path(),
"type": conn.get_type(),
"autostart": conn.get_autostart(),
"volumes": conn.update_volumes(),
"volumes": volumes,
}
serializer = StorageSerializer(

View file

@ -26,6 +26,11 @@ def _log(op, state, action):
return compute, "", "", message if state == Operation.SUCCEEDED else f"{message} ({state})"
def is_iso(name):
"""An ISO image, by its name: libvirt calls its format raw. It is uploaded, not cloned."""
return name.lower().endswith(".iso")
def _storage(params):
c = Compute.objects.get(pk=params["compute"])
return c, wvmStorage(c.hostname, c.login, c.password, c.type, params["pool"])
@ -51,6 +56,8 @@ class CloneVolume(Kind):
volumes = stg.get_volumes()
if params["image"] not in volumes:
raise util.OperationError(f"The pool has no volume named {params['image']}")
if is_iso(params["image"]):
raise util.OperationError("An ISO image is not cloned")
target = stg.clone_name(params["name"], params["format"] or stg.get_volume_format_type(params["image"]))
if target in volumes:
raise util.OperationError("Name of volume already in use")
@ -102,7 +109,7 @@ class UploadVolume(Kind):
name = params["name"]
if not name or name in (".", "..") or "\x00" in name:
raise ValueError("Invalid file name")
if len(name) <= 4 or not name.lower().endswith(".iso"):
if len(name) <= 4 or not is_iso(name):
raise ValueError("Only .iso files can be uploaded")
if params["size"] <= 0:
raise ValueError("Invalid file size")

View file

@ -28,10 +28,18 @@
<dd class="col-6">{% if not path %}{% trans "None" %}{% else %}{{ path }}{% endif %}</dd>
<dt class="col-6">{% trans "Pool status" %}</dt>
<dd class="col-6">{% if not status %}{% trans "None" %}{% else %}{{ status }}{% endif %}</dd>
{% if not state %}
<dt class="col-6">{% trans "Size" %}</dt>
<dd class="col-6">{% trans "Unavailable while the pool is stopped." %}</dd>
{% elif size is None %}
<dt class="col-6">{% trans "Size" %}</dt>
<dd class="col-6">{% trans "The pool's space could not be read." %}</dd>
{% else %}
<dt class="col-6">{% trans "Size" %} </dt>
<dd class="col-6">{{size|filesizeformat }}</dd>
<dt class="col-6">{% trans "Usage" %} ({{ percent }}%)</dt>
<dd class="col-6">{{ used|filesizeformat }}</dd>
{% endif %}
<dt class="col-6">{% trans "State" %}</dt>
<dd class="col-6">
<form action="" method="post" role="form" aria-label="Storage start/stop form">{% csrf_token %}
@ -63,129 +71,137 @@
</div>
{% if state %}
{% if not state %}
<div class="alert alert-secondary">
{% bs_icon 'info-circle' %} {% trans "This pool is stopped: its volumes cannot be listed or managed until it is started. Their files stay." %}
</div>
{% elif volumes %}
<div class="clearfix">
{% include 'search_block.html' %}
<h5 class="mb-3">{% trans "Volumes" %}</h5>
</div>
{% if volumes %}
<div class="table-responsive">
<table class="table table-hover align-middle" data-sortable>
<thead>
<tr>
<th scope="col" data-sort-method="number">#</th>
<th scope="col">{% trans "Name" %}</th>
<th scope="col" data-sort-method="number">{% trans "Allocated" %}</th>
<th scope="col" data-sort-method="number">{% trans "Size" %}</th>
<th scope="col">{% trans "Format" %}</th>
<th scope="colgroup" data-sort-method="none" colspan="2">{% trans "Action" %}</th>
</tr>
</thead>
<tbody class="searchable">
{% for volume in volumes %}
<tr>
<td>{{ forloop.counter }}</td>
<td>{{ volume.name|truncatechars:55 }}</td>
<td data-sort="{{ volume.allocation }}">{{ volume.allocation|filesizeformat }}</td>
<td data-sort="{{ volume.size }}">{{ volume.size|filesizeformat }}</td>
<td>{{ volume.type }}</td>
<td>
<!-- Modal Clone -->
<div class="modal fade" id="Clone{{ forloop.counter }}" tabindex="-1" role="dialog"
aria-labelledby="addHostLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h5 class="modal-title">{% trans "Clone image" %} <span class="text-danger">{{ volume.name }}</span></h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<form method="post" role="form" aria-label="Clone image file form">{% csrf_token %}{% submission_key %}
<div class="modal-body">
<div class="row">
<label class="col-sm-3 col-form-label">{% trans "Name" %}</label>
<div class="col-sm-6">
<input type="text" class="form-control" name="name" placeholder="{% trans "Name" %}" required pattern="[a-zA-Z0-9\.\-_]+">
<input type="hidden" name="image" value="{{ volume.name }}">
</div>
</div>
<div class="row">
<label class="col-sm-3 col-form-label" for="clone{{ forloop.counter }}_convert">{% trans "Convert" %}</label>
<div class="col-sm-6">
<input class="volume-convert" type="checkbox" name="convert" value="true" id="clone{{ forloop.counter }}_convert">
</div>
</div>
<div class="row format-convert">
<label class="col-sm-3 col-form-label">{% trans "Format" %}</label>
<div class="col-sm-6">
<select name="format" class="form-select image-format">
<option value="raw">{% trans "raw" %}</option>
<option value="qcow">{% trans "qcow" %}</option>
<option value="qcow2">{% trans "qcow2" %}</option>
</select>
</div>
</div>
<div class="row meta-prealloc d-none">
<label class="col-sm-3 col-form-label">{% trans "Metadata" %}</label>
<div class="col-sm-6">
<input type="checkbox" name="meta_prealloc" value="true">
</div>
</div>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{% trans "Close" %}</button>
<button type="submit" class="btn btn-primary" name="cln_volume">{% trans "Clone" %}</button>
</div>
</form>
</div> <!-- /.modal-content -->
</div> <!-- /.modal-dialog -->
</div> <!-- /.modal -->
{% if volume.type != "iso" %}
<button class="btn btn-sm btn-secondary" data-bs-toggle="modal" data-bs-target="#Clone{{ forloop.counter }}" title="{% trans "Clone" %}">
{% bs_icon 'files' %}
</button>
{% else %}
<button class="btn btn-sm btn-secondary disabled">
{% bs_icon 'files' %}
</button>
{% endif %}
</td>
<td>
<form action="" method="post" role="form" aria-label="Delete volume form">{% csrf_token %}
<input type="hidden" name="volname" value="{{ volume.name }}">
<button type="submit" class="btn btn-sm btn-secondary" name="del_volume" title="{% trans "Delete" %}"
data-confirm="{% blocktrans with name=volume.name %}Delete volume {{ name }}? Its file and data are removed; a volume a VM uses is not deleted.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
{% bs_icon 'trash' %}
</button>
</form>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
{% else %}
<div class="col-lg-12">
<div class="alert alert-warning shadow-sm">
{% bs_icon 'exclamation-triangle' %} <strong>{% trans "Warning" %}:</strong> {% trans "Hypervisor doesn't have any Volumes" %}
<div class="table-responsive">
<table class="table table-hover align-middle" data-sortable>
<thead>
<tr>
<th scope="col">{% trans "Name" %}</th>
<th scope="col" data-sort-method="number">{% trans "Allocated" %}</th>
<th scope="col" data-sort-method="number">{% trans "Size" %}</th>
<th scope="col">{% trans "Format" %}</th>
<th scope="colgroup" data-sort-method="none" colspan="2">{% trans "Action" %}</th>
</tr>
</thead>
<tbody class="searchable">
{% for volume in volumes %}
<tr data-search="{{ volume.name }}">
<td class="text-break">{{ volume.name }}</td>
<td data-sort="{{ volume.allocation }}">{{ volume.allocation|filesizeformat }}</td>
<td data-sort="{{ volume.size }}">{{ volume.size|filesizeformat }}</td>
<td>{{ volume.type }}</td>
<td>
{# an ISO image is uploaded, not cloned (storages.jobs.is_iso) #}
<button type="button" class="btn btn-sm btn-secondary" data-bs-toggle="modal" data-bs-target="#CloneVolume" data-volume="{{ volume.name }}"
title="{% trans "Clone" %}" aria-label="{% blocktrans with name=volume.name %}Clone {{ name }}{% endblocktrans %}"{% if volume.iso %} disabled{% endif %}>
{% bs_icon 'files' %}
</button>
</td>
<td>
<form action="" method="post" aria-label="{% trans "Delete volume" %}">{% csrf_token %}
<input type="hidden" name="volname" value="{{ volume.name }}">
<button type="submit" class="btn btn-sm btn-secondary" name="del_volume" title="{% trans "Delete" %}"
aria-label="{% blocktrans with name=volume.name %}Delete {{ name }}{% endblocktrans %}"
data-confirm="{% blocktrans with name=volume.name %}Delete volume {{ name }}? Its file and data are removed; a volume a VM uses is not deleted.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
{% bs_icon 'trash' %}
</button>
</form>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
{# a live region from the start: screen readers hear the message when the search empties the table #}
<div role="status">
<div id="no-match" class="text-center text-body-secondary py-4" hidden>{% trans "No volume matches the search." %}</div>
</div>
{# one dialog for every volume: the button that opens it names the volume #}
<div class="modal fade" id="CloneVolume" tabindex="-1" aria-labelledby="CloneVolumeLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h5 class="modal-title" id="CloneVolumeLabel">{% trans "Clone image" %} <span class="text-danger" id="clone-image-name"></span></h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="{% trans "Close" %}"></button>
</div>
<form method="post" aria-labelledby="CloneVolumeLabel">{% csrf_token %}{% submission_key %}
<div class="modal-body">
<input type="hidden" name="image" id="clone-image">
<div class="row mb-3">
<label class="col-sm-3 col-form-label" for="clone-name">{% trans "Name" %}</label>
<div class="col-sm-6">
<input type="text" class="form-control" name="name" id="clone-name" required pattern="[a-zA-Z0-9\.\-_]+">
</div>
</div>
<div class="row mb-3">
<label class="col-sm-3 col-form-label" for="clone-convert">{% trans "Convert" %}</label>
<div class="col-sm-6">
<input class="form-check-input mt-2" type="checkbox" name="convert" value="true" id="clone-convert">
</div>
</div>
<div class="row mb-3 d-none" id="clone-format-row">
<label class="col-sm-3 col-form-label" for="clone-format">{% trans "Format" %}</label>
<div class="col-sm-6">
<select name="format" class="form-select" id="clone-format">
<option value="raw">{% trans "raw" %}</option>
<option value="qcow">{% trans "qcow" %}</option>
<option value="qcow2">{% trans "qcow2" %}</option>
</select>
</div>
</div>
<div class="row d-none" id="clone-prealloc-row">
<label class="col-sm-3 col-form-label" for="clone-prealloc">{% trans "Metadata" %}</label>
<div class="col-sm-6">
<input class="form-check-input mt-2" type="checkbox" name="meta_prealloc" value="true" id="clone-prealloc">
</div>
</div>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{% trans "Close" %}</button>
<button type="submit" class="btn btn-primary" name="cln_volume">{% trans "Clone" %}</button>
</div>
</form>
</div>
</div>
{% endif %}
</div>
{% else %}
<div class="alert alert-warning shadow-sm">
{% bs_icon 'exclamation-triangle' %} {% trans "This pool has no volumes." %}
</div>
{% endif %}
{% endblock %}
{% block script %}
<script src="{% static 'js/poll.js' %}"></script>
{% job_panel_script job_resource %}
<script src="{% static 'js/table_filter.js' %}"></script>
{% if volumes %}<script src="{% static 'js/table_filter.js' %}"></script>{% endif %}
<script>
// each clone dialog shows its own format and metadata rows
$('.format-convert').hide();
$(document).on('change', '.volume-convert, .image-format', function () {
var form = $(this).closest('form');
var convert = form.find('.volume-convert').prop('checked');
form.find('.format-convert').toggle(convert);
form.find('.meta-prealloc').toggleClass('d-none', !(convert && form.find('.image-format').val() == 'qcow2'));
// the clone dialog: filled for the volume whose button opened it, its
// choices reset; the format rows show while converting
function showCloneRows() {
var convert = $('#clone-convert').prop('checked');
$('#clone-format-row').toggleClass('d-none', !convert);
$('#clone-prealloc-row').toggleClass('d-none', !(convert && $('#clone-format').val() == 'qcow2'));
}
$('#CloneVolume').on('show.bs.modal', function (event) {
var volume = event.relatedTarget.dataset.volume;
$('#clone-image').val(volume);
$('#clone-image-name').text(volume);
$('#clone-name').val('');
$('#clone-convert, #clone-prealloc').prop('checked', false);
$('#clone-format').val('raw');
showCloneRows();
});
$('#clone-convert, #clone-format').on('change', showCloneRows);
$(document).ready(function() {
var pendingUpload = null;

View file

@ -1,14 +1,16 @@
"""The pool page and its volume list still answer while a volume is cloned in
the pool (libvirt refuses to refresh it then) and for a pool without a size;
the page measures the pool after refreshing it and says what it cannot show;
a new volume is at least 1 GiB."""
from unittest.mock import patch
from unittest.mock import call, patch
from computes.models import Compute
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.urls import reverse
from libvirt import libvirtError
from lxml import html
from storages.forms import CreateVolumeForm
@ -38,3 +40,73 @@ class PoolPageTests(TestCase):
form = CreateVolumeForm({"name": "disk1", "format": "qcow2", "size": 0})
self.assertIn("size", form.errors)
self.assertIn("Size (GiB)", str(form["size"].label_tag()))
def test_the_pool_is_measured_after_its_refresh(self):
# update_volumes refreshes the pool: libvirt's space figures are as of the last refresh
self.conn.get_size.return_value = (10, 4)
self.client.get(reverse("storage", args=[self.compute.pk, "default"]))
calls = [c for c in self.conn.mock_calls if c in (call.update_volumes(), call.get_size())]
self.assertEqual(calls, [call.update_volumes(), call.get_size()])
def test_a_space_it_cannot_read_is_not_zero(self):
# the size and the status both read the pool's info: neither failure is a server error
self.conn.get_size.side_effect = libvirtError("cannot read")
self.conn.get_status.side_effect = libvirtError("cannot read")
response = self.client.get(reverse("storage", args=[self.compute.pk, "default"]))
self.assertContains(response, "The pool's space could not be read.")
self.assertNotContains(response, "0 bytes")
def test_a_stopped_pool_says_why_it_lists_no_volumes(self):
# and loads no search script: the page has no search box for it
self.conn.is_active.return_value = False
self.conn.get_size.return_value = (0, 0)
response = self.client.get(reverse("storage", args=[self.compute.pk, "default"]))
self.assertContains(response, "This pool is stopped")
self.assertContains(response, "Unavailable while the pool is stopped.")
self.assertNotContains(response, "0 bytes")
self.assertNotContains(response, "table_filter.js")
self.conn.update_volumes.assert_not_called()
def test_volumes_are_searched_by_name_and_share_one_clone_dialog(self):
self.conn.get_size.return_value = (10, 4)
self.conn.update_volumes.return_value = [
{"name": name, "size": 1, "allocation": 1, "type": "raw"} for name in ("data.img", "install.iso", "Boot.ISO")]
page = html.fromstring(self.client.get(reverse("storage", args=[self.compute.pk, "default"])).content)
self.assertEqual(page.xpath("//tbody/tr/@data-search"), ["data.img", "install.iso", "Boot.ISO"])
self.assertEqual(len(page.xpath("//div[contains(concat(' ', @class, ' '), ' modal ')][.//button[@name='cln_volume']]")), 1)
# an ISO image is not cloned, whatever case its name has
clone = {b.get("data-volume"): b.get("disabled") is None for b in page.xpath("//button[@data-volume]")}
self.assertEqual(clone, {"data.img": True, "install.iso": False, "Boot.ISO": False})
self.assertTrue(page.xpath("//div[@id='no-match'][@hidden]"))
class PoolApiTests(TestCase):
"""The API's pool, like the page, is measured after its refresh; a stopped pool lists no volumes."""
def setUp(self):
self.compute = Compute.objects.create(name="pa", hostname="192.0.2.24", login="root", password="", type=1)
self.client.force_login(get_user_model().objects.create_superuser("pa-admin", "pa@example.com", "pw"))
patcher = patch("storages.api.viewsets.wvmStorage")
self.conn = patcher.start().return_value
self.addCleanup(patcher.stop)
self.conn.get_size.return_value = (10, 4)
self.conn.get_status.return_value = "Running normally"
self.conn.get_target_path.return_value = "/var/lib/libvirt/images"
self.conn.get_type.return_value = "dir"
self.conn.get_autostart.return_value = 1
self.conn.update_volumes.return_value = []
self.url = f"/api/v1/computes/{self.compute.id}/storages/default/"
def test_an_active_pool_is_measured_after_its_refresh(self):
self.conn.is_active.return_value = True
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200, response.content)
calls = [c for c in self.conn.mock_calls if c in (call.update_volumes(), call.get_size())]
self.assertEqual(calls, [call.update_volumes(), call.get_size()])
def test_a_stopped_pool_lists_no_volumes(self):
self.conn.is_active.return_value = False
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200, response.content)
self.assertIsNone(response.json()["volumes"])
self.conn.update_volumes.assert_not_called()

View file

@ -49,6 +49,21 @@ class VolumeCloneJobTestCase(TransactionTestCase):
self.assertEqual(op.intent["created"], "copy.qcow2")
self.assertIn(f"key=pool%3A{self.compute.pk}%3Apool-a", self.client.get(url).content.decode())
@patch("storages.views.wvmStorage")
def test_a_refused_copy_is_a_message(self, page):
# a name in use, an ISO image: the job's plan refuses, the page says why
page.return_value.is_active.return_value = False
page.return_value.get_size.return_value = (1, 1)
url = reverse("storage", args=[self.compute.pk, "pool-a"])
self.stg.get_volumes.return_value = ["base.qcow2", "copy.qcow2", "install.ISO"]
for key, image, why in (("key-vol00002", "base.qcow2", "Name of volume already in use"),
("key-vol00003", "install.ISO", "An ISO image is not cloned")):
with self.subTest(image=image):
response = self.client.post(url, {"cln_volume": "1", "image": image, "name": "copy",
"submission_key": key}, follow=True)
self.assertContains(response, why)
self.assertFalse(Operation.objects.exists())
def test_recovery(self):
self.stg.get_volumes.return_value = ["base.qcow2", "copy.qcow2"]
op = Operation(params={"compute": self.compute.pk, "pool": "pool-a"}, intent={"target": "copy.qcow2"})

View file

@ -19,6 +19,7 @@ from vrtManager.util import OperationError
from vrtManager.storage import wvmStorage, wvmStorages
from storages.forms import AddStgPool, CloneImage, CreateVolumeForm
from storages.jobs import is_iso
from storages.upload import write_chunk
@ -163,26 +164,25 @@ def storage(request, compute_id, pool):
storages = conn.get_storages()
state = conn.is_active()
# first: it refreshes the pool, and libvirt measures a pool's space only then
volumes = conn.update_volumes() if state else None
for volume in volumes or []:
volume["iso"] = is_iso(volume["name"])
try:
size, free = conn.get_size()
status = conn.get_status()
used = size - free
if state and size: # an iSCSI pool without LUNs has no size
percent = (used * 100) // size
else:
percent = 0
except libvirtError:
size, free, used, percent = 0, 0, 0, 0
except libvirtError: # both read the pool's info: the page says it could not be read
size = free = used = percent = status = None
status = conn.get_status()
path = conn.get_target_path()
type = conn.get_type()
autostart = conn.get_autostart()
if state:
volumes = conn.update_volumes()
else:
volumes = None
if request.method == "POST":
subject = f"Storage pool {pool}"
if "start" in request.POST:
@ -240,7 +240,7 @@ def storage(request, compute_id, pool):
entry.skip = True # the job writes its row when it ends
except claims.Busy as busy:
messages.error(request, busy_message(busy, request.user))
except (jobs.SubmissionConflict, libvirtError) as err:
except (jobs.SubmissionConflict, OperationError, libvirtError) as err:
messages.error(request, str(err))
else:
messages.info(request, _("The volume is being cloned; follow it in the job panel."))

View file

@ -281,21 +281,6 @@ class wvmStorage(wvmConnect):
def refresh(self):
return self.pool.refresh(0)
def get_volumes_details(self):
with contextlib.suppress(Exception):
self.refresh()
vols = self.get_volumes()
return [
{
"name": volname,
"size": self.get_volume_size(volname),
"allocation": self.get_volume_allocation(volname),
"type": self.get_volume_format_type(volname),
}
for volname in vols
]
def get_volume_details(self, volname):
with contextlib.suppress(Exception):
self.refresh()