In preparation to move block job related helpers to a dedicated
module. Like this, moving the code will be clearly visible in the diff
without any changed lines sticking out in between.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
For now, move only the vm_resume() and vm_suspend() functions. Others
like vm_stop() and friends, vm_reboot() and vm_start() would require
more preparation.
Apart from slightly improving modularity, this is in preparation to
add a BlockJob module, where vm_resume() and vm_suspend() need to be
called after drive-mirror, to avoid a cyclic dependency with the main
QemuServer module.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The QemuMigrate module is high-level and should not be called from
many other places, while also being an implementation of
AbstractMigrate, so using a separate module is much more natural.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
While not the main motivation, this has the nice side effect of
removing a call from QemuConfig to the QemuServer main module.
This is in preparation to introduce a RunState module which does not
call back into the main QemuServer module. In particular, qm_suspend()
will be moved to RunState which needs to call find_vmstate_storage().
Intuitively, the StateFile module seems like the most natural place
for find_vmstate_storage(), but moving find_vmstate_storage() requires
moving foreach_storage_used_by_vm() too and that function calls into
QemuConfig. Now, QemuConfig also calls find_vmstate_storage(), meaning
a cyclic dependency would result.
Note that foreach_storage_used_by_vm(), is related to foreach_volume()
and also uses foreach_volume(), so QemuConfig is the natural place for
that function.
So the arguments for moving find_vmstate_storage() to QemuConfig are:
1. most natural way to avoid cylcic dependencies.
2. related function foreach_storage_used_by_vm() belongs there too.
3. qm_suspend() and other functions relating to the run state already
call other QemuConfig methods.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Makes it possible to call into the module from the main QemuServer
module and other modules that are used by QemuServer without adding a
cyclic dependency.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The QemuConfig module uses qga_check_running() which is planned to be
moved to the Agent module. Loading the config on the call-sites of
agent_cmd(), qemu_exec() and qemu_exec_status() makes it possible to
avoid introducing that cyclic dependency.
Note that the import for the QemuConfig module was already missing.
Also drops unused variables $write and $res from the 'file-write' API
endpoint implementation.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Both agent_available() and agent_cmd() have no callers that use the
$noerr argument.
The current implementation was not ideal: agent_cmd() did not check
the return value of agent_available() in the $noerr scenario. It
should return early. In agent_available(), failure was silently
ignored in the $noerr scenario. Having a message or warning then would
have been more useful.
The agent_available() function is renamed to assert_agent_available()
and it is not exported anymore, the single caller outside the module
can just call it with the full module path.
The import of 'agent_available' in qm.pm was not used and is dropped.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Grepping in /usr/share/perl5/PVE, the standard option is never used
and it was accidentally assigned $netdesc rather than $ipconfigdesc.
This can be seen in commit 0c9a7596 ("implement cloudinit").
Can still be added correctly later if the need arises.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
pve-manager >= 8.2.10 has a hard dependency on libpve-network-perl
which includes the required modules.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This is in preparation for the switch to -blockdev, where it will be
necessary to specify the 'pflash0' and 'pflash1' machine flags.
Suggested-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
For EFI disks in raw format, it is necessary to specify a precise size
and have no padding. See commit 818ce80e ("fix efidisks on storages with
minimum sizes bigger than OVMF_VARS.fd") for details.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Enforce the 'writeback' cache setting for an EFI disk with RBD driver
to work around issue #3229. See also commit 6aaad230 ("fix #3329: turn
on cache=writeback for efidisks on rbd").
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> Using $a or $b outside sort() at line 193, column 13. $a and $b are
> special package variables for use in sort() and related functions.
> Declaring them as lexicals like "my $a" may break sort(). Use
> different variable names. (Severity: 4)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
For the HMP 'drive_add' command, the used timeout is 1 minute and for the
'drive_del' command, the used timeout is 10 minutes, because IO might
need to be finished. Use the same for 'blockdev-add' respectively
'blockdev-del'.
For 'drive-mirror', 10 minutes is used, so use the same for
'blockdev-mirror'.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
In the bug report, the user mentioned that 7 seconds was enough. For
the HMP 'drive_add' command, the used timeout is 1 minute and for the
'drive_del' command, the used timeout is 10 minutes, because IO might
need to be finished. While something similar might be true for certain
objects/devices, there were no issues reported with the *_del
operations using the default timeout until now and the callers can
still use a higher timeout if they know the specific device/object
requires it.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
it reset the pve-specific machine suffix to 0 again, and some preparatory
changes for the upcoming switch to blockdev are already visible in the
commandlines.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
The drive device and node structure is:
front-end device {ide-hd,scsi-hd,virtio-blk-pci} (id=$drive_id)
- throttle node (node-name=$drive_id)
- format node (node-name=f$encoded-info)
- file node (node-name=e$encoded-info)
The node-name can only be 31 characters long and needs to start with a
letter. The throttle node will stay inserted below the front-end
device. The other nodes might change however, because of drive
mirroring and similar. There currently are no good helpers to
query/walk the block graph via QMP, x-debug-query-block-graph is
experimental and for debugging only. Therefore, necessary information
is encoded in the node name to be able to find it again. In
particular, this is the volume ID, the drive ID and optionally a
snapshot name. As long as the configuration file matches with the
running instance, this is enough to find the correct node for
block operations like mirror and resize.
The 'snapshot' option, for QEMU's snapshot mode, i.e. writes are only
temporary, is not yet supported.
[FE: split up patch
expand commit message
explicitly test for drivers with aio setting
improve readonly handling
improve CD-ROM handling
fix failure for storage named 'nbd' by always using full regex
improve node name generation
fail when drive->{snapshot} is set]
Originally-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
These properties cannot be specified via '-blockdev' like they could
with '-drive', because they are properties of the front-end drive
device.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
With the 'blockdev' command line option, the cache options are split
up. While cache.direct and cache.no-flush can be set in the -blockdev
options, cache.writeback is a front-end property and was intentionally
removed from the 'blockdev' options by QEMU commit aaa436f998 ("block:
Remove cache.writeback from blockdev-add"). It needs to be configured
as the 'write-cache' property for the ide-hd/scsi-hd/virtio-blk
device.
Table from 'man kvm':
┌─────────────┬─────────────────┬──────────────┬────────────────┐
│ │ cache.writeback │ cache.direct │ cache.no-flush │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writeback │ on │ off │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│none │ on │ on │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writethrough │ off │ off │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│directsync │ off │ on │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│unsafe │ on │ off │ on │
└─────────────┴─────────────────┴──────────────┴────────────────┘
Suggested-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Look at the 'qdev' value, because the 'device' property is not
initialized with '-blockdev'. This can be seen in the QEMU source code
(the device property is the name of the block backend and blk->name is
assigned a value only in a code path reached via drive_new()). This
most likely was done to avoid confusion/clashes, since with
'-blockdev', the node that's inserted for a front-end device can
change and then both the block backend and the node would be named
the same, but not connected.
[FE: fix commit message and comment - it does not depend on the presence of media
escape dot in regex
skip right away if qdev is undef to avoid warning when regex matching]
Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Never reserve PCI devices when config_to_command() is called for
'showcmd'. Previously, choose_hostpci_devices() only skipped reserving
PCI devices when the VM was already running as a heuristic. Make it
explicit via a new parameter. Adapt the config-to-command test to
never expect actual reservation. There is a dedicated test for this
since commit "test: add tests for PCI reservations".
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The only supported machine for aarch64 is 'virt', so there is no need
to check if that is the machine. Also many (all?) other machines for
aarch64 in QEMU also don't have a 'pci' bus by default.
The parameter is also transitively removed from the functions:
1. print_hostpci_devices()
2. print_rng_device_commandline()
3. get_usb_controllers()
4. print_netdevice_full()
5. print_vga_device()
Should this ever be required in the future again, or also for the
$arch itself right now, it would be nicer to properly abstract this
away instead of passing the parameters along everywhere, e.g. by using
a class.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This is currently tested as part of the config-to-command tests, but
the plan is to make command generation for 'qm showcmd' not actually
reserve anything. The reserve_pci_usage() function also serves as
checking what already is reserved by other VMs at the same time.
Thus, the config-to-command test will not be able to test whether
reservations are actually respected after the above-mentioned change.
Add a dedicated test for this.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
With '-blockdev', it is necessary to activate the volumes to generate
the command line, because it can be necessary to check whether the
volume is a block device or a regular file.
Do not deactivate after commandline generation for 'qm showcmd',
because there can be concurrent operations acting on the volumes.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>