conf: rename and improve virDomainDefFindAudioForSound
The virDomainDefFindAudioForSound only takes a virDomainSoundDefPtr as
its arg, but we want to use the same functionality for VNC graphics.
In addition if audio ID is zero, then we want to return the first
available audio backend.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
conf: add coverage for all QEMU audio backend types
The current <audio> element only allows an "OSS" audio backend, as this
is all that BHyve needed. This is now extended to cover most QEMU audio
backends. These backends all have a variety of attributes they support,
but this initial impl does the bare minimum, relying on built-in
defaults for everything. The only QEMU backend omitted is "dsound" since
the libvirt QEMU driver is not built on Windows platforms.
The SDL audio driver names are based on the SDL 2.0 drivers. It is not
intended to support SDL 1.2 drivers.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To prepare for the introduction for more backend specific audio options,
move the OSS options into a dedicated struct and introduce separate
helper methods for parse/format/free.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
conf: add helper to test for sound device codec support
The check for ICH6 || ICH9 is repeated in many places in the code. The
new virDomainSoundModelSupportsCodecs() method provides a helper to
standardize this check.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
conf: don't force existance of audio child elements
The attributes on the elements are optional, so we should not force the
elements themselves to be present, especially since we omit them when
formating the XML thus breaking round-tripping.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Aleksei Zakharov [Fri, 19 Feb 2021 20:08:34 +0000 (23:08 +0300)]
qemu: add per-vcpu delay stats
This patch adds delay time (steal time inside guest) to libvirt
domain per-vcpu stats. Delay time is an important performance metric.
It is a consequence of the overloaded CPU. Knowledge of the delay
time of a virtual machine helps to understand if it is affected and
estimate the impact.
As a result, it is possible to react exactly when needed and
rebalance the load between hosts. This is used by cloud providers
to provide quality of service, especially when the CPU is
oversubscribed.
It's more convenient to work with this metric in a context of a
libvirt domain. Any monitoring software may use this information.
Signed-off-by: Aleksei Zakharov <zaharov@selectel.ru> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The preprocessor macro we use to check whether we're on Linux
has not been spelled properly, and so we will always report the
error message intended for other platforms.
Fixes: 879bcee08ce0f91f556fddfe452c3fbed5318468 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow the directory to be chosen at installation time, to support local
conventions e.g. versioning.
Signed-off-by: Chris Mayo <aklhfex@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virnetdevbandwidth: Don't generate burst outside of boundaries
When generating TC rules for domain's outbound traffic, Libvirt
will use the 'average' as the default for 'burst' - it's been
this way since the feature introduction in v0.9.4-rc1~22. The
reason is that 'average' considers 'burst' for policing. However,
when parsing its command line TC uses an unsigned int (with
overflow detection) to store the 'burst' size. This means, that
the upper limit for the value is UINT_MAX, well UINT_MAX / 1024
because we are putting the value in KiB onto the command line.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912210 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Up until now we've implicitly relied on the fact that failures
reported from this function were simply ignored, but that's
about to change and so we need a proper mock.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This behavior reflects the needs of the QEMU driver and has no
place in a generic module such as virProcess.
Thanks to the changes made with the previous commit, it is now
safe to remove these checks and make all virProcessSetMax*()
functions finally behave the same way.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemu: Set limits only when explicitly asked to do so
The current code is written under the assumption that, for all
limits except the core size, asking for the limit to be set to
zero is a no-op, and so the operation is performed
unconditionally.
While this is the behavior we want for the QEMU driver, the
virCommand and virProcess facilities are generic, and should not
implement this kind of policy: asking for a limit to be set to
zero should result in that limit being set to zero every single
time.
Add some checks in the QEMU driver, effectively moving the
policy where it belongs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuProcessLaunch() is the correct place to set process limits,
and in fact is where we were dealing with almost all of them,
but the memory locking limit was handled in
qemuBuildCommandLine() instead for some reason.
The code is rewritten so that the desired limit is calculated
and applied in separated steps, which will help with further
changes, but this doesn't alter the behavior.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 23 Feb 2021 17:52:26 +0000 (18:52 +0100)]
util: Introduce virProcess{Get,Set}Limit()
These functions abstract part of the existing logic, which is
the same in all virProcessSetMax*() functions, and changes it
so that which underlying syscall is used depends on their
availability rather than on the context in which they are
called: since prlimit() and {g,s}etrlimit() have slightly
different requirements, using the same one every single time
should make for a more consistent experience.
As part of the change, we also remove the special case for
passing zero to virProcessSetMax*() functions: we have removed
all callers that depended on that functionality in the previous
commit, so this is now safe to do and makes the semantics
simpler.
This commit is better viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, the functions accept either an explicit pid or zero,
in which case the current process should be modified: the latter
might sound like a convenient little feature, but in reality
obtaining the pid of the current process is a single additional
function call away, so it hardly makes a difference.
Removing the few cases in which we're passing zero will allow us
to simplify and improve the functions later.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The <graphics type="vnc" .... powerControl="yes"/> option instructs the
VNC server to enable an extension that lets the client perform a
graceful shutdown, reboot and hard reset.
This is enabled by default since it cannot be assumed that the VNC
client user has administrator rights over the guest OS. In the case
where the VNC user is a guest administrator though, it is reasonable
to allow direct power control host side too.
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virDevMapperGetTargetsImpl: Use correct length when copying into dm.name
For reasons unknown, when rewriting this code and dropping
libdevmapper I've mistakenly used incorrect length of dm.name. In
linux/dm-ioctl.h the dm_ioctl struct is defined as follows:
After decryption, this results in the following size: 3241737483.
Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Fri, 5 Mar 2021 09:38:49 +0000 (10:38 +0100)]
virFirewallApply: Fix possible NULL dereference on error
Commit bbc25f0d03d443efd35381463efc81b01cb6ae96 juggled around some
error reporting. Unfortunately virFirewallApply tries to report the
errno stored in the firewall object and we'd try to do that when the
firewall object is NULL too. Report EINVAL if 'firewall' is NULL.
Found by Coverity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Thu, 4 Mar 2021 17:09:42 +0000 (18:09 +0100)]
virBufferAdd: Ensure that the buffer is initialized also when len == 0
There's an optimization in virBufferAdd which returns early when the
length of the added string is 0 (given that auto-indent is disabled).
The optimization causes inconsistent behaviour between these two cases:
virBufferAdd(buf, "", 0); // this doesn't initialize the buffer
and
virBufferAdd(buf, "", -1); //this initializes the buffer
Since using an empty string is used to prime the buffer to an empty
string it can be confusing. Remove the optimization.
This fixes such a wrong initialization done in x86FeatureNames.
Note that our code in many places expects that if no virBuffer APIs are
used on a buffer object, then NULL should be retured, so we can't always
prime the buffer to an empty string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 3 Mar 2021 09:20:00 +0000 (10:20 +0100)]
virIndexToDiskName: Make 'idx' unsigned and remove check
We can remove the check that 'idx' is negative by forcing callers to
pass unsigned numbers, which they do already or have a check that 'idx'
is positive.
This in turn allows us to remove most return value NULL checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 1 Mar 2021 10:04:54 +0000 (11:04 +0100)]
virCommandSetSendBuffer: Provide saner semantics
The function is used to automatically feed a buffer into a pipe which
can be used by the command to read contents of the buffer.
Rather than passing in a pipe, let's create the pipe inside
virCommandSetSendBuffer and directly associate the reader end with the
command. This way the ownership of both ends of the pipe will end up
with the virCommand right away reducing the need of cleanup in callers.
The returned value then can be used just to format the appropriate
arguments without worrying about cleanup or failure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 1 Mar 2021 09:25:18 +0000 (10:25 +0100)]
virPipeImpl: Don't overwrite error
If WITH_PIPE2 is not defined we attempt to set the pipe to nonblocking
operation after they are created. We errorneously rewrote the existing
error message on failure to do so or even reported an error if quiet
mode was requested.
Fixes: ab36f729470c313b9d5b7debdbeac441f7780dec Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 25 Feb 2021 16:52:47 +0000 (17:52 +0100)]
qemuProcessReportLogError: Don't mark "%s: %s" as translatable
The function is constructing an error message from a prefix and the
contents of the qemu log file. Marking just two string modifiers as
translatable is pointless and will certainly confuse translators.
Remove the marking and add a comment which bypasses the
sc_libvirt_unmarked_diagnostics check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 23 Feb 2021 14:27:07 +0000 (15:27 +0100)]
qemuProcessReportLogError: Remove unnecessary math for max error message
Now that error message formatting doesn't use fixed size buffers we can
drop the math for calculating the maximum chunk of log to report in the
error message and use a round number. This also makes it obvious that
the chosen number is arbitrary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 23 Feb 2021 14:21:13 +0000 (15:21 +0100)]
util: virprocess: Use local maximum error message size
Use of VIR_ERROR_MAX_LENGTH is actually misleading to the readers
because it implies that the strings in virError are 1024 bytes at most.
That isn't true at least for the 'message' field as it's constructed
from concatenating the detail string which (was) max 1024 bytes with
the string variant of the error code without limiting to 1024.
Use a local copy for declaring the struct for error transport with a
comment so that's obvious that it's a local decision to use 1k buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 25 Feb 2021 16:20:12 +0000 (17:20 +0100)]
util: virerror: Avoid a copy of the error messages
Some error message reporting functions already have allocated buffers
which were used to format the error message, so copying the strings is
redundant.
Extract the internals from 'virRaiseErrorFull' to
'virRaiseErrorInternal' which takes allocated strings as arguments and
steals them, so that callers can reuse the buffers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 23 Feb 2021 14:38:02 +0000 (15:38 +0100)]
util: virerror: Don't use stack'd buffers in error report helpers
This was (probably) a relict from times when we cared about OOM
conditions and the possibility to report the error. Nowadays it doesn't
make sense as virRaiseErrorFull will do an allocated copy of the strings
and also concatenate the error message prefix with the detail which
doesn't guarantee that the result will be less than 1024 chars.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 2 Mar 2021 10:00:23 +0000 (11:00 +0100)]
util: virstring: Always copy string in virStrcpy
15 out of 72 invocations of virStrcpy(Static) ignore the return value as
it's either impossible to fail or in certain cases a truncated copy is
still good enough. Unfortunately virStrcpy doesn't copy anything in
such case as the checks are done first.
Fix this by using g_strlcpy for the implementation and removing
G_GNUC_WARN_UNUSED_RESULT from the function so that callers can decide
when it's okay.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Surprisingly, we never documented the relationship between
connection URI and the location of qemu.conf. Users might wonder
what qemu.conf is loaded when they are connecting to the session
daemon or embed URI. And what to do if the file doesn't exist for
the URI they're using.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Jim Fehlig [Mon, 1 Mar 2021 22:57:05 +0000 (15:57 -0700)]
build: Require glib >= 2.56
There are a few uses of g_autoslist in the qemu driver and likely more
will come throughout the codebase in the future. g_autoslist first
appeared in glib 2.56, so bump the minimum version
Bumping the minimum version is an opportune time to update the list of
minimum glib versions found on the distros targeted by libvirt's
platform support policy
FreeBSD 13.x and newer ship BSD grep which apparently has some
performance issues causing certain syntax check tests to run longer than
the default 30 seconds timeout used by meson.
However, GNU grep is still available through the textproc/gnugrep port,
so require it on FreeBSD if /usr/bin/grep is a BSD grep to make checks
pass in a reasonable time.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tim Wiederhake [Thu, 4 Mar 2021 08:35:31 +0000 (09:35 +0100)]
domain_conf: Fix check for hyperv stimer
VIR_DOMAIN_HYPERV_STIMER happens to have the same numerical value as
VIR_DOMAIN_FEATURE_HYPERV, resulting in the if-block to always being
executed when a "<hyperv>" tag is found, whether or not it actually
contained a "<stimer>" tag. This had no ill effects, as virXPathNodeSet()
would simply return 0 if that tag does not exist.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Wed, 3 Mar 2021 13:26:53 +0000 (14:26 +0100)]
cgroup: use virCgroupSetCpuShares instead of virCgroupSetupCpuShares
Now that we enforce the cpu.shares range kernel will no longer silently
change the value that libvirt configures so there is no need to read
the value back to get the actual configuration.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Wed, 3 Mar 2021 13:10:15 +0000 (14:10 +0100)]
vircgroup: enforce range limit for cpu.shares
Before the conversion to using systemd DBus API to set the cpu.shares
there was some magic conversion done by kernel which was documented in
virsh manpage as well. Now systemd errors out if the value is out of
range.
Since we enforce the range for other cpu cgroup attributes 'quota' and
'period' it makes sense to do the same for 'shares' as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Mon, 1 Mar 2021 22:41:44 +0000 (15:41 -0700)]
libxl: Fix node device detach when driver unspecified
Commit 887dd0d331 caused a small regression in NodeDeviceDetach in the libxl
driver when the 'driver' parameter is not specified. E.g.
# virsh nodedev-detach pci_0000_0a_10_0
error: Failed to detach device pci_0000_0a_10_0
error: An error occurred, but the cause is unknown
If the driver name is not specified, NULL is passed to
virDomainDriverNodeDeviceDetachFlags, in which case virPCIDeviceSetStubDriver
is never called to set the stub to pciback. Fix it by setting the driver to
"xen" if it is not specified when invoking NodeDeviceDetach.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 24 Feb 2021 10:34:00 +0000 (11:34 +0100)]
util: virerror: Remove virReportOOMError
Trying to report an OOM error is pointless since our infrastructure to
report error needs to allocate memory to report the error.
In addition our code mistakenly reported OOM errors even in cases where
a function could fail for another reason, which would make issues harder
to debug.
Remove the virReportOOMError and backend so that programmers are forced
to think about what can happen. In case when there's another failure
possible a specific error should be reported and otherwise a direct
abort() is better since the logger would abort on g_new anyways.
This patch also removes the syntas-check which forces use of
virReportOOMError instead of using VIR_ERR_NO_MEMORY with other
functions. This allows possible future use when we'd end up in a
situation where trying to recover from an OOM would make sense, such as
when attempting to allocate a massive buffer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Peter Krempa [Wed, 24 Feb 2021 09:22:52 +0000 (10:22 +0100)]
storage: Don't report OOM error on failure of glfs_new
OOM isn't the only failure glfs_new can encounter. Report an error which
might give more insight. libgfapi seems to be setting errno but
reporting a system error migt be misleading.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Peter Krempa [Wed, 24 Feb 2021 09:13:27 +0000 (10:13 +0100)]
util: json: Report non-OOM error on yajl failure
The yajl library returns a wide range of error codes so reporting OOM on
any failure is wrong. In case the error was really based by memory issue
the error reporting will probably cause an abort anyways. Change the
error message so that we know that it happened in JSON at least.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
projects / thirdparty / libvirt.git / log
