Unfortunately a LUKS image to be decrypted by qemu has
VIR_STORAGE_FILE_RAW as format, but has encryption properties populated.
Many places in the code don't check it properly and also don't check
properly whether the image is indeed LUKS to be decrypted by qemu.
Introduce helpers which will simplify this task.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Spellchecked-by: Ján Tomko <jtomko@redhat.com>
QEMU's blockdev-mirror job doesn't allow copy into a destination which
isn't exactly the same size as source. This is a problem for
non-shared-storage migration when migrating into a raw block device, as
there it's very hard to ensure that the destination size will match the
source size.
Rather than failing the migration, we can add a storage slice in such
case automatically and thus make the migration pass.
To do this we need to probe the size of the block device on the
destination and if it differs form the size detected on the source we'll
install the 'slice'.
An additional handling is required when persisting the VM as we want to
propagate the slice even there to ensure that the device sizes won't
change.
Resolves: https://issues.redhat.com/browse/RHEL-4607
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move qemuDomainStorageUpdatePhysical, qemuDomainStorageOpenStat,
qemuDomainStorageCloseStat to qemu_domain.c and export them. They'll be
reused in the migration code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function will be used to setup storage for non-shared-storage
migration, not just precreate images.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When a user provides a migration XML via the VIR_MIGRATE_PARAM_DEST_XML
it's expected that they want to change ABI-compatible aspects of the XML
such as the disk paths or similar.
If the user requests persisting of the VM but does not provide an
explicit persistent XML libvirt would take the persistent XML from the
source of the migration as the persistent config. This usually involves
the old paths to images.
Doing this would result into failure to start the VM.
It makes more sense to take the XML used for migration and use that as
the base for persisting the config.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While it's intended that qemuMigrationDstPrecreateDisk is called with
any kind of the disk, the logic in qemuMigrationDstPrecreateStorage
which checks the existence of the image wouldn't properly handle e.g.
network backed disks, where it would attempt to use virFileExists() on
the disk's 'src->path'.
Fix the logic by first skipping disks not meant for migration, then do
the existence check only when 'disk->src' is local storage.
Since qemuMigrationDstPrecreateDisk has a debug statement there's no
need to have an extra one right before calling into it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic pointer freeing for 'conn' and remove the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the error messages so that they can be used to identify the
problematic disk or image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 501825011c switched the remote driver to using g_auto, but missed
one case of needing to steal a pointer holding the hypervisor type.
Without it, memory is freed and the output of 'virsh version' has random
output
Compiled against library: libvirt 10.0.0
Using library: libvirt 10.0.0
Using API: ��%�U 10.0.0
Running hypervisor: ��U 8.1.3
Ths change also fixes random SIGABRT from perl processes running
libvirt-tck tests.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Report both block count and size together when either one of them is
present equivalently to what the schema type 'blockData' in
'schemas/nodedev.rng' defines.
Resolves: https://issues.redhat.com/browse/RHEL-18165
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unfortunately the XML is designed in a weird way, where based on whether
media in the device is removable the sizing is either part of a
subelement or placed directly on top level. The logic itself is
identical so it can be extracted into a function to simplify the
formatter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBufferEscapeString is specifically designed for formatting XMLs and
thus skips the whole formatting if the singular string argument is NULL.
Remove redundant conditions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's no point in skiping the validation step:
- on the source, the VM is parsed for ABI stability checking, thus the
equivalent config was validated when the VM was started
- on the destination, the XML will be validated inside qemuProcessInit
very soon after it is parsed
This fixes problems such as if the user uses a relative path in the disk
source or omits the source, as the disk migration code reasonably
expects that all checks were performed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Fix incorrect log message for timestamp value.
Probably this line was copied from the check for attr.
Found by Linux Verification Center (linuxtesting.org).
Fixes: 7cfb7aab57 ("security_util: Remove stale XATTRs")
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Convert prototype of virFileLinkPointsTo to return bool.
Remove dead checks in virDomainObjListLoadConfig and
virNetworkLoadConfig.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
If libvirt is built in client only mode, the libvirtd/virtqemud/etc
daemons won't exist. If the client is told to connect to a local
hypervisor, it'll see the socket doesn't exist, try to spawn the
daemon and then re-try connecting to the socket for a few seconds.
Ultimately this will fail because the daemon doesn't exist and the
user gets an error message
error: Failed to connect socket to '/run/user/1000/libvirt/virtqemud-sock': No such file or directory
technically this is accurate, but it doesn't help identify the root
cause. With this change it will now report
error: binary 'virtqemud' does not exist in $PATH: No such file or directory
and will skip all the socket connect retries
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On device-update, when user requests change of
trustGuestRxFilters we currently do nothing. Nor error out, nor
act on the request. While we can just throw an error,
implementing this is pretty trivial.
Resolves: https://issues.redhat.com/browse/RHEL-735
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Sometimes it may be handy to just issue the query-rx-filter
monitor command without actually parsing the output. Adapt
qemuMonitorJSONQueryRxFilter() to this behavior.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When cold plugging a memory device we check whether there's
enough free memory slots to accommodate new module. Well, this
checks makes sense only for those memory devices that are plugged
into DIMM slots (DIMM and NVDIMM models). Other memory device
models, like VIRTIO_MEM, VIRTIO_PMEM or SGX_EPC are attached into
PCI bus, or no bus at all.
Resolves: https://issues.redhat.com/browse/RHEL-15480
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The code that handles coldplug of a memory device is pretty
trivial and such could continue to live in the huge switch()
where other devices are handled. But the code is about to get
more complicated. To help with code readability, move it into a
separate function.
And while at it, make the function accept a double pointer to the
memory device definition to make the ownership transfer obvious
(the device is part of the domain on successful run).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We only use it at runtime, not during the build process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We only use it at runtime, not during the build process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We only use it at runtime, not during the build process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The VIR_PCI_DEVICE_ADDRESS_FMT macro is used only in virpci.c and
nowhere else. It's not necessary to expose it in the header file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When building a hostdev props, its PCI address is formatted via
g_strdup_printf(VIR_PCI_DEVICE_ADDRESS_FMT, ...); Well, we have a
function that does exactly that: virPCIDeviceAddressAsString().
Use the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The VIR_PF_PHYS_PORT_NAME_REGEX macro is used only in
virPCIGetNetName() and nowhere else. It's not necessary to expose
it in the header file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The hotplug functionality added earlier really supports only live
addition of devices, no coldplug yet rendering @devConf in
testDomainAttachDeviceLiveAndConfig() an unused variable. Remove
it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a basic support for hotplug and hotunplug of PCI
<hostdev/>-s.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A bug in qemuProcessStartWithMemoryState caused that we would start qemu
with '-loadvm SNAP' and '-incoming defer' together. qemu doesn't expect
that and crashes on an assertion failure [1].
[1]: https://issues.redhat.com/browse/RHEL-16782
Fixes: 8a88d3e586
Resolves: https://issues.redhat.com/browse/RHEL-17841
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
During CH driver initialization (chStateInitialize()) the
driver's capabilities bitmap is allocated
(virCHCapsInitCHVersionCaps()), but corresponding free call is
missing in chStateCleanup().
And while at it, reorder calls to virObjectUnref() inside of
chStateCleanup() to be the reverse order of that in
chStateInitialize() so that it's easier to spot missing
free/unref call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The raw driver layer is not needed in this case and can be dropped.
Removing the nodename will cause other pieces of the code to pick up and
stop adding the layer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only caller was converted to use the common blockdev infrastructure
thus this function is no longer needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rewrite the code to use the common tooling for removing blockdevs
instead of the ad-hoc qemuBlockStorageSourceDetachOneBlockdev helper.
Use of the common infrastructure will properly handle cases when the raw
driver is ommited from the block graph.
Since the TLS data object is shared for all migration QMP commands and
objects we need to strip its alias from the definition of the storage
source before attempting to detach it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make the helper reopening a blockdev for access pick the correct layer
to reopen based on what is currently in use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Take the virJSONValue array object which is passed to the
'blockdev-reopen' command as the 'options' argument rather than making
the caller wrap all the properties.
The code was a leftover from the time when the blockdev-reopen command
had a different syntax, and thus can be cleaned up.
Also note that the logging of the node name never worked as the top
level object didn't ever contain a 'node-name' property.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move all the logic into the new function and remove the old one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We want to preserve the wrappers for clarity but the inner logic can be
extracted to a common function qemuBlockReopenAccess. In further patches
the code from qemuBlockReopenFormat will be merged into the new wrapper
as well as logic for handling scenarios with missing 'format' layers
will be added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Return the effective storage nodename if the format layer is not
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to other bits of code, we don't need to setup the format layer
if it will not be formatted. Add logic which uses
qemuBlockStorageSourceNeedsFormatLayer to see whether the setup of the
format node is needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Setup the data for detaching of the 'format' layer only when it's
present.
Restructure the logic to follow the same order as
qemuBlockStorageSourceAttachPrepareBlockdev in terms of
format/slice/storage -blockdev objects, and drop the now-misleading
comment for 'slice' of raw disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Restructure the code logic so that the function is prepared for the
possibility that the 'format' blockdev layer may be missing if not
needed.
To achieve this we need to introduce logic that selects which node
(format/slice/storage) becomes the effective node and thus formats the
correct set of arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow using the slice layer as effective layer once we stop formatting
the unnecessary 'raw' driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'format' layer is not required in certain cases. As the logic for
this will be a bit more involved create a helper function to do the
decision.
For now we'll keep to always format the 'format' -blockdev layer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a note stating that qemuBlockStorageSourceNeedsStorageSliceLayer
must be used only when setting up a new blockdev, any other case when
the device might been already set up must use the existence of the
nodename to do so.
Adjust qemuBlockStorageSourceAttachPrepareBlockdev to do so and refactor
qemuBlockStorageSourceDetachPrepare to use the same logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The helper retrieves the nodename of the slice layer if it's present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu allows and in some cases uses protocol driver names ('file',
'host_device', 'nbd', ...) in the 'backing file format' field of a qcow
to denote a image where the dummy 'raw' driver was not used on top.
Adapt our backing store parser for such cases. The examples added in
previous patch show the difference in behaviour.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace the return values by 0 because none of the callers care and some
of the backing store parser functions return this state also in cases
the rest of the code would consider as success.
Subsequently the parsers will be refactored and proper error reporting
returned.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we consider the failure of parsing the backing store to be
actually success based on the value we return to the caller, we should
continue parsing also features and the 'compat' field so that we don't
have a partial definition if e.g. the backing store format is not known.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
None of the backing store parser functions actually use it. Remove it to
avoid confusion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In v9.9.0-104-gc472ce024b I've introduced another value to
virDomainAudioType enum. But I forgot to add corresponding case
into switch() in bhyveBuildSoundArgStr().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This is mostly straightforward, except for a teensy-weensy
detail: usually, there's no system wide daemon running, no system
wide available socket that anybody could connect to. PipeWire
uses a per user daemon approach instead. But this in turn means,
that the socket location floats between various locations and is
derived from various environment variables (just like the actual
socket name) and thus we must pass the variables to QEMU.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/560
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU gained support for PipeWire audio backend (see QEMU commit
of v8.0.0-403-gc2d3d1c294). Its configuration knobs are basically
the same as pulseaudio's, except for PA's server name. Therefore,
a lot of code is copied over from pulseadio and fixed by
s/Pulse/Pipewire/ or s/pulseaudio/pipewire/.
There's one ley difference to PA though: pipewire daemon is
usually on per user basis (just like our qemu:///session).
Therefore, introduce this 'runtimeDir' attribute, which allows
specifying path to pipewire daemon socket (useful for
qemu:///system for instance).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On systems with humongous pages (16GiB) and 32bit int it's easy
to hit integer overflow in virNumaGetPages(). What happens is,
inside of virNumaGetPages() as we process hugepages for given
NUMA node (e.g. in order to produce capabilities XML), we keep a
sum of sizes of pools in an ULL variable (huge_page_sum). In each
iteration, the variable is incremented by 1024 * page_size *
page_avail. Now, page_size is just an uint, so we have:
ULL += U * U * ULL;
and because of associativity, U * U is computed first and since
we have two operands of the same type, no type expansion happens.
But this means, for humongous pages (like 16GiB) the
multiplication overflows.
Therefore, move the multiplication out of the loop. This helps in
two ways:
1) now we have ULL += U * ULL; which expands the uint in
multiplication,
2) it saves couple of CPU cycles.
Resolves: https://issues.redhat.com/browse/RHEL-16749
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If any of the images in a chain above a raw image have bitmaps, libvirt
would attempt to merge them when doing a block commit or block copy
operation, which would result into a error in the logs as creating
persistent bitmaps in a raw image is not supported.
Since libvirt cares only about persistent bitmaps we can simply skip the
operation if the target of a block copy or block commit is a raw image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The VM will require access also to the detected images. Unfortunately a
recent reordering of the code introduced a bug where the backing chain
was probed after setting up cgroups/selinux/namespaces, which caused
that any detected images were not allowed/added and qemu was then not
able to use them.
Fixes: 9b8bb536ff
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virDomainMemoryDefCheckConflict() already does the same set
of checks. There's no need to duplicate them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since we're iterating over def->mems array, might as well check
for dimm slot duplicates.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far we check whether virtio-mem and/or virtio-pmem memory
devices do not overlap with each other. But we allow specifying
address where dimm and nvdimm memory devices are mapped too. And
there are left out from this collision check. Not anymore.
This leaves just sgx model out, but that's expected since it
can't have any address (see virDomainMemoryDefValidate()).
Resolves: https://issues.redhat.com/browse/RHEL-4452
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
At the end of virDomainMemoryDefValidate() there's a code that
checks whether two virtio-mem/virtio-pmem devices don't overlap.
Separate this code into its own function
(virDomainMemoryDefCheckConflict()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Differences from qemu:
* "vmx-ept-uc" (bit 8) and "vmx-ept-wb" (bit 14) are not added to
qemu's list of named features yet, but used in several qemu cpu
models never the less. Add to libvirt regardless.
* "vmx-invvpid-single-context" (bit 41) is erroneously called
"vmx-invept-single-context" in qemu. This is the name of the
feature associated with bit 25 in both libvirt and qemu.
* "vmx-invvpid-single-context-noglobals" (bit 43) is erroneously
called "vmx-invept-single-context-noglobals". Use the correct name.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Some guest OSes require cpu features from the vmx-* family,
e.g. vmx-xsaves. Up to now, libvirt ignored these features as they
were not required yet. qemu does not automatically enable e.g.
"vmx-xsaves" when requesting "xsaves":
qmp="qemu-kvm -machine accel=kvm -nodefaults -nographic -qmp stdio"
$(qmp) <<-EOF | jq | grep "xsaves"
{ "execute": "qmp_capabilities" }
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "full",
"model": {
"name": "Skylake-Client-v1",
"props": { "xsaves": true } `# set to "true" or "false"`
}
}
}
{ "execute": "quit" }
EOF
with xsaves "false":
"xsaves": false,
"vmx-xsaves": false,
with xsaves "true":
"xsaves": true,
"vmx-xsaves": false,
Stop ignoring vmx-* features and begin adding them to libvirt's
database.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
While glibc provides qsort(), which usually is just a mergesort,
until sorting arrays so huge that temporary array used by
mergesort would not fit into physical memory (which in our case
is never), we are not guaranteed it'll use mergesort. The
advantage of mergesort is clear - it's stable. IOW, if we have an
array of values parsed from XML, qsort() it and produce some
output based on those values, we can then compare the output with
some expected output, line by line.
But with newer glibc this is all history. After [1], qsort() is
no longer mergesort but introsort instead, which is not stable.
This is suboptimal, because in some cases we want to preserve
order of equal items. For instance, in ebiptablesApplyNewRules(),
nwfilter rules are sorted by their priority. But if two rules
have the same priority, we want to keep them in the order they
appear in the XML. Since it's hard/needless work to identify
places where stable or unstable sorting is needed, let's just
play it safe and use stable sorting everywhere.
Fortunately, glib provides g_qsort_with_data() which indeed
implement mergesort and it's a drop in replacement for qsort(),
almost. It accepts fifth argument (pointer to opaque data), that
is passed to comparator function, which then accepts three
arguments.
We have to keep one occurance of qsort() though - in NSS module
which deliberately does not link with glib.
1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a435afcc3048e0b697b6cc04
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Originally the disk hotplug code didn't know how to attach a CD-ROM
drive, thus didn't have the necessary logic to handle empty cdroms.
Other disks can't be empty which is enforced by the parser validation
logic.
When support for hotplugging cdroms was added the code was not adjusted
to deal with empty drives thus attempted to setup the blockdev backend
for it.
Fixes: 3078799fef
Resolves: https://issues.redhat.com/browse/RHEL-16870
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit allowing hotplug of CDROMs moved the logic forbidding the hotplug
to the appropriate blocks based on the disk frontend but forgot to
actually bail out on such error.
Fixes: 3078799fef
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When I've originally refactored the function in commit 0d981bcefc
the logic was still correct, but then later in commit 52f8655439
I've moved most of the image setup logic into the function neglecting to
add the 'goto cleanup;' needed to skip over the setup of the disk
images.
Fixes: 52f8655439
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reword the error message to clearly state that the machine type doesn't
support the address type. It doesn't matter which device it's for.
Additionally the alias may be still NULL at the point when the error is
being reported misleading users that they have something wrong with a
specific device.
Resolves: https://issues.redhat.com/browse/RHEL-16878
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
A domain name is expected to be non-empty, and we validate this when
parsing XML, or accepting a new name during renames. We fail to
enforce this property, however, when performing a migration. This
was discovered when a user complained about inaccessible VMs after
migrating with the Rust APIs which mistakenly hardcoded 'dname' to
the empty string.
Fixes: https://gitlab.com/libvirt/libvirt-rust/-/issues/11
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As a guard against programming errors, one part of the condition
only dereferences srcpool if it exists, other one does not.
Move the check up one level so that it actually has a chance to do
something useful.
Fixes: 19b1c0d319
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Now that we have virXMLParseWithIndent() and
virXMLParseStringCtxtWithIndent(), we can use them directly and
drop calls to xmlKeepBlanksDefault().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When parsing an XML it may be important to keep indentation to
produce a better looking result when formatting the XML back.
Just look at all those xmlKeepBlanksDefault() calls just before
virXMLParse() is called.
Anyway, as of libxml2 commit v2.12.0~108 xmlKeepBlanksDefault()
is deprecated. Therefore, introduce virXMLParse...WithIndent()
variants which would do exactly xmlKeepBlanksDefault() did but
with non-deprecated APIs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virXMLParseHelper() can work in two modes: either it parses a
file or a string. Either way, the same set of flags is specified
in call of corresponding function. Save flags in a local variable
instead.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After libxml2's commit of v2.12.0~101 we no longer get
xmlIndentTreeOutput declaration by us including just
libxml/xpathInternals.h and libxml2's header files leakage.
Resolves: https://bugs.gentoo.org/917516
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in previous commit, VirtualBox has its own snapshot
XML which we parse, change and then format back. During this, we
ought to keep the indentation to produce better looking result
(especially when we want to compare the output in tests later on,
like we do in vboxsnapshotxmltest).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When working with VirtualBox's snapshots, the snapshot XML is
firstly parsed, stored in memory (with some parts being stored as
verbatim XML snippets, strings), requested changes are made and
then this modified XML is formatted via
virVBoxSnapshotConfSaveVboxFile() which calls
xmlParseInNodeContext() to format those previously stored XML
snippets.
The first parse of whole VirtualBox snapshot file is done using
virXMLParse() (in virVBoxSnapshotConfLoadVboxFile()) and thus
with XML_PARSE_NONET specified.
But those ad-hoc parsings when formatting the XML back pass zero
flags mask: xmlParseInNodeContext(..., options = 0, ...);
This is potentially dangerous.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
gpg-agent can be used instead of ssh-agent to authenticate
against an SSH server, but in order to do so the GPG_TTY and
TERM environment variables need to be passed through.
For obvious reasons, we avoid doing that when no_tty=1 is found
in the connection URI.
https://bugs.debian.org/843863https://gitlab.com/libvirt/libvirt/-/merge_requests/290
Thanks: Guilhem Moulin <guilhem@guilhem.org>
Thanks: Kunwu Chan <chentao@kylinos.cn>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When reverting to inactive snapshot updating the domain definition needs
to happen after the new overlays are created otherwise qemu-img will
correctly fail with error:
Trying to create an image with the same filename as the backing file
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When we revert to non-leaf snapshot and create new branch or branches
the overlay in snapshot metadata is no longer usable as a disk source
for deletion of that snapshot. We need to use other places to figure out
the correct storage source.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/534
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
CCW addresses need to be also checked for ABI stability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
We recently unified all services and sockets, except a couple
were missed. Finish the job.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Problem with HW_PHYSMEM sysctl on 64-bit macOS is that it
returns a 32-bit signed value. Thus it overflows. Switching to
HW_MEMSIZE is recommended as it's of an uint_64 type [1].
1: https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/bsd/sys/sysctl.h
Reported-by: Jaroslav Suchanek <jsuchane@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Xcode 15, which provides the compiler toolchain for building libvirt
on macOS has switched to a new linker that warns about duplicated
"-lblah" options on the ld commandline. In practice this is impossible
to prevent in a large project, and also harmless.
Fortunately the new ld command also has an option,
-no_warn_duplicate_libraries, that supresses this harmless/pointless
warning, meson has a simple way to check if that option is supported,
and libvirt's meson.build files already have examples of adding an
option to the ld commandline if it's available.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This replaces use of 'rpcgen' with our new python impl of
the RPC code generator. Since the new impl generates code
that matches our style/coding rules, and does not contain
long standing bugs, we no longer need to post-process the
output.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current RPC code is post-processed to introduce an
intermediate variable, rather than casting directly
to char ** at time of use. This is said to be a workaround
for type-puning warnings that the compiler emitted.
Neither GCC or CLang emit any warnings for the code in
question today, across any of the architectures we
test in CI. Thus it is presumed that somewhere in the
15 years since the workaround was done, the compilers
have got smarter and do the right thing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit changing the code to allow passing NULL as @data into
qemuSaveImageDecompressionStart() was not correct as it left the
original call into the function as well.
Introduced-by: 2f3e582a1a
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2247754
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the default for the version of rpcgen shipped with
Linux distributions, but the one in macOS and possibly others
default to K&R C, which modern compilers don't appreciate.
Luckily, all versions of rpcgen shipped with our target
platforms seem to support the -C option.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_SKIP_UNMAP is no longer
referenced inside the code.
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_AUTO_READONLY is passed from
various code paths to the qemuBlockStorageSourceGetBackendProps helper,
but it's no longer used.
Both thus can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code was refactored to format the 'read-only' and 'auto-read-only'
flags via the common helper, so the logic determining their values can
be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the qemuBlockStorageSourceAddBlockdevCommonProps helper when
formatting protocol layer both when it's used as backing for a format
node and when it's used as the effective node.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The resulting properties are identical, as the hostdev backend code
doesn't set any of the extra properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a mode where the protocol layer -blockdev will be formatted
so that it can be used as the effective node (used to access data from
the device). For this new mode we'll use
qemuBlockStorageSourceAddBlockdevCommonProps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new helper in qemuBlockStorageSourceGetBlockdevStorageSliceProps
to format the common bits.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The new helper replaces qemuBlockStorageSourceGetBlockdevFormatCommonProps
and the two inline instances generating the common properties for a
blockdev layer.
The new helper is to be used for both the format layer and the storage
backing layer, thus a new parameter 'effective' switches between the
modes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the same ordering of the relevant fields as we do for the format
layer -blockdev so that later they can be refactored without test
fallout.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the return value type to 'virDomainDiskGetDetectZeroes'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
At this point only a single code path (for formatting -drive for legacy
SD cards) uses the 'legacy' output and that code path doesn't populate
the node name. Thus we can unify the code block and simplify the JSON
formatters.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'systemd-analyze security' command looks at the unit file
configuration and reports on any settings which increase the
attack surface for the daemon. Since most systemd units are
fairly minimalist, this is generally informing us about settings
that we never put any thought into using before.
In its current configuration it reports
# systemd-analyze security virtlogd.service
...snip...
→ Overall exposure level for virtlogd.service: 9.6 UNSAFE 😨
which is pretty terrible as a score.
If we apply all of the recommendations that appear possible
without (knowingly) breaking functionality it reports:
# systemd-analyze security virtlogd.service
...snip...
→ Overall exposure level for virtlogd.service: 2.2 OK 🙂
which is a pretty decent improvement.
Some of the settings we would like to enable require a systemd
version that is newer than that available in our oldest distro
target - RHEL-8 at v239.
NB, RestrictSUIDSGID is technically newer than 239, but RHEL-8
backported it, and other distros we target have it by default.
Remaining recommendations are
✗ CapabilityBoundingSet=~CAP_(DAC_*|FOWNER|IPC_OWNER)
We block FOWNER/IPC_OWNER, but can't block the two DAC
capabilities. Historically apps/users might point QEMU
to log files in $HOME, pre-created with their own user
ID.
✗ IPAddressDeny=
Not required since RestrictAddressFamilies blocks IP
usage. Ignoring this avoids the overhead of creating
a traffic filter than will never be used.
✗ NoNewPrivileges=
Highly desirable, but cannot enable it yet, because it
will block the ability to transition to the virtlogd_t
SELinux domain during execve. The SELinux policy needs
fixing to permit this transition under NNP first.
✗ PrivateTmp=
There is a decent chance people have VMs configured
with a serial port logfile pointing at /tmp. We would
cause a regression to use private /tmp for logging
✗ PrivateUsers=
This would put virtlogd inside a user namespace where
its root is in fact unprivileged. Same problem as the
User= setting below
✗ ProcSubset=
Libraries we link to might read certain non-PID related
files from /proc
✗ ProtectClock=
Requires v245
✗ ProtectHome=
Same problem as PrivateTmp=. There's a decent chance
that someone has a VM configured to write a logfile
to /home
✗ ProtectHostname=
Requires v241
✗ ProtectKernelLogs
Requires v244
✗ ProtectProc
Requires v247
✗ ProtectSystem=
We only set it to 'full', as 'strict' is not viable for
our required usage
✗ RootDirectory=/RootImage=
We are not capable of running inside a custom chroot
given needs to write log files to arbitrary places
✗ RestrictAddressFamilies=~AF_UNIX
We need AF_UNIX to communicate with other libvirt daemons
✗ SystemCallFilter=~@resources
We link to libvirt.so which links to libnuma.so which has
a constructor that calls set_mempolicy. This is highly
undesirable todo during a constructor.
✗ User=/DynamicUser=
This is highly desirable, but we currently read/write
logs as root, and directories we're told to write into
could be anywhere. So using a non-root user would have
a major risk of regressions for applications and also
have upgrade implications
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Setup the VDPA bits of the appropriate part of the image chain for block
copy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code which opens the VDPA device and prepares it for FD passing was
not called in the hotplug code path, preventing hotplug of VDPA disks
with:
error: internal error: argument key 'path' must not have null value
Use the new helper qemuProcessPrepareHostStorageDisk to setup the VDPA
definition.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/539
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently the code sets up only VDPA backends but will be used later in
hotplug code too.
This patch also uses normal forward iteration in the loop in
qemuProcessPrepareHostStorage as we don't need to remove disks from the
disk list at that point.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Given that this variable now controls not just whether C tests
are built, but also whether any test at all is executed, the new
name is more appropriate.
Update the description for the corresponding meson option
accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Currently, passing -Dtests=disabled only disables a subset of
tests: those that are written in C and thus require compilation.
Other tests, such as the syntax-check ones and those that are
implemented as scripts, are always enabled.
There's a potentially dangerous consequence of this behavior:
when tests are disabled, 'meson test' will succeed as if they
had been enabled. No indication of this will be shown, so the
user will likely make the reasonable assumption that everything
is fine when in fact the significantly reduced coverage might
be hiding failures.
To solve this issues, disable *all* tests when asked to do so,
and inject an intentionally failing test to ensure that 'meson
test' doesn't succeed.
Best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Return whether a relevant cachemode was presented rather than returning
an error, so that callers can be simplified. Use the proper enum type as
argument rather than typecasting in the switch statement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aforementioned fields in virStorageSource struct are copies of the
disk properties, but were not converted to the proper type yet.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Certain disk config fields are mirrored between the disk and storage
source definitions, but the proper types are not available for use in
the virStorageSource definition. Move them so they can be used properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Formatting of the 'nbdkit' driven backend breaks out of the switch
statement so we don't need to have an unnecessary block and indentation
level for the case when nbdkit is not in use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuBuildDriveSourceStr' used to build the legacy -drive commandline
for SD cards is the only user of qemuDiskSourceGetProps. Move the helper
directly inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'auto-read-only' blockdev option is available in all supported qemu
versions so we can remove the migration hack which disabled it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We want at least one file to always be present, so that it can
serve as a pointer for users. Ensure that this is the case by
unconditionally using the value of the respective keys.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It's somewhat confusing that some of the services have a
corresponding foo.service.extra.in and foo.socket.extra.in, some
have just one of the two, and some have neither.
In order to make things more approachable, make sure that both
files exists for each service.
In most cases the extra units are currently unused, so they will
just contain a comment briefly explaining their purpose and
pointing users to meson.build, where they can find more
information. The same comment is also added to the top of
extra units that already have some contents in them for
consistency.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the underlying script is able to merge an arbitrary
number of units into the base template, expose this possibility
in the build system.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It uses custom templates which already hardcode the correct
value.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Starting v18, cloud-hypervisor supports serial and console devices in
parallel. Drop related check based on ch version.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Starting with v28.0 cloud-hypervisor requires the use of "payload" api to pass
kernel, initramfs and cmdline options. Extend ch driver to use the new
api based on ch version.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function virHostCPUGetPhysAddrSize was introduced with commit be1b7d5b18
fails on architectures other than x86 and SuperH. The commit 8417c1394c
fixed the issue only for s390 but the problem is still seen on other
architectures like ppc which does not report Physical address size in their
cpuinfo output.
command:
systemctl restart libvirtd.service
Output :
<snip>
dnsmasq[2377]: read /var/lib/libvirt/dnsmasq/default.addnhosts - 0
addresses
dnsmasq-dhcp[2377]: read /var/lib/libvirt/dnsmasq/default.hostsfile
libvirtd[3163]: libvirt version: 9.8.0
libvirtd[3163]: hostname: xxxxxxxxxx
libvirtd[3163]: internal error: Missing or invalid CPU address size in
/proc/cpuinfo
libvirtd.service: Deactivated successfully.
</snip>
This patch fixes this issue by returning the size=0 for architectures
other than x86 and SuperH.
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
vmDef->fss[i]->src->path may be NULL,
so check is needed before passing it to VIR_DEBUG.
Also removed checking vmDef->fss[i]->src for NULL, since it may not be NULL.
Fixes: 57487085dc ("lxc: don't try to reference NULL when mounting filesystems")
Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, libvirt doesn't send events when devices are attached,
detached or updated. Thus, any services that listen to events are
unaware of the change to persistent config.
Signed-off-by: Fima Shevrin <efim.shevrin@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Considering that at the virSecuritySELinuxSetFilecon() function can only
return 0 or -1 and so does the virSecuritySELinuxFSetFilecon(), the check
for '1' at the end of virSecuritySELinuxSetImageLabelInternal() is
effectively a dead code. Drop it.
Co-developed-by: sdl.qemu <sdl.qemu@linuxtesting.org>
Signed-off-by: Sergey Mironov <mironov@fintech.ru>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While the name itself doesn't matter, this rename is done to prove that
all places using 'nodeformat' were converted to the appropriate
accessors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The persistent bitmaps are stored in the format layer, using 'effective'
bitmap name is the most reasonable approach in this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The blockjob, NBD export and setup of the cookie data all care about the
effective nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The frontend device needs to access the blocks directly so it cares
about the effective nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In most cases the bitmap operations are relevant only on qcow2 images
thus the 'format' layer will be present. Although in certain specific
cases temporary bitmaps can be created on top of other images as well,
thus we use the 'effective' bitmap name in all cases for bitmap
operations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
I case of statistics we're interested in the statistics of the effective
bitmap whatever it happens to be.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The disk backend setup code is concerned only about the effective
nodename. Doing this conversion will also simplify further changes
needed to drop the 'raw' layer in cases when it's not really needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code setting the nodenames needs to use the 'true' nodename of the
format layer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the main -blockdev JSON object setup code to use the new
accessors. In these we use mainly the real 'format' layer node name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the effective nodename for naming the job as we use that one now.
It doesn't matter too much which one we pick, because it's used just for
the name of the job, which we preserve in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Both modified cases in this patch require the effective nodename as they
deal with the data being backed up.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a set of accessors, which return node names based on
semantics. This will allow to us to modify how we setup the backing
chain in cases when e.g. the format driver can be omitted, without
breaking all the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the name itself doesn't matter, this rename is done to prove that
all places using 'nodestorage' were converted to the appropriate
accessors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to keep setting the block threshold on the real storage layer
per semantics of the API. Use the appropriate accessor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In all cases we want to probe stats from the 'storage' layer as we're
interested in the 'threshold' value, which we set there.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new nodename accessors for any storage layer helper object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the code settin up data structures used to attach/detach disks
and SCSI hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the code which assigns the 'storage' layer nodenames for disks.
scsi hostdevs and pflash backend.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to use the 'effective' storage nodename (one which includes the
optional storage slice 'raw' intermediate layer) in the code which
formats the 'format' layer props.
All other cases need the real storage driver nodename as they either
generate the 'storage' layer props, or the storage slice, which refers
to the proper storage backend.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new accessors in the private XML formatters and parsers and the
recovery code.
Specifically in all instances we use the proper (not effective) storage
nodename. In the virStorageSource private data it is what we need to
store. In blockjobs status XML it simply serves us to find the
appropriate 'virStorageSource' struct so using the storage layer node
name is simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'qemuBlockStorageSourceGetEffectiveStorageNodename' in all the JSON
props formatters for setting up a 'blockdev-create' job of a format
layer.
In case of the blockjob name designator we're okay to use just the
storage layer nodename as that serves only to find the appropriate
entry.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The lookup by nodename requires the proper storage nodename which we use
also in status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a set of accessors, which return node names based on
semantics. This will allow to us to modify how we setup the backing
chain in cases when e.g. the format driver can be omitted, without
breaking all the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuBlockStorageSourceGetFormatProps as it formats the properties of
the 'format' driver in qemu. Adjust the comment which was hinting
otherwise.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Restructure the conditions so that we can use virJSONValueObjectAdd with
a clearer logic for backing store control.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the node name of the storage access driver to identify the block job
volumes. This will prepare the blockjob code for the possibility that the
format layer may be missing. Our lookup code can find either of them,
thus we can safely switch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'virt-aa-helper' process gets a XML of the VM it needs to create a
profile for. For a disk type='volume' this XML contained only the
pool and volume name.
The 'virt-aa-helper' needs a local path though for anything it needs to
label. This means that we'd either need to invoke connection to the
storage driver and re-resolve the volume. Alternative which makes more
sense is to pass the proper data in the XML already passed to it via the
new XML formatter and parser flags.
This was indirectly reported upstream in
https://gitlab.com/libvirt/libvirt/-/issues/546
The configuration in the issue above was created by Cockpit on Debian.
Since Cockpit is getting more popular it's more likely that users will
be impacted by this problem.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Re-translating the disk source pools when reconnecting to a VM makes no
sense as the volume might have changed or pool became inactive. The VM
still uses the original volume though. Failing to re-translate the pool
also causes the VM to be killed.
Fix this by storing the original translation in the status XML.
Resolves: https://issues.redhat.com/browse/RHEL-7345
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Re-translating a disk type='volume' definition from a storage pool is
not a good idea in cases when the volume might have changed or we might
not have access to the storage driver.
Specific cases are if a storage pool is not activated on daemon restart,
then re-connecting to a VM fails, or if the virt-aa-helper program tries
to setup labelling for apparmor.
Add a new flag which will preserve the translated data in the
definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a disk definition was already translated re-doing it makes no sense.
Skip the translation if the 'actualtype' is already populated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Register autoptr cleanup function for virStorageSourcePoolDef and
refactor the parser to simplify the logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use proper enum type and refactor the formatter accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When loading a secret value fails, the control jumps over to the
'cleanup' label where explicit call to virSecretDefFree()
happens. This is unnecessary as the corresponding variable can be
declared with g_autoptr() after which all error paths can just
return NULL instantly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When loading virSecret configs, the @path variable holds path to
individual config files. In each iteration it is freed explicitly
using VIR_FREE(). Switch it to g_autofree and remove those
explicit calls.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanup the virBuildPathInternal() function is no
longer used. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our virBuildPath() constructs a path from given arguments.
Exactly like g_build_filename(), except the latter is more
generic as it uses backslashes on Windows. Therefore, replace the
former with the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In my recent commit of v9.8.0-rc1~7 I've introduced validation
wrt other memory devices. And mostly works, except when doing
memory device update ('virsh update-memory-device') because then
@mem is just parsed <memory/> device XML and thus its pointer is
not in the vm->def->mem, yet. Thus my algorithm which skips over
the same entry fails. Fortunately, we require full device XML on
device update and thus we can use device address and aliases to
detect duplicity.
Fixes: 3fd64fb0e2
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While qemu is still reporting the 'device' field in the tray even the
code was not ready for the possibility of it missing. Fix the condition
for clearing 'devAlias' if qemu doesn't report the 'device' field.
Signed-off-by: Sergey Mironov <mironov@fintech.ru>
Comparison "if (ret == -1)" is always false.
This statement was forgotten during switching to g_new0()
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 0275b06a55 ("util: command: use g_new0")
Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We need to skip all disks that have snapshot type other than 'external'.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When used with internal snapshots there is no memory state file so we
have no data to load and decompression is not needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When called from snapshot code we will need to pass snapshot object in
order to make internal snapshots work correctly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When called by snapshot code we will need to use different reason.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function will no longer be used only when restoring VM as it will
be used when reverting snapshot as well so move it to qemu_process
and rename it accordingly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
These new helpers separates the code from the logic used to start new
QEMU process with memory state and will make it easier to move
qemuSaveImageStartProcess() into qemu_process.c file.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
The function is supposed to return the number of items filled into the
array and not zero. Also change the initialization of the "randomness"
to be based on the startCell so that the values are different for each
cell even for separate calls.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 208569b07b6479e0acd05c5a7e1978b0b641e188)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virProcessKillPainfullyDelay() currently almost always returns 1 or -1,
even though the documentation indicates that it should return 0 if the
process was terminated gracefully. But the computation of the return
code is faulty and the only case where it currently returns 0 is when it
is called with the pid of a process that does not exist.
Since no callers ever even distinguish between the 0 and 1 response
codes, simply get rid of the distinction and return 0 for both cases.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Like the Description, these are intended to be displayed to the
user, so it makes sense to have them towards the top of the file
before all the information that systemd will parse to calculate
dependencies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@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>
Currently we only set this for the main sockets, which means
that
$ systemctl stop virtqemud.socket
will make the socket disappear from the filesystem while
$ systemctl stop virtqemud-ro.socket
won't. Get rid of this inconsistency.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This results in all sockets for a service being enabled when a
single one of them is.
The -tcp and -tls sockets are intentionally excluded, because
enabling them should require explicit action on the
administrator's part; moreover, disabling them should not result
in the local sockets being disabled too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
systemd will automatically infer this dependency based on the
socket's Service=foo.service setting.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We have already declared the mirror relationship, so this one
is now redundant.
Moreover, this version was incomplete: it only ever worked for
the monolithic daemon, but the modular daemons for QEMU and Xen
also want the sockets to be active.
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>
Only the main socket is actually necessary for the service to be
usable.
In the past, we've had security issues that could be exploited via
access to the read-only socket, so a security-minded administrator
might consider disabling all optional sockets. This change makes
such a setup possible.
Note that the services will still try to activate all their
sockets on startup, even if they have been disabled. To make sure
that the optional sockets are never started, they will have to be
masked.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is the strongest relationship that can be declared between
two units, and causes the service to be terminated immediately
if its main socket disappears. This is the behavior we want.
Note that we don't do the same for the read-only/admin sockets,
because those are not as critical for the core functionality of
services as the main socket it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We want to make sure that, at any given time, we have either the
modular daemons or the monolithic one running, never both. In
order to achieve that, make every single modular unit conflict
with the corresponding libvirtd unit.
We set both Conflicts=libvirtd.unit and After=libvirtd.unit: this
tells systemd that, whenever virtfood.unit and libvirtd.unit are
part of the same transaction, the former should win out.
Thanks to this, if both the modular daemons and the monolithic
one have been enabled because of outdated automation or a simple
mistake of the administrator, the request to start libvirtd at
boot will be ignored and the result will be a regular modular
deployment.
If the request to start libvirtd is made when the modular daemons
are already running, we have no way to prevent systemd from
complying with that request; however, thanks to the way the
conflict relationship has been declared, they will be shut down
cleanly before libvirtd is started. From the user's point of
view, the transition from modular to monolithic will be
completely transparent: it's basically the same scenario as a
regular package upgrade, just with an extra twist.
Note that, while switching from modular to monolithic at runtime
happens automatically, going back requires manual intervention,
i.e. starting all the necessary sockets one by one. That's okay:
the goal here is to prevent misconfiguration and force of habit
to accidentally disrupt a working setup, not to encourage the
scenario. In a correctly configured and managed host, it should
never occur.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Up until now the files have been used as template for most
services, but now that those have been converted to common
templates we can drop parametrization and make it clear that
these files are for libvirtd only.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the migration to common templates has been completed,
we no longer need these.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
All services are still listing their input files explicitly, so
no changes to the output files will occur yet.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We already use templating to generate sockets, which are all
based off libvirtd's. Push the idea further, and extend it to
cover services as well.
This is more challenging, as the various modular daemons each have
their own needs in terms of what system services needs to be
available before they can be started, which other components of
libvirt they depend on, and so on.
In order to make this sort of per-service tweaks possible, we
introduce a Python script that can merge two systemd units
together. The script is aware of the semantics of systemd's unit
definition format, so it can intelligently merge sections
together.
This generic systemd unit merging mechanism will also supersede
the extremely ad-hoc @deps@ variable, which is currently used in
a single scenario.
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>
These will be useful during the upcoming migration to common
templates for systemd units and will be dropped as soon as all
services have been converted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It is currently considered required, but we're soon going to
provide a default that will be suitable for most services.
Since all services currently provide a value explicitly, we
can implement a default without breaking anything.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
They're similar to the existing socket_in/socket_out variables
and will make future changes nicer.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The idea behind these is to prevent running both modular daemons
and monolithic daemon at the same time. We will implement a more
effective solution for that shortly.
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: 0108deb944
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 socket paths can be derived trivially from
the name of the daemon: for virtqemud, for example, they will be
/run/libvirt/virtqemud-sock
/run/libvirt/virtqemud-sock-ro
/run/libvirt/virtqemud-admin-sock
libvirtd and virtproxyd are the exceptions, since their socket
paths will be
/run/libvirt/libvirt-sock
/run/libvirt/libvirt-sock-ro
/run/libvirt/libvirt-admin-sock
So we still need to be able to provide a custom @sockprefix@ in
those cases, but in the most common scenario we can do away with
the requirement by introducing a sensible default.
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 decision is based only on whether Polkit support is enabled,
so there's no need to go through it again for every single
service.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The meaning of the _def suffix might not be immediately obvious,
especially since it's also used to refer to the output of the
meson-gen-def.py script elsewhere in the same file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The information is not used anywhere right now, but the
documentation for virt_daemon_units claims it's mandatory.
We also intend to actually start using it later on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This tells systemd that the services in question support the
native socket activation protocol.
virtlogd and virtlockd, just like all the other daemons, implement
the necessary handshake.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
While systemd will automatically match foo.socket with foo.service
based on their names, it's nicer to connect the two explicitly.
This is what we do for all services, with virtlogd and virtlockd
being the only exceptions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This annotation being missing resulted in virtlogd and virtlockd
being marked as "indirect" services, i.e. services that cannot
be started directly but have to be socket activated instead.
While this is our preferred configuration, we shouldn't prevent
the admin to start them at boot if they want to.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When libvirtd, virtlog and virtlockd are enabled, we want their
admin sockets to be enabled for socket activation as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 93af79fb removed a cleanup label in favor of returning error
values directly in certain cases. But the final return value was changed
from -1 to 0. If we get to the end of the function, that means that
we've waited for the process to exit but it still exists. So we should
return -1. The error message was still being set correctly, but we were
returning a success status (0).
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In a modular daemon configuration, virtxend does not support the
virNetwork* APIs. It should open a connection to virtnetworkd when
using those APIs, but currently always opens a connection to
"xen:///system". Switch to using virGetConnectNetwork to obtain a
valid connection instead of using the hardcoded URI.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewing the code I found that return value of function
udev_device_get_sysattr_value() is dereferenced without a check.
udev_device_get_sysattr_value() may return NULL by number of reasons.
v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE()
v3: More checks added, to skip earlier. More verbose VIR_DEBUG.
Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
As of v9.4.0-rc2~5 it is possible to specify guest address where
a virtio-mem/virtio-pmem memory device is mapped to. What that
commit forgot to introduce was a check for overlaps.
And yes, this is technically an O(n^2) algorithm, as
virDomainMemoryDefValidate() is called over each memory device
and after this, virDomainMemoryDefValidate() also iterates over
each memory device. But given there's usually only a handful of
such devices, and this runs only when parsing domain XML I guess
code readability wins over some less obvious solution.
Resolves: https://issues.redhat.com/browse/RHEL-4452
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of virDomainMemoryDefValidate() there's a check that
address where a virtio-mem memory device is mapped to is a
multiple of its block size. But this check is off by a couple of
bits, because the memory address is in bytes while the block size
is in kibibytes. Therefore, when checking whether address is a
multiple of the block size, the latter has to be multiplied by a
factor of 1024.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU mandates the VIRTIO_PMEM address is aligned to a pagesize.
This is a very reasonable requirement. So much so, that it
deserves to be in hypervisor agnostic validation code
(virDomainMemoryDefValidate()). Not that any other hypervisor
would support VIRTIO_PMEM yet. But even if they did, this would
surely be still valid.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
Our test driver lacks implementation for
virConnectGetDomainCapabilities(). Provide one, though a trivial
one. Mostly so that something else than VIR_ERR_NO_SUPPORT error
is returned.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
