Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
secdrivers: Rename @stdin_path argument of virSecurityDomainSetAllLabel()
The argument (if not NULL) points to the file the domain is
restoring from. On QEMU command line this used to be '-incoming
$path', but we've switched to passing FD ages ago and thus this
argument is used only in AppArmor (which loads the profile on
domain start). Anyway, the argument does not refer to stdin,
rename it to 'incomingPath' then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Sat, 27 Jun 2020 04:28:17 +0000 (06:28 +0200)]
qemu: Use qemuSecuritySetSavedStateLabel() to label restore path
Currently, when restoring from a domain the path that the domain
restores from is labelled under qemuSecuritySetAllLabel() (and after
v6.3.0-rc1~108 even outside transactions). While this grants QEMU
the access, it has a flaw, because once the domain is restored, up
and running then qemuSecurityDomainRestorePathLabel() is called,
which is not real counterpart. In case of DAC driver the
SetAllLabel() does nothing with the restore path but
RestorePathLabel() does - it chown()-s the file back and since there
is no original label remembered, the file is chown()-ed to
root:root. While the apparent solution is to have DAC driver set the
label (and thus remember the original one) in SetAllLabel(), we can
do better.
Turns out, we are opening the file ourselves (because it may live on
a root squashed NFS) and then are just passing the FD to QEMU. But
this means, that we don't have to chown() the file at all, we need
to set SELinux labels and/or add the path to AppArmor profile.
And since we want to restore labels right after QEMU is done loading
the migration stream (we don't want to wait until
qemuSecurityRestoreAllLabel()), the best way to approach this is to
have separate APIs for labelling and restoring label on the restore
file.
I will investigate whether AppArmor can use the SavedStateLabel()
API instead of passing the restore path to SetAllLabel().
These APIs don't use namespaces because the
virSecurityManagerSetSavedStateLabel() runs
when the namespace doesn't exist yet and thus
the virSecurityManagerRestoreSavedStateLabel()
has to run without namespace too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
These APIs were removed/renamed in v6.5.0-rc1~142 and v6.5.0-rc1~141
because they deemed unused. And if it wasn't for the RFE [1] things
would stay that way.
The RFE asks for us to not change DAC ownership on the file a domain is
restoring from. We have been doing that for ages (if not forever),
nevertheless it's annoying because if the restore file is on an NFS
remembering owner won't help - NFS doesn't support XATTRs yet. But more
importantly, there is no need for us to chown() the file because when
restoring the domain the file is opened and the FD is then passed to
QEMU. Therefore, we really need only to set SELinux and AppArmor.
The difference to the original code is that secdrivers are now
not required to provide dummy implementation to avoid
virReportUnsupportedError(). The callback is run if it exists, if
it doesn't zero is returned without any error.
When locking files for metadata change, we open() them for R/W
access. The write access is needed because we want to acquire
exclusive (write) lock (to mutually exclude with other daemons
trying to modify XATTRs on the same file). Anyway, the open()
might fail if the file lives on a RO filesystem. Well, if that's
the case, ignore the error and continue with the next file on the
list. We won't change any seclabel on the file anyway - there is
nothing to remember then.
Reported-by: Olaf Hering <olaf@aepfle.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Wed, 27 May 2020 11:37:54 +0000 (13:37 +0200)]
tests: commandhelper: change how we detect if running as daemon
The old code works correctly with make and running directly from shell
but it failed with Meson test suite where session ID and process group
are the same in both cases.
What changes in both cases is parent process ID so use that instead of
session ID.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Wed, 24 Jun 2020 09:25:58 +0000 (11:25 +0200)]
src: util: Makefile: drop undefined OPENPTY_LIBS
Commit <f650e86703847af544762d02f79c70131ff7fbab> added check for
openpty function from util library using AC_CHECK_LIB(). However, that
macro doesn't define OPENPTY_LIBS, it only defines WITH_LIBUTIL and
prepends -lutil into LIBS for the whole project.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Mon, 4 May 2020 15:28:54 +0000 (17:28 +0200)]
src: remove unnecessary -I$(srcdir)/secret include
Commit <894556ca813ad3c4ebb01083b7971d73b4f53c8b> moved function
virSecretGetSecretString out of secret directory but forgot to update
CFLAGS in places where the include is no longer needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Mon, 29 Jun 2020 15:36:45 +0000 (17:36 +0200)]
po: change the format of POTFILES.in
There is no need to provide relative paths to the current directory if
we provide search paths using --directory option for xgettext.
In addition it will make libvirt.pot file look cleaner as it will not
contain relative paths to current directory. It improves the situation
for developers which are using different build path as that would
change the relative path in libvirt.pot as well. After this patch
it will not happen anymore.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Tue, 23 Jun 2020 22:15:15 +0000 (00:15 +0200)]
m4: virt-sanlock: drop check for sanlock_killpath()
Function sanlock_killpath() was introduced in 2.4 version and had
modified one of the arguments from `char *` into `const char *` in
version 2.7. All of this is available in all supported OSes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Tue, 23 Jun 2020 22:05:57 +0000 (00:05 +0200)]
m4: virt-sanlock: use pkg-config to find libsanlock_client
The last distribution supported by libvirt and lacking pkg-config file
for libsanlock_client was Ubuntu 16.04. It is no longer supported so
switch to pkg-config.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We can safely replace check for sanlock_inq_lockspace as that function
was introduced in sanlock-1.9. The oldest used version, sanlock-2.2,
is by Ubuntu 16.04.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Mon, 4 May 2020 16:00:47 +0000 (18:00 +0200)]
build: use DLOPEN_LIBS directly
There is no need to have DRIVER_MODULES_LIBS as it's used only for
libvirt.so. The other places are using DLOPEN_LIBS directly and dlopen
is required if building with libvirtd.
- Add a check for asm/hwcap.h header presence,
- Add a check for getauxval() function that is used
on Linux, and for elf_aux_info() which is a FreeBSD
equivalent.
This is based on a patch submitted by Mikael Urankar in
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247722.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When preparing for the removal of GNULIB commit 18dca21a32e9 removed the
unneeded O_DIRECTORY, but unfortunately started opening the directory for
writing which fails every time for a directory. There is also no need for that
as flock() works on O_RDONLY file descriptor as well, even for LOCK_EX.
Laine Stump [Sun, 5 Jul 2020 03:43:52 +0000 (23:43 -0400)]
libxl: eliminate extra copy of string
libxlMakeNic was calling g_strdup(virBufferCurrentContent(&buf)) to
make a copy of the buffer contents, and then later freeing the buffer
without ever using it again. Instead of this extra strdup, just
transfer ownership of the virBuffer's string with
virBufferContentAndReset(), and be done with it.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Sat, 4 Jul 2020 22:09:21 +0000 (18:09 -0400)]
remove redundant calls to virBufferFreeAndReset()
There are several calls to virBufferFreeAndReset() when functions
encounter an error, but the caller never uses the virBuffer once an
error has been encountered (all callers detect error by looking at the
function return value, not the contents of the virBuffer being
operated on), and now that all virBuffers are auto-freed there is no
reason for the lower level functions like these to spend time freeing
a buffer that is guaranteed to be freed momentarily anyway.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Sat, 4 Jul 2020 21:55:59 +0000 (17:55 -0400)]
conf: consistently check for error when calling virSysinfoFormat()
Every other caller of this function checks for an error return and
ends their formatting early if there is an error. This function
happily continues on its way.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Sat, 4 Jul 2020 21:45:57 +0000 (17:45 -0400)]
qemu: remove unnecessary virBufferFreeAndReset() after virCommandAddArgBuffer()
The latter function is guaranteed to always clear out the virBuffer
anyway, so this is redundant and could add to extra cargo-cult code if
used as an example.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Thu, 2 Jul 2020 22:03:45 +0000 (18:03 -0400)]
bhyve: use g_auto() for all virBuffers
In most cases this eliminates one or more calls to
virBufferClearAndReset(), but even when it doesn't it's better because:
1) it makes the code more consistent, making it more likely that new
contributors who are "learning by example" will to the right thing.
2) it protects against future modifications that might have otherwise
needed to add a virBufferClearAndReset()
3) Currently some functions don't call virBufferClearAndReset() only
because they're relying on some subordinate function to call it for
them (e.g. bhyveConnectGetSysinfo() in this patch relies on
virSysinfoFormat() to clear out the buffer when there is an
error). I think this is sloppy behavior, and that the toplevel
function that defines and initializes the buffer should be the
function clearing it at the end.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
conf, qemu: consider available CPUs in vcpupin/emulatorpin output
The output of vcpupin and emulatorpin for a domain with vcpu
placement='static' is based on a default bitmap that contains
all possible CPUs in the host, regardless of the CPUs being offline
or not. E.g. for a Linux host with this CPU setup (from lscpu):
This is benign by its own, but can make the user believe that all
CPUs from the 0-191 range are eligible for pinning. Which can lead
to situations like this:
$ sudo ./run tools/virsh vcpupin vcpupin_test 0 1
error: Invalid value '1' for 'cpuset.cpus': Invalid argument
This is exarcebated by the fact that 'virsh vcpuinfo' considers only
available host CPUs in the 'CPU Affinity' field:
$ sudo ./run tools/virsh vcpuinfo vcpupin_test
(...)
CPU Affinity: y-------y-------y-------(...)
This patch changes the default bitmap of vcpupin and emulatorpin, in
the case of domains with static vcpu placement, to all available CPUs
instead of all possible CPUs. Aside from making it consistent with
the behavior of 'vcpuinfo', users will now have one less incentive to
try to pin a vcpu in an offline CPU.
The idea is to have a function that calls virHostCPUGetOnlineBitmap()
but, instead of returning NULL if the host does not have CPU
offlining capabilities, fall back to a bitmap containing all
present CPUs.
Next patch will use this helper in two other places.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function reads the string in sysfspath/cpu/present and
parses it manually to retrieve the number of present CPUs.
virHostCPUGetPresentBitmap() reads and parses the same file,
using a more robust parser via virBitmapParseUnlimited(),
but returns a bitmap. Let's drop all the manual parsing done
here and simply return the size of the resulting bitmap
from virHostCPUGetPresentBitmap().
Given that no more parsing is being done manually in the function,
rename it to virHostCPUCountLinux().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_auto* pointers to avoid the need for the cleanup label. The
type of the pointer 'virDomainPtr dom' was changed to its alias
'virshDomainPtr' to allow the use of g_autoptr().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since the ESX virtual hardware version 4.0, virtual machines support up
to 10 virtual NICs instead of 4 previously. This changes the limit
accordingly based on the provided `virtualHW.version`.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A few commits ago, in aeecbc87b73, I've implemented command line
generation for ACPI HMAT. For this, we need to know if at least
one guest NUMA node has vCPUs. This is tracked in
@masterInitiator variable, which is initialized to -1, then we
iterate through guest NUMA nodes and break the loop if we find a
node with a vCPU. After the loop, if masterInitiator is still
negative then no NUMA node has a vCPU and we error out. But this
exact check was missing comparison for negativeness.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Thu, 28 May 2020 15:23:00 +0000 (17:23 +0200)]
conf: Validate NUMA HMAT configuration
There are several restrictions, for instance @initiator and
@target have to refer to existing NUMA nodes (daa), @cache has to
refer to a defined cache level and so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 27 May 2020 09:46:33 +0000 (11:46 +0200)]
conf: Parse and format HMAT
To cite ACPI specification:
Heterogeneous Memory Attribute Table describes the memory
attributes, such as memory side cache attributes and bandwidth
and latency details, related to the System Physical Address
(SPA) Memory Ranges. The software is expected to use this
information as hint for optimization.
According to our upstream discussion [1] this is exposed under
<numa/> as <cache/> under NUMA <cell/> and <latency> or
<bandwidth/> under numa/latencies.
Michal Privoznik [Wed, 27 May 2020 11:42:22 +0000 (13:42 +0200)]
Allow NUMA nodes without vCPUs
QEMU allows creating NUMA nodes that have memory only.
These are somehow important for HMAT.
With check done in qemuValidateDomainDef() for QEMU 2.7 or newer
(checked via QEMU_CAPS_NUMA), we can be sure that the vCPUs are
fully assigned to NUMA nodes in domain XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 27 May 2020 09:49:51 +0000 (11:49 +0200)]
numa_conf: Make virDomainNumaSetNodeCpumask() return void
There is only one caller of virDomainNumaSetNodeCpumask() which
checks for the return value but because the function will return
NULL iff the @cpumask was NULL in the first place. But in that
place @cpumask can't be NULL because it was just allocated by
virBitmapParse().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Fri, 29 May 2020 12:57:20 +0000 (14:57 +0200)]
qemu_command: Rename qemuBuildNumaArgStr()
The function doesn't just build the argument for -numa. Since the
-numa can be repeated multiple times, it also puts -numa onto the
cmd line. Also, the rest of the functions has 'Command' infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Thu, 28 May 2020 14:52:22 +0000 (16:52 +0200)]
numa_conf: Drop CPU from name of two functions
There are two functions virDomainNumaDefCPUFormatXML() and
virDomainNumaDefCPUParseXML() which format and parse domain's
<numa/>. There is nothing CPU specific about them. Drop the
infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Thu, 28 May 2020 10:38:43 +0000 (12:38 +0200)]
conf: Move and rename virDomainParseScaledValue()
There is nothing domain specific about the function, thus it
should not have virDomain prefix. Also, the fact that it is a
static function makes it impossible to use from other files.
Move the function to virxml.c and drop the 'Domain' infix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 27 May 2020 12:24:56 +0000 (14:24 +0200)]
qemuxml2xmltest: Add "numatune-distance" test case
This test case checks that expanding NUMA distance works. On
input we accept if only distance from A to B is specified. On the
output we format the B to A distance too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Wed, 8 Jul 2020 07:13:42 +0000 (09:13 +0200)]
qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
This is okay for the command line visitor, but the property is declared
as boolean in qemu and thus it will not work when using QMP.
Modify the type to boolean. This changes the command line, but
fortunately the command line visitor in qemu parses both 'yes' and 'on'
as true for the property.
projects / thirdparty / libvirt.git / log
