The same several lines were repeated, once in a loop iterating through
all log targets, and again to output to stderr when there are no log
targets specified. This just moves those lines into a helper function,
making it easier and less error prone to add additional info to the
banner that is logged each time a daemon starts logging.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For some reason there were some uses of this struct where "struct
_virLogMetadata" was used instead of just using the typedef
"virLogMetadata" (they are both defined in the same file -
virlog.h). Possibly at one point the struct was in virlog.c and
outsiders could only see it as an opaque object, but even if that was
the case, there are already cases of the typedef being used outside of
virlog.c, and constinuing to use "struct _virLogMetadata" in some
places both looks too much K&R 1st edition and might incorrectly imply
to someone that there *is* data abstraction/hiding going on when there
really isn't. So let's just always use plain virLogMetadata.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The message previously just said "Could not create log directory", but
didn't provide the name of the directory, which could be helpful in
determine why the failure occured.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemuDomainOpenFile() only acts on the 'dac' driver, where 'label'
and 'imagelabel' are always identical (see virSecurityDACGenLabel()).
So there's nothing TODO here
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Every caller passes this in, we can depend on it being non-NULL.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
When fs->sock is set, virtiofs is externally managed, and most
of qemu_virtiofs.c should do nothing.
Some qemu_virtiofs.c functions handle this case, but some require the
caller to guard against it.
Standardize on making this the responsibility of qemu_virtiofs.c
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
When support for vhostuser devices was added, it was just
blanket-prevented from making any changes to a live device with
update-device. This is problematic because the link state of a network
device is modified with update-device. Most all of the parameters of a
vhostuser network device are individually checked within
qemuDomainChangeNet() anyway, so we don't need to just do a BRS (Big
Red Switch) forbidding of any change. We do need to check for
modifications to the socket parameters (path, type, reconnect) though,
since those are vhostuser-specific (we're not already checking for
them elsewhere) and they can't be changed on a live interface.
Resolves https://redhat.atlassian.net/browse/RHEL-152533
Resolves: https://bugs.passt.top/show_bug.cgi?id=198
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Somehow we've never checked for this, and nobody has ever tested for
it or complained about it, but certainly attempting to change a user
or vhostuser network device to/from a passt backend wouldn't work. Now
we check for it and log an error.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We would leak ret if the remaining two virJSONValueObjectAdd() calls
would fail but it will not happen with current code.
Fixes: c7570bbef8
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Refactor the configuration data setup to pass the localstatedir variable
directly during initialization.
No functional change intended.
Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The monolithic libvirtd.service currently has a dependency on
virt-secret-init-encryption.service. This causes libvirtd to fail
to start on systems where the secret driver is not installed or
enabled, as systemd cannot satisfy the Requires= unit or the
LoadCredentialEncrypted= path. See below,
Requires=virt-secret-init-encryption.service
LoadCredentialEncrypted=secrets-encryption-key:@localstatedir@/lib/libvirt/secrets/secrets-encryption-key
This patch decouples the secrets encryption logic from the main
libvirtd service file. It is moved into a new systemd drop-in
(10-secret.conf) which is only installed when libvirt is built
with secret driver support. The override snippet is added to the
daemon-driver-secret package.
Fixes: 97758bc9a0
Signed-off-by: Arun Menon <armenon@redhat.com>
Fix-Suggested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The hyperv hypervisor also has guest agent, in fact multiple ones
[1][2]. The first one, KVP, is for storing Key-Value Pairs and in
fact it's already used by our hyperv driver when querying
domifaddr (see v12.1.0-rc1~148 for more info). Anyhow, the KVP
service is capable of more, it can provide guest OS info, guest
FQDN and others. These informations are exposed via
GuestIntrinsicExchangeItems member of the
Msvm_KvpExchangeComponent struct [3]. You may have noticed the
member is an array of strings, well those strings are in fact XML
documents. For instance:
<INSTANCE CLASSNAME="Msvm_KvpExchangeDataItem">
<PROPERTY NAME="Caption" TYPE="string"/>
<PROPERTY NAME="Data" TYPE="string">
<VALUE>6.12.61-1-lts</VALUE>
</PROPERTY>
<PROPERTY NAME="Description" TYPE="string"/>
<PROPERTY NAME="ElementName" TYPE="string"/>
<PROPERTY NAME="InstanceID" TYPE="string"/>
<PROPERTY NAME="Name" TYPE="string">
<VALUE>OSBuildNumber</VALUE>
</PROPERTY>
<PROPERTY NAME="Source" TYPE="uint16">
<VALUE>2</VALUE>
</PROPERTY>
</INSTANCE>
This is a bit messy to work with, because it's not like in QEMU's
world where each type of guest info (virDomainGuestInfoTypes)
corresponds 1:1 to a guest agent command. Hence the lookupTable
in hypervGetServicesProcessOne().
NB, the original jira issue asks for exposing plain fact whether
KVP daemon is running inside the guest and this commit implements
seemingly different feature. Well, thing is, in case of QEMU
there's a domain XML part where guest agent is configured and
where we expose whether there's somebody listening inside the
guest. But in case of hyperv there's no <channel/> to be
configured as communication with KVP daemon happens through
vmbus [4]. Users are advised to call the virDomainGetGuestInfo()
API with non-zero 'types' argument and if they get an error with
VIR_ERR_AGENT_UNRESPONSIVE code then the KVP daemon is not
running.
1: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/hv/hv_kvp_daemon.c
2: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/hv/hv_vss_daemon.c
3: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-kvpexchangecomponent
4: https://docs.kernel.org/virt/hyperv/vmbus.html
Resolves: https://redhat.atlassian.net/browse/RHEL-147661
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Ever since of commit v1.2.13-rc1~66 the model attribute of a
<seclabel/> is validated against secdriver names enabled. In
nearly all cases this is something users want so that domain XML
does not claim to set seclabels of a model that's not enabled.
However, consider the following seclabel:
<seclabel type='none' model='selinux'/>
It tells us to not bother setting selinux labels on given domain.
A mgmt app might format this into domain XML if it sees selinux
is disabled on the host. But if that's the case, selinux driver
is not loaded and this virSecurityManagerCheckModel() doesn't
find it and reports an error.
Well, the error doesn't need to be reported as we will just
ignore selinux as each driver callback checks if relabel is false
(which it is for type='none'). This is true for other secdrivers
too.
Resolves: https://redhat.atlassian.net/browse/RHEL-156689
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Let's use automatic memory freeing inside of
virSecurityManagerCheckModel() as it will simplify future
commits.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Prior to v7.10.0-rc1~26 seclabels defaulted to
VIR_DOMAIN_SECLABEL_DYNAMIC (type='dynamic'). But after switching
the parser to virXMLPropEnum() the type is overwritten to
VIR_DOMAIN_SECLABEL_DEFAULT because the first thing that the
helper function does is to set variable that holds the result to
zero. Switch to virXMLPropEnumDefault() to restore the previous
behavior.
Fixes: f7ff8556ad
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In case of a host that has a large number of cpus offline the count of
host cpus and the last bit set in the virHostCPUGetOnlineBitmap might
diverge significantly. This can e.g. be the case when disabling smt via
/sys/devices/system/cpu/smt/control.
On the host this looks like:
```
$ cat /sys/devices/system/cpu/present
0-255
$ cat /sys/devices/system/cpu/online
0-127
```
However in this case virBitmapToData previously only allocated 16 bytes
for the output bitmap. This is becase the last set bit is on the 15th
byte.
Users of virHostCPUGetMap however rely on the "cpumap" containing enough
space for all existing cpus (so they would expect 32 bytes in this case).
E.g. cmdNodeCpuMap relies on this for its output. It will then actually
read 32 bytes from the start of the "cpumap" address where in this case
the last 16 of these bytes are uninitialized.
This manifests itself in flapping outputs of "virsh nodecpumap --pretty" like:
```
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,202
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,197
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,196-197
```
This in turn potentially causes users of this data to report wrong cpu
counts.
Note that this only seems to happen with at least 256 physical cpus
where at least 128 are offline.
We fix this by preallocating the expected bitmap size.
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit goes through all virhostcpudata subdirectories and checks
that `virHostCPUGetMap()` returns valid data for each one of them.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are some features/improvements/bug fixes I've either
contributed or reviewed/merged. Document them for upcoming
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If hotplugging host device that needs to add iommufd object fails we
need to remove the iommufd object as well. We also need to update the
iommufdState otherwise hotplugging any host device with iommufd will
fail for the same VM.
Fixes: 5d16bef1a6
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On systems with selinux enabled starting a VM with managed host device
using IOMMUFD backend can run into race-condition where both libvirt and
udev are setting selinux label on /dev/vfio/devices/vfioX device. If
udev is the last one to set selinux label starting VM fails with:
error: internal error: QEMU unexpectedly closed the monitor (vm='test'): 2026-03-26T15:47:36.620422Z qemu-kvm: -device {"driver":"vfio-pci","id":"hostdev0","iommufd":"iommufd0","fd":"20","bus":"pci.7","addr":"0x0"}: vfio hostdev0: Failed to add fd 20 to KVM VFIO device: Invalid argument
We need to wait for udev to finish processing all events.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Document both the behaviour if requested length isn't enough to read the
file as well as the semantics of NUL-termination of the buffer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use 'virFileReadAllQuiet' since the function doesn't want to report
errors on other code paths.
The function also assumed that the file which it reads always 7 bytes
isn't true at least in the test suite. This didn't cause a problem
because the test data had strings 6 bytes long so it didn't cause a
write beyond the end of the buffer.
Clear the newline by using strchrnul instead to find it rather than
assuming where it is.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The buffer is already terminated. Luckily the last character in the
buffer was a newline so no information was mangled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The buffer returned from 'virFileReadAll' is NUL terminated no need to
do it explicitly or to shorten it since this is now also done.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The returned class string from the kernel isn't guaranteed to be always
9 bytes, thus the write to the buffer could happen beyond the guaranteed
length.
Since 'virFileReadAll' already NUL-terminates the buffer just delete
the redundant overwrite.
This fixes an invalid write beyond the end of the buffer happening since
458c6a2810 where the returned buffer is shortened.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The bufer resizing algorithm resizes the buffer in increments of the
value of the 'BUFSIZ' macro. On linux it's currently 8k. In case when
the caller wants to retain the buffer for long time with little data
read this creates massive overhead.
Realloc the buffer to actual size before returning it to the user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
==2413993== 8,193 bytes in 1 blocks are definitely lost in loss record 4,201 of 4,225
==2413993== at 0x48FC897: realloc (vg_replace_malloc.c:1804)
==2413993== by 0x4ED0B0A: g_realloc (in /usr/lib64/libglib-2.0.so.0.8600.4)
==2413993== by 0x494112B: virReallocN (viralloc.c:52)
==2413993== by 0x49684F6: saferead_lim (virfile.c:1511)
==2413993== by 0x4968C8B: virFileReadLimFD (virfile.c:1587)
==2413993== by 0x4968D2C: virFileReadAll (virfile.c:1613)
==2413993== by 0xC2CC488: virGetSecretsEncryptionKey (secret_config.c:93)
==2413993== by 0xC2CC488: virSecretDaemonConfigNew (secret_config.c:162)
==2413993== by 0xC2CADFD: secretStateInitialize (secret_driver.c:533)
==2413993== by 0x4BABC76: virStateInitialize (libvirt.c:667)
==2413993== by 0x40148D4: daemonRunStateInit (remote_daemon.c:583)
==2413993== by 0x49BB8A5: virThreadHelper (virthread.c:256)
==2413993== by 0x52DD463: start_thread (in /usr/lib64/libc.so.6)
Fixes: 1c86748cf1
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
==2413993== 23 (16 direct, 7 indirect) bytes in 1 blocks are definitely lost in loss record 1,238 of 4,225
==2413993== at 0x48FC683: calloc (vg_replace_malloc.c:1678)
==2413993== by 0x4ED0A51: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.8600.4)
==2413993== by 0x49A62CF: virPortAllocatorRangeNew (virportallocator.c:104)
==2413993== by 0xEA35F27: qemuStateInitialize (qemu_driver.c:723)
==2413993== by 0x4BABC76: virStateInitialize (libvirt.c:667)
==2413993== by 0x40148D4: daemonRunStateInit (remote_daemon.c:583)
==2413993== by 0x49BB8A5: virThreadHelper (virthread.c:256)
==2413993== by 0x52DD463: start_thread (in /usr/lib64/libc.so.6)
==2413993== by 0x5360413: clone (in /usr/lib64/libc.so.6)
Fixes: 36cf50f85b
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Laine Stump
Cole Robinson
Pavel Hrdina
Arun Menon
Michal Privoznik
Martin Kletzander
Felix Huettner
Jiri Denemark
Jonathon Jongsma
Peter Krempa
Pavel Borecki
Remus-Gabriel Chelu
김인수
jianqing yan
Fco. Javier F. Serrador
Nicolás Gal
Weblate