Part of qemuSaveImageStartVM() function will be used when reverting
external snapshots. To avoid duplicating code and logic extract the
shared bits into separate function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, nbdkit support will automatically be enabled as long as
the pidfd_open(2) syscall is available. Optionally, libnbd is used
to generate more user-friendly error messages.
In theory this is all good, since use of nbdkit is supposed to be
transparent to the user. In practice, however, there is a problem:
if support for it is enabled at build time and the necessary
runtime components are installed, nbdkit will always be preferred,
with no way for the user to opt out.
This will arguably be fine in the long run, but right now none of
the platforms that we target ships with a SELinux policy that
allows libvirt to launch nbdkit, and the AppArmor policy that we
maintain ourselves hasn't been updated either.
So, in practice, as of today having nbdkit installed on the host
makes network disks completely unusable unless you're willing to
compromise the overall security of the system by disabling
SELinux/AppArmor.
In order to make the transition smoother, provide a convenient
way for users and distro packagers to disable nbdkit support at
compile time until SELinux and AppArmor are ready.
In the process, detection is completely overhauled. libnbd is
made mandatory when nbdkit support is enabled, since availability
across operating systems is comparable and offering users the
option to make error messages worse doesn't make a lot of sense;
we also make sure that an explicit request from the user to
enable/disable nbdkit support is either complied with, or results
in a build failure when that's not possible. Last but not least,
we avoid linking against libnbd when nbdkit support is disabled.
At the RPM level, we disable the feature when building against
anything older than Fedora 40, which still doesn't have the
necessary SELinux bits but will hopefully gain them by the time
it's released. We also allow nbdkit support to be disabled at
build time the same way as other optional features, that is, by
passing "--define '_without_nbdkit 1'" to rpmbuild. Finally, if
nbdkit support has been disabled, installing libvirt will no
longer drag it in as a (weak) dependency.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Wrap the macro body in a new block and move the declaration of 'tmp'
into it, to avoid the need to mix g_autofree with manual freeing.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Hypervisors are referred to by their user-facing name rather
than the name of their libvirt driver, the monolithic daemon is
explicitly referred to as legacy, and a consistent format is
used throughout.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Requires/Wants only tells systemd that the corresponding unit
should be started when the current one is, but that could very
well happen in parallel. For virtlogd/virtlockd, we want the
socket to be already active when the hypervisor driver is
started.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We're about to change the defaults and start migrating to common
templates: in order to be able to switch units over one at a
time, make the input files that are currently used explicit
rather than implicit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virBitmapFormat returns the string that should be freed.
All strings in three ADD_BITMAP calls in qemuDomainGetGuestVcpusParams
are contained in tmp. So memory leak is possible here without VIR_FREE.
Fixes: 0108deb944af5ca6f1da350c9d0352c8ed18738b
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that providing the value is optional, we can remove almost
all uses.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
For most services, the value provided explicitly matches the
documented default.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The current message can be misleading, because it seems to suggest
that no firmware of the requested type is available on the system.
What actually happens most of the time, however, is that despite
having multiple firmwares of the right type to choose from, none
of them is suitable because of lacking some specific feature or
being incompatible with some setting that the user has explicitly
enabled.
Providing an error message that describes exactly the problem is
not feasible, since we would have to list each candidate along
with the reason why we rejected it, which would get out of hand
quickly.
As a small but hopefully helpful improvement over the current
situation, reword the error message to make it clearer that the
culprit is not necessarily the firmware type, but rather the
overall domain configuration.
Suggested-by: Michael Kjörling <7d1340278307@ewoof.net>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function virGetConnectSecret() can return NULL so we need to check it
since in virSecretGetSecretString() it gets dereferenced.
Reported-by: coverity
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
It's not possible to use password-protected ssh keys directly with
libvirt because libvirt doesn't have any way to prompt a user for the
password. To accomodate password-protected key files, an administrator
can add these keys to an ssh agent and then configure the domain with
the path to the ssh-agent socket.
Note that this requires an administrator or management app to
configure the ssh-agent with an appropriate socket path and add the
necessary keys to it. In addition, it does not currently work with
selinux enabled. The ssh-agent socket would need a label that libvirt
would be allowed to access rather than unconfined_t.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For ssh disks that are served by nbdkit, we can support logging in with
an ssh key file. Pass the path to the configured key file and the
username to the nbdkit process.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For ssh disks that are served by nbdkit, use the configured value for
knownHosts and pass it to the nbdkit process.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For ssh disks that are served by nbdkit, lookup the password from the
configured secret and securely pass it to the nbdkit process using fd
passing.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When using nbdkit to serve a network disk source, the nbdkit process
will start and wait for an nbd connection before actually attempting to
connect to the (remote) disk location. Because of this, nbdkit will not
report an error until after qemu is launched and tries to read from the
disk. This results in a fairly user-unfriendly error saying that qemu
was unable to start because "Requested export not available".
Ideally we'd like to be able to tell the user *why* the export is not
available, but this sort of information is only available to nbdkit, not
qemu. It could be because the url was incorrect, or because of an
authentication failure, or one of many other possibilities.
To make this friendlier for users and easier to detect
misconfigurations, try to connect to nbdkit immediately after starting
nbdkit and before we try to start qemu. This requires adding a
dependency on libnbd. If an error occurs when connecting to nbdkit, read
back from the nbdkit error log and provide that information in the error
report from qemuNbdkitProcessStart().
User-visible change demonstrated below:
Previous error:
$ virsh start nbdkit-test
2023-01-18 19:47:45.778+0000: 30895: error : virNetClientProgramDispatchError:172 : internal
error: process exited while connecting to monitor: 2023-01-18T19:47:45.704658Z
qemu-system-x86_64: -blockdev {"driver":"nbd","server":{"type":"unix",
"path":"/var/lib/libvirt/qemu/domain-1-nbdkit-test/nbdkit-libvirt-1-storage.socket"},
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}: Requested export not
available
error: Failed to start domain 'nbdkit-test'
error: internal error: process exited while connecting to monitor: 2023-01-18T19:47:45.704658Z
qemu-system-x86_64: -blockdev {"driver":"nbd","server":{"type":"unix",
"path":"/var/lib/libvirt/qemu/domain-1-nbdkit-test/nbdkit-libvirt-1-storage.socket"},
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}: Requested export not
available
After this change:
$ virsh start nbdkit-test
2023-01-18 19:44:36.242+0000: 30895: error : virNetClientProgramDispatchError:172 : internal
error: Failed to connect to nbdkit for 'http://localhost:8888/nonexistent.iso': nbdkit: curl[1]:
error: problem doing HEAD request to fetch size of URL [http://localhost:8888/nonexistent.iso]:
HTTP response code said error: The requested URL returned error: 404
error: Failed to start domain 'nbdkit-test'
error: internal error: Failed to connect to nbdkit for 'http://localhost:8888/nonexistent.iso]:
error: problem doing HEAD request to fetch size of URL [http://localhost:8888/nonexistent.iso]:
HTTP response code said error: The requested URL returned error: 404
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Adds the ability to monitor the nbdkit process so that we can take
action in case the child exits unexpectedly.
When the nbdkit process exits, we pause the vm, restart nbdkit, and then
resume the vm. This allows the vm to continue working in the event of a
nbdkit failure.
Eventually we may want to generalize this functionality since we may
need something similar for e.g. qemu-storage-daemon, etc.
The process is monitored with the pidfd_open() syscall if it exists
(since linux 5.3). Otherwise it resorts to checking whether the process
is alive once a second. The one-second time period was chosen somewhat
arbitrarily.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We were testing the arguments that were being passed to qemu when a disk
was being served by nbdkit, but the arguments used to start nbdkit
itself were not testable. This adds a test to ensure that we're invoking
nbdkit correctly for various disk source definitions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For virStorageSource objects that contain an nbdkitProcess, start that
nbdkit process to serve that network drive and then pass the nbdkit
socket to qemu rather than sending the network url to qemu directly.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Rather than passing passwords and cookies (which could contain
passwords) to nbdkit via commandline arguments, use the alternate format
that nbdkit supports where we can specify a file descriptor which nbdkit
will read to get the password or cookies.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add xml to the private data for a disk source to represent the nbdkit
process so that the state can be re-created if the libvirt daemon is
restarted. Format:
<nbdkit>
<pidfile>/path/to/nbdkit.pid</pidfile>
<socketfile>/path/to/nbdkit.socket</socketfile>
</nbdkit>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This prepares encryption secrets and authentication secrets. When we add
nbdkit-backed network storage sources, we will not need to send
authentication secrets to qemu, since they will be sent to nbdkit
instead. So split this into two different functions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add new DO_TEST_CAPS_LATEST_NBDKIT macro to test xml2argv for various
nbdkit capability scenarios.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
log stderr and stdout from nbdkit into its own log so that
nbdkit-related issues can be debugged more easily.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This code can be used by the nbdkit implementation for reading back
filtered log data for error reporting. Move it to qemuLogContext so that
it can be shared. Renamed to qemuLogContextReadFiltered().
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow us to use it for nbdkit logging in upcoming commits.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Allow to specify a basename for the log file so that
qemuDomainLogContextNew() can be used to create log contexts for
secondary loggers.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add some helper functions to build a virCommand object and run the
nbdkit process for a given virStorageSource.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Rather than hard-coding the nbdkit module directory, query the nbdkit
binary for the location to these directories. nbdkit provides a
--dump-config optiont that outputs this information and can be easily
parsed. We can also get the version from this output rather than
executing `nbdkit --version` separately.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
An object for storing information about a nbdkit process that is serving
a specific virStorageSource. At the moment, this information is just
stored in the private data of virStorageSource and not used at all.
Future commits will use this data to actually start a nbdkit process.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add the virFileCache implementation for nbdkit capabilities to the qemu
driver. This allows us to determine whether nbdkit is installed and
which plugins are supported. it also has persistent caching and the
capabilities are re-queried whenever something changes.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Implement the loadFile and saveFile virFileCacheHandlers callbacks so
that nbdkit capabilities are cached perstistently across daemon
restarts. The format and implementation is modeled on the qemu
capabilities, but simplified slightly.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Preparatory step for caching nbdkit capabilities. This patch implements
the newData and isValid virFileCacheHandlers callback functions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In order to add caching of the nbdkit capabilities, we will need to
compare against file modification times, etc. So look up this
information when creating the nbdkit caps.
Add a nbdkit_moddir build option to allow the builder to specify the
location to look for nbdkit plugins and filters.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In future commits, we will optionally use nbdkit to serve some remote
disk sources. This patch queries to see whether nbdkit is installed on
the host and queries it for capabilities. The data will be used in later
commits.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There are few places where the following pattern occurs:
if (var)
other = g_strdup(var);
where @other wasn't initialized before g_strdup(). Checking for
var != NULL is useless in this case, as that's exactly what
g_strdup() does (in which case it returns NULL).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parsers to use
virXMLPropEnumDefault().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field, adjust the XML parsers to use virXMLPropEnum()
and fill in missing cases to switch() statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parsers to use
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parser to use
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parser to use
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and fill in missing cases to switch()
statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field, and fill in missing cases to switch()
statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field, adjust the XML parser to use
virXMLPropEnumDefault() and fill in missing cases to switch()
statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parser to use
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'bus' member of _virDomainDiskDef is already declared of
virDomainDiskModel type. Hence, there is no need to typecast the
variable when passing to switch() statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'mode' member of _virDomainDiskDef is already declared of
virDomainDiskModel type. Hence, there is no need to typecast the
variable when passing to switch() statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
