Each open VM page asks for its statistics every 10 seconds. For a
running VM the stats view measured the CPU, then each disk, then each
NIC, sleeping one second for each. With two disks and a NIC a request
held a sync gunicorn worker for over 4 s, so a few dozen open VM pages
could keep every worker busy.
wvmInstance.usage() replaces cpu_usage, disk_usage and net_usage, the
stats view's only callers: it reads all counters, sleeps one second
once and reads them again. Values and units are unchanged (CPU percent
of the host, disk bytes, network bits per second); compared against the
previous methods on the same counters, the results are identical.
Measured on the test host for a running VM with two disks and a NIC:
the stats request took 4.1-4.3 s before, 1.1-1.3 s now. The new test
fails on the previous code.
A VM owner who may manage the console (is_change and is_vnc) could set
its listen address to all interfaces and remove its password: a VNC
console reachable from the network without a password. Checked on a
test host: the owner's form changed the VM's listen address to 0.0.0.0
and then removed the password.
- The listen address field is disabled for users who are not
superusers; a disabled field ignores posted values. They can still
set or change the password and the keymap.
- A console whose persistent definition does not bind it to loopback
(or a Unix socket) only loses its password through an administrator.
A binding the XML does not pin down counts as not local: listen type
network, or no address (the host's default in qemu.conf).
On the test host, the owner's form now leaves the listen address alone
and is refused when it removes the password of a console listening on
0.0.0.0; with the console on localhost the password can be removed.
The new tests fail on the previous code.
The connection manager registered libvirt's event implementation and
started the event loop thread when vrtManager.connection was imported.
novncd imports it and then forks a child per console (websockify's
multiprocessing), so every child was forked from a process with a
running libvirt thread, which libvirt does not support, and the child
had no loop thread of its own: keepalive never ran there. Measured in
the test setup: in such a child, a call on a host that went silent
still waited after 70 s.
- The loop starts with the first get_connection of a process. The
state is per process and shared by all managers; libvirt's event
implementation is registered once and a child inherits it.
- After a fork, the child renews the lock and starts its own loop on
first use. It stops using the inherited connections, which are the
parent's sockets, but keeps them: releasing them would run libvirt
destructors in the child. No process connects before forking today
(novncd's parent does not connect, gunicorn does not preload); this
keeps such a case safe.
With the change, novncd's parent has no libvirt thread, the console
child starts its own, and the same call fails after 29 s with a
keepalive timeout. A test forks a real child; it fails without the
fork handling.
libvirt has no timeout for opening a connection. Measured in the test
setup: an unreachable host blocked qemu+tcp for 135 s and qemu+ssh for
133 s; a daemon that accepts and never answers blocks forever. Gunicorn
runs sync workers and the instance list connects to every compute, so
one unreachable compute held a worker for minutes per page load.
- A connection is opened in a helper thread, at most one per
connection at a time. The request waits LIBVIRT_CONNECT_TIMEOUT
(default 5 s). An open nobody waits for any more keeps running; until
it ends, requests get its error at once, and if it succeeds later the
connection is used. A failed open is not repeated for
LIBVIRT_RETRY_AFTER (default 30 s). A stuck host holds one thread and
does not keep other hosts from connecting.
- qemu+ssh runs ssh through vrtManager/libvirt-ssh (the URI's command
parameter), which adds ConnectTimeout=10 and BatchMode=yes, so an
abandoned ssh process ends and no prompt can hang. The user's ssh
configuration (aliases, ProxyJump, keys) still applies.
- The close callback acts only for the connection it was registered
for: a late callback of a replaced connection dropped its successor.
The reason is reported as text. A connection that closed before it
was published is an error.
- Compute.status is a bool (it returned the exception object, so the
"host is down" check in refresh_instance_database never fired); the
message is Compute.connection_error.
Unit tests cover the timeout, retry delay, late success, single open,
a stuck host next to a healthy one and the close callback. Live tests
(vrtManager/test_live_connection.py), for TCP and SSH computes of the
test setup: an unreachable address (no ssh left after 10 s), an endpoint
that never answers, a slow open completing later, a host that goes
silent on an established connection (keepalive ends calls within 45 s,
the next request reconnects), an ssh_config alias and a ProxyJump host.
Creating each option on a RHEL qemu-kvm host showed:
- CD-ROM: the create page offered every disk bus, but the empty CD-ROM
of a new VM works only on ide, sata and scsi. fdc and virtio failed
("Invalid bus type", "does not support ejectable media"), usb failed
with a device named 'Nonea' because vol_dev_type() had no prefix for
usb, and after that libvirt refuses an empty usb disk. Only ide, sata
and scsi are offered now, and another value is refused before any
storage is allocated. vol_dev_type() names usb disks sdX, which also
fixes data volumes on the usb bus.
- USB controller: any input device added a bare <controller
type='usb'/>, which libvirt turns into piix3-uhci (USB 1.1) on q35
instead of its default qemu-xhci. The bare controller is gone; libvirt
adds the machine's default for USB inputs.
- MACs: with more networks than MAC addresses, create_instance indexed
past the list and the request failed with a 500. NICs without a given
MAC now get one from libvirt. MAC addresses are validated (format, not
more than networks) by the page and the API before anything is
created; they went into the XML unchecked.
Unit tests cover the refused values, the MAC rule and the usb prefix.
Live tests create a VM with every offered CD-ROM bus and start it,
check the USB controller with a USB input device and create a VM with
two networks and one MAC.
The form checked both with re.match("^[0-9a-f./:]+|^$"), which tests
only how a value starts: "2001:db8::10/64'/><evil/>" passed and went
into the interface XML, which is built with f-strings, while upper-case
hex was refused. An address without a prefix passed too and then failed
with a ValueError when the XML was built.
The address is parsed with ipaddress.IPv6Interface and stored as
address/prefix (/128 when none is given), the gateway with
IPv6Address. A static configuration needs an address, and the XML gets
a route only when a gateway is given instead of gateway=''. The HTML
patterns accept upper-case hex.
Eight of the ten form and XML checks fail on the previous code.
attach_disk (new or existing volume, CD-ROM), add_guest_agent and
remove_guest_agent acted only when the status was running (1) or shut
off (5). On a paused VM they changed neither definition and the request
reported success. They now use the same rule as the NIC edits: the
running definition while the VM is active, paused included, and the
persistent one always. remove_guest_agent reads each definition from its
own XML, so a channel present only in the persistent one is removed too.
Unit tests check paused, running and stopped VMs (paused fails on the
previous code). The live effects test adds attach_disk and
add_guest_agent, and now also checks that edits meant for the running VM
reach it, which showed that an ejected CD-ROM keeps an empty <source>
in the live XML; NIC unplug is left to the guest tests, as these VMs
have no OS to acknowledge it.
Comments, docstrings and live test names cited item IDs of a roadmap
that is not part of the repository. They now say what is meant, or
nothing where the sentence stands without them. Test assertions and
behavior are unchanged.
- host_is_up returned the exception object for a down host, which is
truthy: check_user_quota then connected to it and failed, so a user
with a VM on a down host could not open its page, clone or resize,
and migrate_instance's "target is down" check never fired. It returns
True or False now, closes its socket, and accepts IPv6 and host:port.
- The connection manager no longer holds its global lock while a new
connection is opened, so one slow host does not delay new connections
to the others.
Bounding the connection time itself (including SSH aliases and jump
hosts) is left for a separate change.
- change_network detached the NIC and attached one built from scratch,
dropping its QoS, link state and other settings. It now edits the NIC
of the persistent definition in place and defines once; a running VM
gets the change at its next start, as the UI already says.
- add_network, delete_network and set_link_state did nothing on a
paused VM, and set_link_state rewrote the persistent NIC from the live
XML. They now update the running definition when the VM is active and
the persistent one always, each from its own XML.
- Boot and console edits used xml.etree, which renamed the namespace
prefixes of other tools' metadata (<app:info> became <ns0:info>); they
use lxml now.
- The NIC table and the clone form are filled from the persistent
definition (Instance.config_networks, config_disks), so a pending NIC
change is shown and can be edited again.
A live test runs 18 domain edits on stopped, running and paused VMs
whose definition has elements WebVirtCloud does not manage, and checks
for each the effect, that nothing else in the persistent definition
changed, and that persistent-only edits leave the running VM alone.
Clone:
- only shut-off VMs are cloned, as the UI already required: a running
guest keeps writing while its disks are copied; the definition comes
from the persistent XML;
- MAC addresses and disk destinations are checked before anything is
allocated;
- every volume the clone creates (NVRAM and disks) is tracked; if a
later copy or the definition fails, only those are deleted, and any
that cannot be deleted are named in the error. Before, a failed clone
left its earlier copies behind.
libvirtError(msg) reports the thread's last libvirt error instead of
msg when there is one, so WebVirtCloud's own errors could show an
earlier, unrelated failure. util.OperationError, a libvirtError
subclass that keeps its message, replaces those raises in vrtManager,
instances and nwfilters; 'except libvirtError' still catches it.
SPICE was removed earlier; its leftovers and the console type choice go
too:
- the console settings form, update_console and the backend no longer
offer or change the type; an invalid console form now shows its
errors instead of being ignored;
- the create form, the API serializer and create_instance no longer take
a graphics type, new VMs get VNC; get_graphics_types and the graphics
entries of the domain capabilities are gone;
- migration appsettings.0013 deletes QEMU_CONSOLE_DEFAULT_TYPE;
- a VM defined elsewhere with another graphics type gets "Console type
... is not supported".
The locale catalogs still reference the deleted SPICE templates in
source comments; regenerating them is left to the translation work.
The old code edited the live XML, called attachDevice and redefined the
domain from it: a stopped VM lost its VNC password, a paused VM crashed
with UnboundLocalError, an unknown CD-ROM was silently ignored (on a
running VM the last disk of the loop could be updated instead), and a
missing ISO raised AttributeError.
Media changes now use updateDeviceFlags on the requested CD-ROM, built
from the live and the persistent definition separately. A CD-ROM
missing from one definition, an unknown ISO, an ISO name present in
more than one pool, or a change that reached only the running VM is
reported to the user. The mount and unmount log messages were swapped.
Edits that redefine a domain read the wrong XML:
- boot menu/order, vCPU hotplug and QoS read XMLDesc(0), which has no
secrets, so every such edit, even on a stopped VM, removed the VNC
password from the definition;
- console, video, CPU, memory and title/description edits read the live
XML, so on a running VM pending changes (e.g. a new memory size) were
reverted and runtime state was written into the definition.
All of them now read VIR_DOMAIN_XML_INACTIVE | SECURE (PERSISTENT_XML).
The console setters take the first graphics device of that definition
instead of looking it up by the live type.
Disk resize no longer redefines the domain. A running VM's image files
grow through blockResize, since qemu-img cannot take the lock QEMU
holds; block volumes (e.g. LVM) are grown by the storage pool first and
QEMU is then told the new size.
Live tests reproduce the losses on a RHEL qemu-kvm host; unit tests
check the XML flags, the console setters and the resize paths in CI.
add_network built the interface XML with f-strings, so a posted model
or nwfilter could add arbitrary devices to the domain. It is built with
lxml now; a missing MAC is left to libvirt instead of address='None'.
Taking or reverting an internal snapshot redefined a pflash VM with a
rom loader around the operation. libvirt 10.10 rejects that definition,
so the request ended in a 500; where it was accepted, the snapshot
recorded the fake loader, and a failure between rewrite and restore
left the VM unable to boot.
The definition is no longer touched: libvirt decides, and its error is
shown (for a running VM with raw NVRAM: 'internal snapshots of a VM
with pflash based firmware require QCOW2 nvram format'). A stopped VM's
disk-only internal snapshot is taken, as libvirt allows it. Revert
errors are shown too. The external snapshot path keeps its rewrite for
now.
Live tests cover running and stopped UEFI VMs; both failed with a 500
on the previous code.
Instance.proxy looked the domain up by its stored name. After a rename
or a name swap outside WebVirtCloud, an operation on one VM changed
another; after a VM was replaced under the same name, operations hit
the new VM.
wvmInstance takes the UUID and uses lookupByUUIDString, with no
fallback to the name: a missing UUID fails instead of picking another
domain. Every caller that has an Instance passes it: proxy, console,
quota, post-migration autostart and the VDI URL. Migration uses the
domain the proxy already resolved. getvvfile and the gstfsd requests
use the host's current name of that UUID.
Live tests cover a name swap and a VM replaced under the same name;
both fail on the previous code.
Destroy:
- Stops every active state, not only running; a paused VM used to keep
running as a transient domain after the undefine.
- Undefines with MANAGED_SAVE and SNAPSHOTS_METADATA, before any disk is
deleted. A managed-save image used to make the undefine fail after the
disks were already gone; now a failed undefine deletes nothing.
- Keeps disks that another domain references, directly or as a backing
file anywhere in its chain (running domains' backingStore, and the
backingStore of the volumes in the active pools for stopped ones), and
says so; they used to be deleted with the VM.
Volume delete:
- Detaches from the live domain in every active state, including paused.
- Deletes the volume only after neither the live nor the persistent
definition has the disk (bounded wait for the guest to release it),
and refuses a volume another VM references. Otherwise the volume is
kept and the user is told why.
Both hold the compute exclusively for the usage scan and the deletion,
so another VM cannot attach the disk in between; the destroy
confirmation page (GET) takes no lock.
Live tests reproduce each case on a RHEL qemu-kvm host and fail on the
previous code; unit tests cover the destroy ordering and the locks.
The edit form posts net-*-<n> fields for NIC n and carries only the
new MAC. The backend read index 0 and looked the NIC up by the new MAC,
so editing a MAC detached nothing and attached a second NIC, and edits
of any NIC but the first were applied with empty values.
The form now also posts the current MAC (net-old-mac-<n>). The view
reads the submitted NIC's fields, validates the new MAC and reports
errors. The backend replaces the NIC with the old MAC, refuses an
unknown one, and on a failed attach restores the old NIC and raises
instead of reporting success.
The view crashed with a TypeError without a Referer header; it now
returns to the instance page, and a Referer from another site is
ignored.
Super-linter ran from the unpinned :latest image and, because of
branches-ignore: [master], never on pull requests to master. Ruff now
checks syntax errors and pyflakes rules (E9, F) on every push and PR.
Fixes the existing findings: unused imports and variables, an unused
duplicate import in instances/tests.py, and the star import in
accounts/views.py. settings-dev.py keeps its star import of the base
settings.
Disk edits:
- edit_disk built a new <disk> from the form and replaced the old one,
losing what the form does not show (e.g. encryption and virtio
driver options); a bus change detached and re-attached the disk.
The disk of the persistent definition is now edited in place and the
domain defined once; a bus change picks a free device name and drops
the old address and virtio-only options. Pending persistent changes of
a running VM are kept.
- Clone omits empty disk devices, refuses a disk without a supported
source and destination, and checks the name before allocating storage.
Locks:
- Every VM change held its compute's exclusive lock, so one slow
operation blocked all VMs on that compute. A VM change now holds its
own VM lock plus a shared compute lock; reconciliation, migration and
cloning hold the compute exclusively (two clones of different VMs
must not pass the name check together).
- Quota-checked changes of one user (CPU, memory and disk resize,
clone) run one at a time under a per-user lock, also across computes,
so concurrent requests cannot all pass the check against the old
allocation. It is the outermost lock; superusers, who have no quota,
skip it.
- A busy lock answers 409 to the API and a message in the web UI. API
power and destroy actions return the view's error instead of success.
A disk without driver/@type could not be edited: the empty format was
rejected, and edit_disk would have written type=''. An empty format is
now accepted and edit_disk leaves the type attribute out.
The edit_volume bus change detached the disk and attached a new one built
as a file disk, so block and rbd disks lost their source and auth, and a
failed attach left the disk detached. Add wvmInstance.change_disk_bus,
which reuses the disk element and only changes <target> (dropping the old
bus address), and re-attaches the original disk if attaching fails.
edit_disk also rebuilt every disk with <source file=...>; it now keeps the
existing <source> and <auth> of block and network disks.
Disk and media mutation views (add_new_vol, add_existing_vol, edit_volume,
delete_vol, detach_vol, add_cdrom, detach_cdrom, mount_iso, unmount_iso)
were reachable by any owner with is_change, although the UI only shows the
Disk tab to superusers. An ordinary user could delete any volume on the
compute or attach arbitrary host files to their VM.
- Require superuser for all disk/media views, matching the UI.
- delete_vol resolves the volume from the VM's own disk XML by dev and
ignores the posted storage/name.
- Validate bus/cache/io/discard/detect_zeroes against the hypervisor's
lists, and format/serial/volume name against strict patterns.
- add_existing_vol only accepts volumes that exist in the selected pool.
- edit_volume only accepts a dev attached to the VM.
- vrtManager: escape all values interpolated into disk XML and pass dev
as an XPath variable in detach_disk/edit_disk.
- Migrate network_size and wvmNetwork IP methods to Python built-in ipaddress
- Remove obsolete vendored vrtManager/IPy.py (1650+ lines)
- Add comprehensive unit test suite in vrtManager/test_network_ip.py (15 tests)
- Clean up obsolete IPy exclusion in .travis.yml
