It's no longer needed and is valid only after virDomainSnapshotAlignDisks
is called while holding the lock.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Last step of the algorithm in virDomainSnapshotAlignDisks is to extend
the array of disks to all VM's disk and provide defaults. This was done
by extending the array, adding defaults at the end and then sorting it.
This requires the 'idx' variable and also a separate sorting function.
If we store the pointer to existing snapshot disk definitions in a hash
table and create a new array of snapshot disk definitions, we can fill
the new array directly by either copying the definition from the old
array or adding the default.
This avoids the sorting step and thus even the need to store the index
of the domain disk altogether.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Remove the use of the 'disk_snapshot' temporary variable since accessing
the disk definition now isn't that much longer to write and use explicit
value checks instead of the (non-)zero check to make it more obvious
what the code is doing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The converted string is used exactly once so we can call the conversion
without storing the result in a variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the disk def to a local variable so that it's more obvious
what's happening and it will also allow further simplification.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are multiple places accessing the domain definition. Extract it to
a local variable so that it's more clear what's happening.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'disk' variable usually refers to a definition of a disk from the
domain definition. Rename it to 'snapdisk' to be clear that we are
talking about the snapshot disk definition especially since this
function also accesses the domain disk definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While this function resides in the snapshot config module, the 'def'
variable is referencing the VM definition in most places. Change the
name to 'snapdef' to avoid ambiguity especially since we are also
dealing with the domain definition in this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic pointer for the bitmap and get rid of the 'cleanup' label
and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After virDomainSnapshotAlignDisks is called the definitions of disks in
the snapshot definition and in the domain definition are in the same
order so they can be addressed using the same index.
This frees up 'idx' to be removed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Before:
$ uname -m
s390x
$ cat passthrough-cpu.xml
<cpu check="none" mode="host-passthrough" />
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
error: Failed to compare hypervisor CPU with passthrough-cpu.xml
error: internal error: unable to execute QEMU command 'query-cpu-model-comp
arison': Invalid parameter type for 'modelb.name', expected: string
After:
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
CPU described in passthrough-cpu.xml is identical to the CPU provided by hy
pervisor on the host
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The alignment for the pSeries NVDIMM does not depend on runtime
constraints. This means that it can be done in device parse
time, instead of runtime, allowing the domain XML to reflect
what the auto-alignment would do when the domain starts.
This brings consistency between the NVDIMM size reported by the
domain XML and what the guest sees, without impacting existing
guests that are using an unaligned size - they'll work as usual,
but the domain XML will be updated with the actual size of the
NVDIMM.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We'll use the auto-alignment function during parse time, in
domain_conf.c. Let's move the function to that file, renaming
it to virDomainNVDimmAlignSizePseries(). This will also make it
clearer that, although QEMU is the only driver that currently
supports it, pSeries NVDIMM restrictions aren't tied to QEMU.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
g_thread_join() eats a reference.
==295055== Invalid read of size 4
==295055== at 0x4DA4AE4: g_thread_unref (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D5FA: vir_event_thread_finalize (vireventthread.c:47)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== by 0x22E98A29: qemuDomainPostParseDataAlloc (qemu_domain.c:5476)
==295055== by 0x49ABF83: virDomainDefPostParse (domain_conf.c:6023)
==295055== Address 0x2acb1c68 is 24 bytes inside a block of size 88 free'd
==295055== at 0x483B9F5: free (vg_replace_malloc.c:538)
==295055== by 0x4D80A4C: g_free (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x491D5F1: vir_event_thread_finalize (vireventthread.c:46)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== Block was alloc'd at
==295055== at 0x483A809: malloc (vg_replace_malloc.c:307)
==295055== by 0x4D80958: g_malloc (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x4DA4C32: g_thread_try_new (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D3BC: virEventThreadStart (vireventthread.c:159)
==295055== by 0x491D3BC: virEventThreadNew (vireventthread.c:185)
...
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f4fc3db920
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
g_variant_new() returns a weak reference which can be consumed by passing
to other g_variant* functions or to g_dbus_connection_call* functions.
This make it possible to call g_variant_new() directly as argument to
the functions above. Because this might be confusing I explicitly call
g_variant_ref_sink() to make it normal reference in both
virGDBusCallMethod() and virGDBusCallMethodWithFD() so the caller is
always responsible for the data.
Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
g_variant_iter_loop() handles freeing all arguments unless we break out
of the loop, in that case we have to free them manually.
Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
cppcheck reports:
style: Argument 'fd<0' to function virSetCloseExec is always 0 [knownArgument]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 4b9919af40
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If the mapping is not present, we should not try to
access its elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 8b5b80f4c5
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Sometimes parallel compilation randomly fails on platforms
that do not have many drivers enabled, like macOS:
In file included from ../tests/esxutilstest.c:13:
../src/esx/esx_vi_types.h:62:10: fatal error: 'esx_vi_types.generated.typedef' file not found
#include "esx_vi_types.generated.typedef"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
List esx_gen_headers as a source to stop meson from building
it before the headers are generated.
https://gitlab.com/libvirt/libvirt/-/jobs/726039284
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Invoke the generator twice and introduce separate
meson targets for headers and C sources.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We used to check the format of reply data with libdbus so we should do
the same with GLib DBus as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to pass pointer to `array`.
Reported-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
virFileComparePaths just return 0 or 1 after commit 7b48bb8
so break while after virFileComparePaths return 1
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Yi Li <yili@winhong.com>
A previous change to this function's password handling broke the use of
default values for credential types other than VIR_CRED_PASSPHRASE and
VIR_CRED_NOECHOPROMPT.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Signed-off-by: Matt Coleman <matt@datto.com>
Support setting a password for the VNC framebuffer using the passwd
attribute on the <graphics/> element, if the driver has the
BHYVE_CAP_VNC_PASSWORD capability.
Note that virsh domxml-from-native does not output the password in the
generated XML, as VIR_DOMAIN_DEF_FORMAT_SECURE is not set when
formatting the domain definition.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduces the BHYVE_CAP_VNC_PASSWORD capability, which is probed by
parsing the error message from the bhyve command. When it is not
supported, bhyve -s 0,fbuf,password= will return an error message.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The resolution of the VNC framebuffer can now be set via the resolution
definition introduced in 5.9.0.
Also, add "gop" to the list of model types the <resolution/>
sub-element is valid for.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv
parameters for a frame-buffer device to <graphics/> and <video/>
definitions.
For now, only the listen address, port, and vga mode are detected.
Unsupported parameters are silently skipped.
This involves upgrading the private API to expose the
virDomainGraphicsDefNew helper function, which is used by
bhyveParsePCIFbuf.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In [1], changes were made to remove the existing auto-alignment
for pSeries NVDIMM devices. That design promotes strange situations
where the NVDIMM size reported in the domain XML is different
from what QEMU is actually using. We removed the auto-alignment
and relied on standard size validation.
However, this goes against Libvirt design philosophy of not
tampering with existing guest behavior, as pointed out by Daniel
in [2]. Since we can't know for sure whether there are guests that
are relying on the auto-alignment feature to work, the changes
made in [1] are a direct violation of this rule.
This patch reverts [1] entirely, re-enabling auto-alignment for
pSeries NVDIMM as it was before. Changes will be made to ease
the limitations of this design without hurting existing
guests.
This reverts the following commits:
- commit 2d93cbdea9
Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
- commit 0ee56369c8
qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type
- commit 07de813924
qemu_domain.c: do not auto-align ppc64 NVDIMMs
- commit 0ccceaa57c
qemu_validate.c: add pSeries NVDIMM size alignment validation
- commit 4fa2202d88
qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html
[2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Currently, slot 1 is only allowed to be used by the LPC device.
Relax this requirement and allow to use slot 1 if it was explicitly
specified by the user for any other device type. In this case the LPC
device will have the next available address.
If slot 1 was not used by the user, it'll be reserved for the LPC
device, even if it is not configured to make address assignment
consistent in case the LPC device becomes necessary (e.g. the user
adds a console or a video device which require LPC).
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Support modeling of the 'isa' controller for bhyve. User can manually
define any PCI slot for the 'isa' controller, including PCI slot 1,
but other devices are not allowed to use this address.
When domain configuration requires the 'isa' controller to be present,
automatically add it on domain post-parse stage.
Now, as this controller is always available when needed, it's not
necessary to implicitly add it to the bhyve command line, so remove
bhyveBuildLPCArgStr().
Also, make bhyveDomainDefNeedsISAController() static as it's no longer
used outside of bhyve_domain.c.
As more than one ISA controller is not supported by bhyve,
and multiple controllers with the same index are forbidden,
so forbid ISA controllers with non-zero index for bhyve.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce 'isa' controller type. In domain XML it looks this way:
...
<controller type='isa' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x0'/>
</controller>
...
Currently, this is needed for the bhyve driver to allow choosing a
specific PCI address for that. In bhyve, this controller is used to
attach serial ports and a boot ROM.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
An extra parameter was added to virQEMUBuildQemuImgKeySecretOpts in
commit ecfc4094d8
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Sep 15 16:30:37 2020 +0100
storage: add support for qcow2 LUKS encryption
but the non-null pointer annotations were not adjusted to take account.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current debug message reports the "mode" after selection has
completed, however, the "mode" value can be changed by the selection
logic. It is thus beneficial to report most values upfront, and only
report newly changed values at the end.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The storage driver was wired up to support creating raw volumes in LUKS
format, but was never adapted to support LUKS-in-qcow2. This is trivial
as it merely requires the encryption properties to be prefixed with
the "encrypt." prefix, and "encrypt.format=luks" when creating the
volume.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Crypt method number 2 indicates LUKS format.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
b_info->u.hvm.{acpi,apic} are deprecated. But also, on recent libxl
version (4.14) the old one seems to be broken. While libxl part should
be fixed too, update the usage here and at some point drop support for
the old version.
b_info->acpi was added in Xen 4.8
b_info->apic was added in Xen 4.10
Xen 4.10 is the oldest version that still has security support (until
December 2020).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
With libdbus our wrappers had a special syntax to create the DBus
messages by defining the DBus message signature followed by list
of arguments providing data based on the signature.
There will be no similar helper with GLib implementation as they
provide same functionality via GVariant APIs. The syntax is slightly
different mostly for how arrays, variadic types and dictionaries are
created/parsed.
Additional difference is that with GLib DBus everything is wrapped in
extra tuple (struct). For more details refer to the documentation [1].
[1] <https://developer.gnome.org/glib/stable/gvariant-format-strings.html>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was one attempt a year ago done by me to drop HAL [1] but it was
never resolved. There was another time when Dan suggested to drop HAL
driver [2] but it was decided to keep it around in case device
assignment will be implemented for FreeBSD and the fact that
virt-manager uses node device driver [3].
I checked git history and code and it doesn't look like bhyve supports
device assignment so from that POV it should not block removing HAL.
The argument about virt-manager is not strong as well because libvirt
installed from FreeBSD packages doesn't have HAL support so it will not
affect these users as well [4].
The only users affected by this change would be the ones compiling
libvirt from GIT on FreeBSD.
I looked into alternatives and there is libudev-devd package on FreeBSD
but unfortunately it doesn't work as it doesn't list any devices when
used with libvirt. It provides libudev APIs using devd.
I also looked into devd directly and it provides some APIs but there are
no APIs for device monitoring and events so that would have to be
somehow done by libvirt.
Main motivation for dropping HAL support is to replace libdbus with GLib
dbus implementation and it cannot be done with HAL driver present in
libvirt because HAL APIs heavily depends on symbols provided by libdbus.
[1] <https://www.redhat.com/archives/libvir-list/2019-May/msg00203.html>
[2] <https://www.redhat.com/archives/libvir-list/2016-April/msg00992.html>
[3] <https://www.redhat.com/archives/libvir-list/2016-April/msg00994.html>
[4] <https://svnweb.freebsd.org/ports/head/devel/libvirt/Makefile?view=markup>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Meson is not told that the .x protocol files are an input for the
generator, so it doesn't know to setup a rebuild dependancy.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This patch is just revert of [1]. Actually we should NOT pass
QEMU_ASYNC_JOB_NONE as that patch suggests while we are in async job in order
to acquire nested jobs correctly. The patch tries to fix issues introduced by
another patch [2] where jobs are mistakenly cleared out in qemuProcessStop.
Later patch [3] fixed the issue introduced by patch [2]. Now we need to revert
[1] as well as we now still have same concurrency crash issues as [3] described
but for the force revert.
[1] 0c4408c83: qemu: Don't use asyncJob after stop during snapshot revert
[2] 888aa4b6b: qemuDomainObjPrivateDataClear: Don't leak @migParams
[3] d75f865fb: qemu: fix concurrency crash bug in snapshot revert
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The operands were reversed, producing an incorrect result.
Co-authored-by: Sri Ramanujam <sramanujam@datto.com>
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'readonly' hostdev property is stored separately from the
virStorageSource as some hostdevs are not described by a virStorage
source. We need to propagate the flag to the virStorage source also for
iSCSI backends as it's used to generate the backend properties.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1868856
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We've put the aliases into the backup job definition after the status
XML was already written so they didn't appear in the on-disk state.
Move the code putting them into the private definition earlier, so that
the status XML update done by saving blockjobs already writes them out.
Also add a note notifying that the block job status update writes the
status XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870488
Fixes: 423576679a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 423576679a implementing TLS forgot to remove the comment.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU's blockdev nodenames which are used to back SCSI/iSCSI hostdevs are
limited to 32 characters. If a user passes a very long user alias as
name of the host device it's easy to end up with a too-long nodename.
To prevent this from happening don't base the nodename on the possibly
user-specified alias but on the normal sequential node name generator.
We then store the name in the status XML for further use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The secret object is used to pass data to the backend so it's better
fitting to base the secret object name on the SCSI host device backend
name.
Since we store the object alias in the status XML this modification is
safe in regards to existing guests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allocate the nodename in the setup function rather than in the command
line generator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is no longer used once we setup per-hostdev data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically we've prepared secrets for all objects in one place. This
doesn't make much sense and it's semantically more appealing to prepare
everything for a single device type in one place.
Move the setup of the (iSCSI|SCSI) hostdev secrets into a new function
which will be used to setup other things as well in the future.
This is a similar approach we do for disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This was a hack when we were locally regenerating the nodename so that
it's not leaked. Now that we use proper virStorageSource with
persistence it's no longer required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the attach/detach data generators to actually use the
virStorageSourceStructure embedded in the SCSI config data rather than
creating an ad-hoc internal one.
The modification will allow us to properly store the nodename used for
the backend in the status XML rather than re-generating it all the
time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For upgrade reasons so that we can modify the used nodename we must
generate the old version for all status XMLs which don't have it stored
explicitly.
The change will be required as using the user-provided alias may result
in too-long nodenames which will be rejected by qemu.
Add code which fills in the appropriate old value and add test cases to
validate that it's added and also that existing nodenames are not
overwritten.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The backend for the SCSI host device is a storage source. While the
definition doesn't look like that it's converted to a storage source
when the VM is running.
Add the storage source to the definition object and also parse/format
its private data which will be used for internal state storage while
the VM is running.
Note that the virStorageSourcePtr may not be allocated all the time so
the private data parser allocates it if there is any private data
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use XPath instead of iterating through the nodes.
Few error messages were modified so that the parser can be written in a
simpler way.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use XPath to get the host list instead of iterating through the nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's just one caller for the function. Move the code into the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add the definition of the GuestNicInfo object, with all the required
objects for it.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implement the .connectListAllNetworks networks API in the esx network
driver.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow vfio-ccw mdev devices to be created besides vfio-pci mdev devices
as well.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Allow to filter for CSS devices.
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Make channel subsystem (CSS) devices available in the node_device driver.
The CCS devices reside in the computer system and provide CCW devices, e.g.:
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
|
+- scsi_target0_0_0
|
+- scsi_0_0_0_0
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Refactor out CCW address parsing for later reuse.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
After we started copying the privateData pointer in
qemuDomainObjRestoreJob, we should also free them
once we're done with them.
Register the clear function and use g_auto.
Also add a check for job->cb to qemuDomainObjClearJob,
to prevent freeing an uninitialized job.
https://bugzilla.redhat.com/show_bug.cgi?id=1878450
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: aca37c3fb2
A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent
the number of the error, even in cases where there is one fitting more.
Hence, replace some of them with better virErrorNumber values.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Implement the .domainGetHostname hypervisor driver API to get the
hostname of a running guest (needs VMware Tools).
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
The original motivation for adding virNetDevIPCheckIPv6Forwarding
(commit 00d28a78b5) was that networking routes would disappear when
ipv6 forwarding was enabled for an interface.
This is a fairly undocumented side-effect of the "accept_ra" sysctl
for an interface. 1 means the interface will accept_ra's if not
forwarding, 2 means always accept_RAs; but it is not explained that
enabling forwarding when accept_ra==1 will also clear any kernel RA
assigned routes, very likely breaking your networking.
The check to warn about this currently uses netlink to go through all
the routes and then look at the accept_ra status of the interfaces.
However, it has been noticed that this problem does not affect systems
where IPv6 RA configuration is handled in userspace, e.g. via tools
such as NetworkManager. In this case, the error message from libvirt
is spurious, and modifying the forwarding state will not affect the RA
state or disable your networking.
If you refer to the function rt6_purge_dflt_routers() in the kernel,
we can see that the routes being purged are only those with the
kernel's RTF_ADDRCONF flag set; that is, routes added by the kernel's
RA handling. Why does it do this? I think this is a Linux
implementation decision; it has always been like that and there are
some comments suggesting that it is because a router should be
statically configured, rather than accepting external configurations.
The solution implemented here is to convert the existing check into a
walk of /proc/net/ipv6_route (because RTF_ADDRCONF is apparently not
exposed in netlink) and look for routes with this flag set. We then
check the accept_ra status for the interface, and if enabling
forwarding would break things raise an error.
This should hopefully avoid "interactive" users, who are likely to be
using NetworkManager and the like, having false warnings when enabling
IPv6, but retain the error check for users relying on kernel-based
IPv6 interface auto-configuration.
Signed-off-by: Ian Wienand <iwienand@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cedric Bosdonnat <CBosdonnat@suse.com>
While we set up perf events for a shutoff domain and check the settings,
All of perf events are reported as 'disabled', unless we add --config,
This is redundant for a shutoff domain.
# virsh domstate $GUEST
shut off
# virsh perf --domain $GUEST
cmt : disabled
mbmt : disabled
mbml : disabled
......
# virsh perf --domain $GUEST --enable mbmt
mbmt : enabled
# virsh perf --domain $GUEST
cmt : disabled
mbmt : disabled
mbml : disabled
......
Use virDomainObjGetOneDefState instead of virDomainObjGetOneDef to fix
the issue. After patch, The perf event status of a shutoff domain is
reported correctly:
# virsh domstate $GUEST
shut off
# virsh perf --domain $GUEST
cmt : disabled
mbmt : disabled
mbml : disabled
......
# virsh perf --domain $GUEST --enable mbmt
mbmt : enabled
# virsh perf --domain $GUEST
cmt : disabled
mbmt : enabled
mbml : disabled
......
Signed-off-by: Lin Ma <lma@suse.de>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
It requires a guest agent configured and running in the domain's guest
OS, So check qemu agent during qemuDomainPMSuspendForDuration().
Signed-off-by: Lin Ma <lma@suse.de>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Note the use of g_clear_pointer(..., g_free) in ppc64DataClear and virCPUx86Baseline.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When the guest agent isn't running, we still report success on a PM
suspend action even though we logged an error correctly, this is because
we poisoned the 'ret' value a few lines above.
Fixes: a663a86081
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In v6.7.0-rc1~86 I've tried to fix a problem where we were not
detecting NUMA nodes properly because we misused behaviour of a
libnuma API and as it turned out the behaviour was correct for
hosts with 64 CPUs in one NUMA node. So I changed the code to use
nodemask_isset(&numa_all_nodes, ..) instead and it fixed the
problem on such hosts. However, what I did not realize is that
numa_all_nodes does not reflect all NUMA nodes visible to
userspace, it contains only those nodes that the process
(libvirtd) an allocate memory from, which can be only a subset of
all NUMA nodes. The bitmask that contains all NUMA nodes visible
to userspace and which one I should have used is: numa_nodes_ptr.
For curious ones:
4a22f22382
And as I was fixing virNumaGetNodeCPUs() I came to realize that
we already have a function that wraps the correct bitmask:
virNumaNodeIsAvailable().
Fixes: 24d7d85208
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1876956
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This function was introduced in the 2.0.6 release which happened
in December 2010. I think it is safe to assume that all libnuma
we deal with have the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since we no longer need to wait for IPv6 DAD to complete, we never
call this function.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
0f7436ca54 added code during virtual network startup to wait for DAD
(Duplicate Address Detection) to complete if there were any IPv6
addresses on the network. This wait was needed because (according to
the commit log) "created problems when [the "dummy" tap device] is set
to IFF_DOWN prior to DAD completing".
That commit in turn referenced commit db488c7917, which had added the
code to set the dummy tap device IFF_DOWN, commenting "DAD has
happened (dnsmasq waits for it)", and in its commit message pointed
out that if we just got rid of the dummy tap device this wouldn't be
needed.
Now that the dummy tap device has indeed been removed (commit
ee6c936fbb), there is no longer any need to set it IFF_DOWN, and thus
nothing requiring us to wait for DAD to complete. At any rate, with
the dummy tap device removed, leaving nothing else on the bridge when
it is first started, DAD never completes, leading to failure to start
any IPv6 network.
So, yes, this patch removes the wait for DAD completion, and IPv6
networks can once again start, and their associated dnsmasq process
starts successfully (this is the problem that the DAD wait was
originally intended to fix)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We are currently adding -lutil and -lkvm to the linker using the
add_project_link_arguments method. On FreeBSD 11.4, this results in
build errors because the args appear too early in the command line.
We need to pass the libraries as dependencies so that they get placed
at the same point in the linker args as other dependencies.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This wires up support for using the new virt-ssh-helper binary with the ssh,
libssh and libssh2 protocols.
The new binary will be used preferentially if it is available in $PATH,
otherwise we fall back to traditional netcat.
The "proxy" URI parameter can be used to force use of netcat e.g.
qemu+ssh://host/system?proxy=netcat
or the disable fallback e.g.
qemu+ssh://host/system?proxy=native
With use of virt-ssh-helper, we can now support remote session URIs
qemu+ssh://host/session
and this will only use virt-ssh-helper, with no fallback. This also lets
the libvirtd process be auto-started, and connect directly to the
modular daemons, avoiding use of virtproxyd back-compat tunnelling.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Switch keyfile and netcat parameters, since the netcat path and
socket path are a logical pair that belong together. This patches
the other constructors.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When accessing libvirtd over a SSH tunnel, the remote driver needs a way
to proxy the SSH input/output stream to a suitable libvirt daemon. This
is currently done by spawning netcat, pointing it to the libvirtd socket
path. This is problematic for a number of reasons:
- The socket path varies according to the --prefix chosen at build
time. The remote client is seeing the local prefix, but what we
need is the remote prefix
- The socket path varies according to remote env variables, such as
the XDG_RUNTIME_DIR location. Again we see the local XDG_RUNTIME_DIR
value, but what we need is the remote value (if any)
- The remote driver doesn't know whether it must connect to the legacy
libvirtd or the modular daemons, so must always assume legacy
libvirtd for back-compat. This means we'll always end up using the
virtproxyd daemon adding an extra hop in the RPC layer.
- We can not able to autospawn the libvirtd daemon for session mode
access
To address these problems this patch introduces the 'virtd-ssh-helper'
program which takes the URI for the remote driver as a CLI parameter.
It then figures out which daemon to connect to and its socket path,
using the same code that the remote driver client would on the remote
host's build of libvirt.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We'll shortly want to reuse code for determining whether to connect to
the system or session daemon from places outside the remote driver
client. Pulling it out into a self contained function facilitates reuse.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteGetUNIXSocketHelper method will be needed by source files
beyond the remote driver client.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We delay converting the remote transport string to enum form until
fairly late. As a result we're doing string comparisons when we
could be just doing enum comparisons.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteSplitURISCheme method will be needed by source files beyond
the remote driver client.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteDriverTransport and remoteDriverMode enums are going to be
needed by source files beyond the remote driver client.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't want to repeat the choice of default netcat binary setting in
three different places. This will also make it possible to do better
error reporting in the helper.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Three parts of the code all build up the same SSH shell script
snippet for remote tunneling the RPC protocol, but in slightly
different ways. Combine them all into one helper method in the
virNetClient code, since this logic doesn't really belong in
the virNetSocket code.
Note that the this change means the shell snippet is passed to
the SSH binary as a single arg, instead of three separate args,
but this is functionally identical, as the three separate args
were combined into one already when passed to the remote system.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The changed condition was always false because the function was always
called with boundary values 0.
Use the free extent's start value to get its start offset from the
cylinder boundary and determine if the needed size for allocation
needs to be expanded too in case the offset doesn't fit within extra
bytes for alignment.
This fixes an issue where vol-create-from will call qemu-img convert
to create a destination volume of same capacity as the source volume
and qemu-img will error 'Cannot grow device files' due to the partition
being too small for the source although both destination partition and
source volume have the same capacity.
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When closing client->waitDispatch in virNetClientIOEventLoopRemoveAll
or virNetClientIOEventLoopRemoveDone, VIR_FREE() is called to free
call->msg directly, resulting in leak of the memory call->msg->buffer
points to.
Use virNetMessageFree(call->msg) instead of VIR_FREE(call->msg).
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
In 8e1804f9f6 I've tried to fix the following use case: domain
is started with path to UEFI only and relies on libvirt to figure
out corresponding NVRAM template to create a per-domain copy
from. The fix consisted of having a check tailored exactly for
this use case and if it's hit then using FW autoselection to
figure it out. Unfortunately, the NVRAM template is not saved in
the inactive XML (well, the domain might be transient anyway).
Then, as a part of that check we see whether the per-domain copy
doesn't exist already and if it does then no template is looked
up hence no template will appear in the live XML.
This works, until the domain is migrated. At the destination, the
per-domain copy will not exist so we need to know the template to
create the per-domain copy from. But we don't even get to the
check because we are not starting a fresh new domain and thus the
qemuFirmwareFillDomain() function quits early.
The solution is to switch order of these two checks. That is
evaluate the check for the old style before checking flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852910
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The call to `g_strfreev` is not required, as in both cases no memory has been allocated.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Using g_autoptr() in virCPUDef pointers allows for more
cleanups in ppc64Compute() and virCPUppc64Baseline()
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This allows for a label removal in ppc64ModelParse().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
We don't need to call virCPUppc64DataFree() in a cleanup label.
This function is already assigned to the 'dataFree' interface
of cpuDriverPPC64, and it will be called by virCPUDataFree(), the
autocleanup function of virCPUDataPtr, via driver->dataFree.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Use autocleanup with virCPUppc64MapPtr to simplify existing
code. Remove labels when possible.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Next patch will use g_autoptr() in virCPUppc64MapPtr pointers
for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce virCPUppc64Map and virCPUppc64MapPtr types to
improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Use autocleanup with virCPUppc64ModelPtr to simplify existing
code. Remove the 'error' label in ppc64ModelCopy() since it is
now obsolete.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Next patch will use g_autoptr() in virCPUppc64ModelPtr pointers
for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce virCPUppc64Model and virCPUppc64ModelPtr types to
improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Use g_autoptr() in virCPUppc64VendorPtr and remove the now
uneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Next patch will use g_autoptr() in virCPUppc64VendorPtr pointers
for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce virCPUppc64Vendor and virCPUppc64VendorPtr types to
improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The correct key for dependencies for virt_modules hash is `deps`.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
g_regex_unref reports an error if called with a NULL argument.
We have two cases in the code where we (possibly) call it on a NULL
argument. The interesting one is in virDomainQemuMonitorEventCleanup.
Based on VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX, we unref
data->regex, which has two problems:
* On the client side, flags is -1 so the comparison is true even if no
regex was used, reproducible by:
$ virsh qemu-monitor-event --timeout 1
which results in an ugly error:
(process:1289846): GLib-CRITICAL **: 14:58:42.631: g_regex_unref: assertion 'regex != NULL' failed
* On the server side, we only create the regex if both the flag and the
string are present, so it's possible to trigger this message by:
$ virsh qemu-monitor-event --regex --timeout 1
Use a non-NULL comparison instead of the flag to decide whether we need
to unref the regex. And add a non-NULL check to the unref in the
VirtualBox test too.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 71efb59a4dhttps://bugzilla.redhat.com/show_bug.cgi?id=1876907
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When starting a domain with <numatune/> set libvirt translates
given NUMA nodes into a set of host CPUs which is then used to
QEMU process affinity. But, if the numatune contains a
non-existent NUMA node then the translation fails with no error
reported. This is because virNumaNodesetToCPUset() calls
virNumaGetNodeCPUs() and expects it to report an error on
failure. Well, it does except for non-existent NUMA nodes. While
this behaviour might look strange it is actually desired because
of how we construct host capabilities. The virNumaGetNodeCPUs()
is called from virCapabilitiesHostNUMAInitReal() where we do not
want any error reported for non-existent NUMA nodes.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1724866
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When starting a domain, qemuProcessLaunch() iterates over all
VIR_PERF_EVENT_* values and (possibly) enables them. While there
is nothing wrong with the code, the for loop where it's done makes
it harder to jump onto next block of code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Propagate the cluster size from the original image as the user might
have configured a custom cluster size for performance reasons. Propagate
the cluster size of a qcow2 image to the new overlay or copy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
'blockdev-create' allows us to create the image with a custom cluster
size if we wish to. Wire it up for 'qcow2'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Configuring the cluster size of an image may have performance
implications. This patch allows us to detect cluster size for existing
images so that we will be able to propagate it to new images which are
based on existing images e.g. during snapshots/block-copy/etc.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When building and populating domain NS a couple of functions are
called that append paths to a string list. This string list is
then inspected, one item at the time by
qemuNamespacePrepareOneItem() which gathers all the info for
given path (stat buffer, possible link target, ACLs, SELinux
label) using qemuNamespaceMknodItemInit(). If the path needs to
be created in the domain's private /dev then it's added onto this
qemuNamespaceMknodData list which is freed later in the process.
But, if the path does not need to be created in the domain's
private /dev, then the memory allocated by
qemuNamespaceMknodItemInit() is not freed anywhere leading to a
leak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is just a clean-up of commit 3791f29b08 using the new parameter of
virProcessSetAffinity() introduced in commit 9514e24984 so that there is
no error reported in the logs.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the past we had to declare @cfg and then explicitly unref it.
But now, with glib we can use g_autoptr() which will do the unref
automatically and thus is more bulletproof.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
In the qemuInterfacePrepareSlirp() function, the qemu driver
config is obtained (via virQEMUDriverGetConfig()), but it is
never unrefed leading to mangled refcounter.
Fixes: 9145b3f1cc
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
On shutdown we just stop accepting new jobs for worker thread so that on
shutdown wait we can exit worker thread faster. Yes we basically stop
processing of events for VMs but we are going to do so anyway in case of daemon
shutdown.
At the same time synchronous event processing that some API calls may require
are still possible as per VM event loop is still running and we don't need
worker thread for synchronous event processing.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We are dropping the only reference here so that the event loop thread
is going to be exited synchronously. In order to avoid deadlocks we
need to unlock the VM so that any handler being called can finish
execution and thus even loop thread be finished too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It it useful to be sure no thread is running after we drop all references to
virEventThread. Otherwise in order to avoid crashes we need to synchronize some
other way or we make extra references in event handler callbacks to all the
object in use. And some of them are not prepared to be refcounted.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This hunk was introduced in [1] in order to avoid loosing
events from monitor on stopping qemu process. But as explained
in [2] on destroy we won't get neither EOF nor any other
events as monitor is just closed. In case of crash/shutdown
we won't get any more events as well and qemuDomainObjStopWorker
will be called by qemuProcessStop eventually. Thus let's
remove qemuDomainObjStopWorker from qemuProcessHandleMonitorEOF
as it is not useful anymore.
[1] e6afacb0f: qemu: start/stop an event loop thread for domains
[2] d2954c072: qemu: ensure domain event thread is always stopped
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently we have issues like [1] on libvirtd shutdown as we cleanup while RPC
and other threads are still running. Let's finish all threads other then main
before cleanup.
The approach to finish threads is suggested in [2]. In order to finish RPC
threads serving API calls we let the event loop run but stop accepting new API
calls and block processing any pending API calls. We also inform all drivers of
shutdown so they can prepare for shutdown too. Then we wait for all RPC threads
and driver's background thread to finish. If finishing takes more then 15s we
just exit as we can't safely cleanup in time.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1828207
[2] https://www.redhat.com/archives/libvir-list/2020-April/msg01328.html
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virNetServerClose and virNetServerShutdownWait are used to start net server
threads shutdown and wait net server threads to actually finish respectively
during net daemon shutdown procedure.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function is used to set shutdown prepare and wait callbacks. Prepare
callback is used to inform other threads of the daemon that the daemon will be
closed soon so that they can start to shutdown. Wait callback is used to wait
for other threads to actually finish.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Second unref was added in [1]. We don't need it actually as
we pass free callback to virNetSocketAddIOCallback thus
when we call virNetSocketRemoveIOCallback the extra ref for
callback will be dropped without extra efforts.
[1] 355d8f470f: virNetServerServiceClose: Don't leak sockets
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Stop just send signal for threads to exit when they finish with
current task. Drain waits when all threads will finish.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Even if we have no priority threads on pool creation we can add them thru
virThreadPoolSetParameters later.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
stateShutdownPrepare is supposed to inform driver that it will be closed soon
so that the driver can prepare and finish all background threads quickly on
stateShutdownWait call.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
A long time ago we introduced a dummy tap device (e.g. virbr0-nic) that
we attached to the bridge device created for virtual networks:
commit 5754dbd56d
Author: Laine Stump <laine@redhat.com>
Date: Wed Feb 9 03:28:12 2011 -0500
Give each virtual network bridge its own fixed MAC address
This was a hack to workaround a Linux kernel bug where it would not
honour any attempt to set a MAC address on a bridge. Instead the
bridge would adopt the numerically lowest MAC address of all NICs
attached to the bridge. This lead to the MAC addrss of the bridge
changing over time as NICs were attached/detached.
The Linux bug was actually fixed 3 years before the libvirt
workaround was added in:
commit 92c0574f11598c8036f81e27d2e8bdd6eed7d76d
Author: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue Jun 17 16:10:06 2008 -0700
bridge: make bridge address settings sticky
Normally, the bridge just chooses the smallest mac address as the
bridge id and mac address of bridge device. But if the administrator
has explictly set the interface address then don't change it.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
but libvirt needed to support RHEL-5 kernels at that time, so
none the less added the workaround.
We have long since dropped support for RHEL-5 vintage distros,
so there's no reason to keep the dummy tap device for the purpose
of setting the bridge MAC address.
Later the dummy TAP device was used for a second purpose related
to IPv6 DAD (Duplicate Address Detection) in:
commit db488c7917
Author: Benjamin Cama <benoar@dolka.fr>
Date: Wed Sep 26 21:02:20 2012 +0200
network: fix dnsmasq/radvd binding to IPv6 on recent kernels
This was again dealing with a regression in the Linux kernel, where
if there were no devices attached to the bridge in the UP state,
IPv6 DAD would not be performed. The virbr0-nic was attached but
in the DOWN state, so the above libvirt fix tenporarily brought
the NIC online. The Linux commit causing the problem was in v2.6.38
commit 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
Author: stephen hemminger <shemminger@vyatta.com>
Date: Mon Mar 7 08:34:06 2011 +0000
bridge: control carrier based on ports online
A short while later Linux was tweaked so that DAD would still occur
if the bridge had no attached devices at all in 3.1:
commit b64b73d7d0c480f75684519c6134e79d50c1b341
Author: stephen hemminger <shemminger@vyatta.com>
Date: Mon Oct 3 18:14:45 2011 +0000
bridge: leave carrier on for empty bridge
IOW, the only reason we need the DAD hack of bringing virbr0-nic
online is because virbr0-nic exists. Once it doesn't exist, then
we hit the "empty bridge" case which works in Linux.
We can rely on distros having Linux kernel >= 3.1, so both things
that the virbr0-nic are doing are redundant.
Fixes https://gitlab.com/libvirt/libvirt/-/issues/53
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Consider a host with 8 CPUs. There are the following possible scenarios
1. Bare metal; libvirtd has affinity of 8 CPUs; QEMU should get 8 CPUs
2. Bare metal; libvirtd has affinity of 2 CPUs; QEMU should get 8 CPUs
3. Container has affinity of 8 CPUs; libvirtd has affinity of 8 CPus;
QEMU should get 8 CPUs
4. Container has affinity of 8 CPUs; libvirtd has affinity of 2 CPus;
QEMU should get 8 CPUs
5. Container has affinity of 4 CPUs; libvirtd has affinity of 4 CPus;
QEMU should get 4 CPUs
6. Container has affinity of 4 CPUs; libvirtd has affinity of 2 CPus;
QEMU should get 4 CPUs
Scenarios 1 & 2 always work unless systemd restricted libvirtd privs.
Scenario 3 works because libvirt checks current affinity first and
skips the sched_setaffinity call, avoiding the SYS_NICE issue
Scenario 4 works only if CAP_SYS_NICE is availalbe
Scenarios 5 & 6 works only if CAP_SYS_NICE is present *AND* the cgroups
cpuset is not set on the container.
If libvirt blindly ignores the sched_setaffinity failure, then scenarios
4, 5 and 6 should all work, but with caveat in case 4 and 6, that
QEMU will only get 2 CPUs instead of the possible 8 and 4 respectively.
This is still better than failing.
Therefore libvirt can blindly ignore the setaffinity failure, but *ONLY*
ignore it when there was no affinity specified in the XML config.
If user specified affinity explicitly, libvirt must report an error if
it can't be honoured.
Resolves: https://bugzilla.redhat.com/1819801
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The conditional was removed in
commit ebbf8ebe4f
Author: Ján Tomko <jtomko@redhat.com>
Date: Tue Sep 1 22:56:37 2020 +0200
util: virnetdevtap: stats: fix txdrop on FreeBSD
That commit was correct about this no longer being required for FreeBSD,
but missed that the code is also built on macOS.
Rather than testing for this field in meson though, we can simply use
a platform conditional test in the code.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Local socket connections were outright disabled because there was no "server"
part in the URI. However, given how requirements and usage scenarios are
evolving, some management apps might need the source libvirt daemon to connect
to the destination daemon over a UNIX socket for peer2peer migration. Since we
cannot know where the socket leads (whether the same daemon or not) let's decide
that based on whether the socket path is non-standard, or rather explicitly
specified in the URI. Checking non-standard path would require to ask the
daemon for configuration and the only misuse that it would prevent would be a
pretty weird one. And that's not worth it. The assumption is that whenever
someone uses explicit UNIX socket paths in the URI for migration they better
know what they are doing.
Partially resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Adds new typed param for migration and uses this as a UNIX socket path that
should be used for the NBD part of migration. And also adds virsh support.
Partially resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
For this we need to make the function accessible (at least privately). The
behaviour will change in following patches and the test helps explaining the
change.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Clean up the semantics by using one extra self-describing variable.
This also fixes the port allocation when the port is specified.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Instead of saving some data from a union up front and changing an overlayed
struct before using said data, let's just set the new values after they are
decided. This will increase the readability of future commit(s).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In 6.7.0 release I've changed how domain namespace is built and
populated. Previously it used to be done from a pre-exec hook
(ran in the forked off child, just before dropping all privileges
and exec()-ing QEMU), which not only meant we had to have two
different code paths for creating a node in domain's namespace
(one for this pre-exec hook, the other for hotplug ran from the
daemon), it also proved problematic because it was leaking FDs
into QEMU process.
To mitigate this problem, we've not only ditched libdevmapper
from the NS population process, I've also dropped the pre-exec
code and let the NS be populated from the daemon (using the
hotplug code). But, I was not careful when doing so, because the
pre-exec code was tolerant to files that doesn't exist, while
this new code isn't. For instance, the very first thing that is
done when the new NS is created is it's populated with
@defaultDeviceACL which contain files like /dev/null, /dev/zero,
/dev/random and /dev/kvm (and others). While the rest will
probably exist every time, /dev/kvm might not and thus the new
code I wrote has to be tolerant to that.
Of course, users can override the @defaultDeviceACL (by setting
cgroup_device_acl in qemu.conf) and remove /dev/kvm (which is
acceptable workaround), but we definitely want libvirt to work
out of the box even on hosts without KVM.
Fixes: 9048dc4e62
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For older FreeBSD, we needed an ifdef guard to use
if_data.ifi_oqdrops, which was introduced by:
commit 61bbdbb94c
Implement interface stats for BSD
But when we dropped the check because we deprecated
building on FreeBSD-10 in:
commit 83131d9714
configure: drop check for unsupported FreeBSD
We started building the wrong side of the ifdef.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 83131d9714
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
We need to use @default_auth@ in the augeas test case to match
its use in the main libvirtd.conf.in file.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The systemd .socket unit files we ship for libvirt daemons use
SocketMode=0666 on the assumption that libvirt is built with
polkit which provides access control.
Some people, however, may have explicitly turned off polkit at
build time and not realize that leaves them insecure unless
they also change the SocketMode. This addresses that problem
by making the SocketMode default to 0600 when polkit is
disabled at compile time.
Note we cannot automatically fix the case where the user
compiles polkit, but then overrides the libvirtd.conf defaults
to disable polkit. This is what lead to CVE-2020-15708 in
Ubuntu 20.10. We can at least improve the inline comments
in the config file to give a clearer warning though, which
may have helped avoid the mistaken config.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since QEMU 1.5.3, the ib700 watchdog device has no options for address,
and not address in device tree:
$ /usr/libexec/qemu-kvm -version
QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-175.el7), Copyright (c) 2003-2008 Fabrice Bellard
$ /usr/libexec/qemu-kvm -device ib700,\?
$ virsh qemu-monitor-command seabios --hmp info qtree|grep ib700 -A 2
dev: ib700, id "watchdog0"
dev: isa-serial, id "serial0"
index = 0
So only allow it to use none address.
Fixes: 8a54cc1d08
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1509908
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices.
However, the QEMU guest agent could also provide the device name in the
"dev" field of the response for other devices instead (well, at least after
fixing another problem in the current QEMU guest agent...). So if creating
the devAlias from the PCI information failed, let's fall back to the name
provided by the guest agent. This helps to fix the empty "Target" fields
that occur when running "virsh domfsinfo" on s390x where CCW devices are
used for the guest instead of PCI devices.
Also add a proper debug message here in case we completely failed to set the
device alias, since this problem here was very hard to debug: The only two
error messages that I've seen were "Unable to get filesystem information"
and "Unable to encode message payload" - which only indicates that something
went wrong in the RPC call. No debug message indicated the real problem, so
I had to learn the hard way why the RPC call failed (it apparently does not
like devAlias left to be NULL) and where the real problem comes from.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1755075
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
libvirt currently silently allows <timer name="kvmclock"/> and some
other timer tags in the guest XML definition for timers that do not
exist on non-x86 systems. We should not silently ignore these tags
since the users might not get what they expected otherwise.
Note: The error is only generated if the timer is marked with
present="yes" - otherwise we would suddenly refuse XML definitions
that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, we are mixing: #if HAVE_BLAH with #if WITH_BLAH.
Things got way better with Pavel's work on meson, but apparently,
mixing these two lead to confusing and easy to miss bugs (see
31fb929eca for instance). While we were forced to use HAVE_
prefix with autotools, we are free to chose our own prefix with
meson and since WITH_ prefix appears to be more popular let's use
it everywhere.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are couple of conditional #includes at the beginning of
virfile.c and they try to be nice and document #endifs. But they
are mostly wrong because either they have the condition in the
comment inverted or the comment refers to a different condition
than they belong to. Just remove the comments as these #includes
are single line mostly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two places where we try to check whether the host
system has net/if.h before including it. But the check is missing
'_H' suffix.
Fixes: 7f3eb533f4
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Setting IFF_VNET_HDR for a tap device passes the whole packet to the
host, reducing emulation overhead and improving performance.
Libvirt bases its decision about applying IFF_VNET_HDR to the tap
interface on whether or not the model of the emulated network device
is virtio. Originally, virtio was the only model to support
IFF_VNET_HDR in QEMU; however, the e1000e & vmxnet3 adapters have also
supported it since their introductions - QEMU commit
786fd2b0f87 for vmxnet3, and QEMU commit 6f3fbe4ed0 for e1000e, so it
should be set for those models too.
Signed-off-by: Patrick Magauran <patmagauran.j@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Add support for the writeFiltering attribute in the domXML to native
config converter. Also include a test.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
By default Xen only allows guests to write "known safe" values into PCI
configuration space, yet many devices require writes to other areas of
the configuration space in order to operate properly. To allow writing
any values Xen supports the 'permissive' setting, see xl.cfg(5) man page.
This change models Xen's permissive setting by adding a writeFiltering
attribute on the <source> element of a PCI hostdev. When writeFiltering
is set to 'no', the Xen permissive setting will be enabled and guests
will be able to write any values into the device's configuration space.
The permissive setting remains disabled in the absense of the
writeFiltering attribute, of if it is explicitly set to 'yes'.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add a configuration option for specifying location of the qemu modules
directory, defaulting to /usr/lib64/qemu. Then use this location to
check for changes in the directory, indicating that a qemu module has
changed and capabilities need to be reprobed.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Use https: links for websites that support them.
The URIs which are used as namespace identifiers
are left alone.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
The list archives, people.redhat.com and bugzilla all support
https.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Also, remove the url from the translatable string,
reducing it to the generic message already used
by virXMLNamespaceRegister.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
The include was introduced by:
commit 3d6fe99c5c
Add vcpu functions to libxl driver
which used ceil() and floor(), but these were later
removed by:
commit 3eb869a04b
libxl: avoid compiler warning
which did not remove the include.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
When creating a standard tap device, if provided with an ifname that
contains "%d", rather than taking that literally as the name to use
for the new device, the kernel will instead use that string as a
template, and search for the lowest number that could be put in place
of %d and produce an otherwise unused and unique name for the new
device. For example, if there is no tap device name given in the XML,
libvirt will always send "vnet%d" as the device name, and the kernel
will create new devices named "vnet0", "vnet1", etc. If one of those
devices is deleted, creating a "hole" in the name list, the kernel
will always attempt to reuse the name in the hole first before using a
name with a higher number (i.e. it finds the lowest possible unused
number).
The problem with this, as described in the previous patch dealing with
macvtap device naming, is that it makes "immediate reuse" of a newly
freed tap device name *much* more common, and in the aftermath of
deleting a tap device, there is some other necessary cleanup of things
which are named based on the device name (nwfilter rules, bandwidth
rules, OVS switch ports, to name a few) that could end up stomping
over the top of the setup of a new device of the same name for a
different guest.
Since the kernel "create a name based on a template" functionality for
tap devices doesn't exist for macvtap, this patch for standard tap
devices is a bit different from the previous patch for macvtap - in
particular there was no previous "bitmap ID reservation system" or
overly-complex retry loop that needed to be removed. We simply find
and unused name, and pass that name on to the kernel instead of
"vnet%d".
This counter is also wrapped when either it gets to INT_MAX or if the
full name would overflow IFNAMSIZ-1 characters. In the case of
"vnet%d" and a 32 bit int, we would reach INT_MAX first, but possibly
someday someone will change the name from vnet to something else.
(NB: It is still possible for a user to provide their own
parameterized template name (e.g. "mytap%d") in the XML, and libvirt
will just pass that through to the kernel as it always has.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There have been some reports that, due to libvirt always trying to
assign the lowest numbered macvtap / tap device name possible, a new
guest would sometimes be started using the same tap device name as
previously used by another guest that is in the process of being
destroyed *as the new guest is starting.
In some cases this has led to, for example, the old guest's
qemuProcessStop() code deleting a port from an OVS switch that had
just been re-added by the new guest (because the port name is based on
only the device name using the port). Similar problems can happen (and
I believe have) with nwfilter rules and bandwidth rules (which are
both instantiated based on the name of the tap device).
A couple patches have been previously proposed to change the ordering
of startup and shutdown processing, or to put a mutex around
everything related to the tap/macvtap device name usage, but in the
end no matter what you do there will still be possible holes, because
the device could be deleted outside libvirt's control (for example,
regular tap devices are automatically deleted when the qemu process
terminates, and that isn't always initiated by libvirt but could
instead happen completely asynchronously - libvirt then has no control
over the ordering of shutdown operations, and no opportunity to
protect it with a mutex.)
But this only happens if a new device is created at the same time as
one is being deleted. We can effectively eliminate the chance of this
happening if we end the practice of always looking for the lowest
numbered available device name, and instead just keep an integer that
is incremented each time we need a new device name. At some point it
will need to wrap back around to 0 (in order to avoid the IFNAMSIZ 15
character limit if nothing else), and we can't guarantee that the new
name really will be the *least* recently used name, but "math"
suggests that it will be *much* less common that we'll try to re-use
the *most* recently used name.
This patch implements such a counter for macvtap/macvlan, replacing
the existing, and much more complicated, "ID reservation" system. The
counter is set according to whatever macvtap/macvlan devices are
already in use by guests when libvirtd is started, incremented each
time a new device name is needed, and wraps back to 0 when either
INT_MAX is reached, or when the resulting device name would be longer
than IFNAMSIZ-1 characters (which actually is what happens when the
template for the device name is "maccvtap%d"). The result is that no
macvtap name will be re-used until the host has created (and possibly
destroyed) 99,999,999 devices.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On some platforms libm (needed for the pow() function) isn't being
linked in somehow. This patch adds the necessary bits to assure that
it's linked in when necessary.
Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 20a62b42ec001310a6329d7ee2021f0737d534ef)
This patch takes care of just the obvious cases: there are
many more situations where the data we pass to configure_file()
could likely be obtained in a more effective way, but we can
address the low-hanging fruits as a first approximation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When editing a domain with hotplug enabled, I removed the only
NUMA node it had and got no error. I got the error later though,
when starting the domain. This is not as user friendly as it can
be. Move the validation call out from command line generator and
into domain validator (which is called prior to starting cmd line
generation anyway).
When doing this, I had to remove memory-hotplug-nonuma xml2xml
test case because there is no way the test case can succeed,
obviously.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There's no need to install sysconf files when init script installation
was not requested, i.e. when configured with init_script=none.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When using [virtiofs], libvirtd must launch [virtiofsd] to provide
filesystem access on the host. When a guest is configured with
virtiofs, such as:
<filesystem type='mount' accessmode='passthrough'>
<driver type='virtiofs'/>
<source dir='/path'/>
<target dir='mount_tag'/>
</filesystem>
Attempting to start the guest fails with:
internal error: virtiofsd died unexpectedly
/var/log/libvirt/qemu/$name-fs0-virtiofsd.log contains (as a single
line, wrapped below):
libvirt: error : cannot execute binary /usr/lib/qemu/virtiofsd:
Permission denied
dmesg contains (as a single line, wrapped below):
audit: type=1400 audit(1598229295.959:73): apparmor="DENIED"
operation="exec" profile="libvirtd" name="/usr/lib/qemu/virtiofsd"
pid=46007 comm="rpc-worker" requested_mask="x" denied_mask="x"
fsuid=0 ouid=0
To avoid this, allow execution of virtiofsd from the libvirtd AppArmor
profile.
[virtiofs]: https://libvirt.org/kbase/virtiofs.html
[virtiofsd]: https://www.qemu.org/docs/master/interop/virtiofsd.html
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Driver module loaders current hardcode ".so" as the file
extension. On MacOS, meson uses ".dylib" as a module file extension.
This patch adds VIR_FILE_MODULE_EXT to virfile.h defined as the
hosts module extension, and updates driver module loaders to make
use of it.
Signed-off-by: Scott Shambarger <scott-libvirt@shambarger.net>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Previous patch handled the runtime case where a non-x86 host is
fetching /proc/cpuinfo data for a microcode info that we know
it doesn't exist. This change alone speeded everything by a
bit for non-x86, but there is at least one major culprit left.
qemuxml2argvtest does several arch-specific tests, and a good
chunk of them are x86 exclusive. This means that 'hostArch'
will be seen as x86 for these tests, even when running in
non-x86 hosts. In a Power 9 server with 128 CPUs, qemuxml2argvtest
takes 298 seconds to complete in average, and 'perf record'
indicates that 95% of the time is spent in
virHostCPUGetMicrocodeVersion().
This patch mocks virHostCPUGetMicrocodeVersion() to always return
0 in the tests, avoiding /proc/cpuinfo reads. This will make all
tests behave arch-agnostic, and the microcode value being 0 has no
impact on any existing test.
This is a CI speed across the board for all archs, including x86,
given that we're not reading /proc/cpuinfo in the tests. For
a Thinkpad T480 laptop with 8 Intel i7 CPUs, qemuxml2argvtest
went from 15.50 sec to 12.50 seconds. The performance gain is even
more noticeable for huge servers with lots of CPUs. For the
Power 9 server mentioned above, this patch speeds qemuxml2argvtest
to 9 seconds, down from 298 sec.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Non-x86 archs does not have a 'microcode' version like x86. This is
covered already inside the function - just return 0 if no microcode
is found. Regardless of that, a read of /proc/cpuinfo is always made.
Each read will invoke the kernel to fill in the CPU details every time.
Now let's consider a non-x86 host, like a Power 9 server with 128 CPUs.
Each /proc/cpuinfo read will need to fetch data for each CPU and it
won't even matter because we know beforehand that PowerPC chips don't
have microcode information.
We can do better for non-x86 hosts by skipping this process entirely.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autofree and remove the cleanup label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Since the macro no longer includes the 'ignore_value'
statement, stop putting another empty statement after it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The VIR_XPATH_NODE_AUTORESTORE contains an ignore_value
statement to silence an unused variable warning on clang.
Use a pragma instead, which is not a statement.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_CGROUP_BACKEND_CALL is exclusively used at the end
of a function, but it declares a variable.
Wrap it in a do..while block.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Declare the variables at the beginning of the function,
then fill them up.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Declare it at the beginning of the function
instead of right before use.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Many of our functions start with a DEBUG statement.
Move the statements after declarations to appease
our coding style.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Split those initializations that depend on a statement
above them.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and move the declarations to the beginning
of the block.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Repeat the whole function header instead of mixing #ifdefs
in the code.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Support qemu commandline passthrough in the domXML to native config
converter. Add tests to check the conversion.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Xen supports passing arbitrary arguments to the QEMU device model via
the 'extra' member of the public libxl_domain_build_info structure.
This patch adds a 'xen' namespace extension, similar to the QEMU and
bhyve drivers, to map arbitrary arguments to the 'extra' member. Only
passthrough of arguments is supported. Passthrough of environment
variables or capabilities adjustments is not supported.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9> introduced a support
to fully allocate qcow2 images when <allocation> matches <capacity> but
it doesn't work as expected.
The issue is that info.size_arg is in KB but the info.allocation
introduced by the mentioned commit is in B. This results in using
"preallocation=falloc," in cases where "preallocation=metadata," should
be used.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These leaks were introduced in commit 15d280fa97, use g_autofree for all
cert_path pointers.
Signed-off-by: Jin Yan <jinyan12@huawei.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use virJSONValueObjectCreate rather than creating the object
piece-by-piece and use new accessors for bitmap to simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Allow to map sound playback and recording devices to host devices
using "<audio type='oss'/>" OSS audio backend.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a new device element "<audio>" which allows
to map guest sound device specified using the "<sound>"
element to specific audio backend.
Example:
<sound model='ich7'>
<audio id='1'/>
</sound>
<audio id='1' type='oss'>
<input dev='/dev/dsp0'/>
<output dev='/dev/dsp0'/>
</audio>
This block maps to OSS audio backend on the host using
/dev/dsp0 device for both input (recording)
and output (playback).
OSS is the only backend supported so far.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
bhyve supports intel hda sound devices that could be specified
on the command like using "-1:0,hda,play=$play_dev,rec=$rec_dev",
where "1:0" is a PCI address, and "$play_dev" and "$rec_dev"
point to the playback and recording device on the host respectively.
Currently, schema of the 'sound' element doesn't allow specifying
neither playback nor recording devices, so for now hardcode
/dev/dsp0, which is the first audio device on the host.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add 'ich7' sound model. This is a preparation for sound support in
bhyve, as 'ich7' is the only model it supports.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Back when macvtap support was added in commit 315baab944 in Feb. 2010
(libvirt-0.7.7), it was setup to autogenerate a name for the device if
one wasn't supplied, in the pattern "macvtap%d" (or "macvlan%d"),
similar to the way an unspecified standard tap device name will lead
to an autogenerated "vnet%d".
As a matter of fact, in commit ca1b7cc8e4 added in May 2010, the code
was changed to *always* ignore a supplied device name for macvtap
interfaces by deleting *any* name immediately during the <interface>
parsing (this was intended to prevent one domain which had failed to
completely start from deleting the macvtap device of another domain
which had subsequently been provided the same device name (this will
seem mildly ironic later). This was later fixed to only clear the
device name when inactive XML was being parsed. HOWEVER - this was
only done if the xml was <interface type='direct'> - autogenerated
names were not cleared for <interface type='network'> (which could
also result in a macvtap device).
Although the names of "vnetX" tap devices had always been
automatically cleared when parsing <interface> (see commit d1304583d
from July 2008 (!)), at the time macvtap support was added, both vnetX
and macvtapX device names were always included when formatting the
XML.
Then in commit a8be259d0c (July 2011, libvirt-0.9.4), <interface>
formatting was changed to also clear out "vnetX" device names during
XML formatting as well. However the same treatment wasn't given to
"macvtapX".
Now in 2020, there has been a report that a failed migration leads to
the macvtap device of some other unrelated guest on the destination
host losing its network connectivity. It was determined that this was
due to the domain XML in the migration containing a macvtap device
name, e.g. "macvtap0", that was already in use by the other guest on
the destination. Normally this wouldn't be a problem, because libvirt
would see that the device was already in use, and then find a
different unused name. But in this case, other external problems were
causing the migration to fail prior to selecting a macvtap device and
successfully opening it, and during error recovery, qemuProcessStop()
was called, which went through all def->nets objects and (if they were
macvtap) deleted the device specified in net->ifname; since libvirt
hadn't gotten to the point of replacing the incoming "macvtap0" with
the name of a device it actually created for this guest, that meant
that "macvtap0" was deleted, *even though it was currently in use by a
different guest*!
Whew!
So, it turns out that when formatting "migratable" XML, "vnetX"
devices are omitted, just as when formatting "inactive" XML. By making
the code in both interface parsing and formatting consistent for
"vnetX", "macvtapX", and "macvlanX", we can thus make sure that the
autogenerated (and unneeded / completely *not* wanted) macvtap device
name will not be sent with the migration XML. This way when a
migration fails, net->ifname will be NULL, and libvirt won't have any
device to try and (erroneously) delete.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Back when the original version of this chunk of code was added (commit
41b087198 in libvirt-0.8.1 in April 2010), we used virExecDaemonize()
to start the qemu process, and would continue on in the function
(which at that time was called qemudStartVMDaemon()) even if a -1 was
returned. So it was possible to get to this code with rv == -1 (it was
called "ret" in that version of the code).
In modern libvirt code, qemu is started with virCommandRun(); then we
call virPidFileReadPath(); those are the only two ways of setting "rv"
prior to this code being removed, and in either case if the new value
of rv < 0, then we immediately skip over the rest of the code to the
cleanup: label.
This means that the code being removed by this patch is
unreachable.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Even if namespaces are disabled, then due to a missing check at the
beginning of qemuDomainBuildNamespace(), the domain startup code
still tries to populate (nonexistent) domain's namespace.
Fixes: 8da362fe62
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
After the recent changes, this function is now always returning
zero. Turn it to 'void' to relieve callers from checking it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't need the auto-alignment now that the user is handling
it by hand.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The existing auto-align behavior for pSeries has the idea to
alleviate user configuration of the NVDIMM size, given that the
alignment calculation is not trivial to do (256MiB alignment
of mem->size - mem->label_size value, a.k.a guest area). We
align mem->size down to avoid end of file problems.
The end result is not ideal though. We do not touch the domain
XML, meaning that the XML can report a NVDIMM size 255MiB smaller
than the actual size the guest is seeing. It also adds one more
thing to consider in case the guest is reporting less memory
than declared, since the auto-align is transparent to the
user.
Following Andrea's suggestion in [1], let's instead do an
size alignment validation. If the NVDIMM is unaligned, error out
and suggest a rounded up value. This can be bothersome to users,
but will bring consistency of NVDIMM size between the domain XML
and the guest.
This approach will force existing non-running pSeries guests to
readjust the NVDIMM value in their XMLs, if necessary. No changes
were made for x86 NVDIMM support.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Suggested-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Next patch will use it outside of qemu_domain.c.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function is not changing the domain definition, it's only
reading from it. The function is going to be used from another
function which already takes const virDomainDef. Make the @def
const to avoid typecasting it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 4362068979 moved the function to
util/virqemu.c which is compiled also on win32 and geteuid()/getegid()
doesn't exist there.
Move it to qemu_domain.c which is compiled only when the qemu driver is
enabled. Originally I didn't want to put it here as qemu_domain.c is a
code dump for helper functions but this is the least invasive fix.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We've dumped all the snapshot helpers and related code into
qemu_driver.c. It accounted for ~10% of overal size of qemu_driver.c.
Separate the code to qemu_snapshot.c/h.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There's a lot of helper code related to the save image handling. Extract
it to qemu_saveimage.c/h.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code to qemu_domain.c so that it can be reused in other parts
of the qemu driver. 'qemu_domain' was chosen as we check the domain
state after closing the wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code to qemu_domain.c so that it can be reused in other parts
of the qemu driver. 'qemu_domain' was chosen as the permissions are
based on the domain configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory allocation to simplify the code and remove the need
for a 'cleanup:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'cleanup:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'error:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'error:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function doesn't have an overly verbose cleanup section as there
isn't any error code path. Unify it with the rest of the functions which
will simplify adding a possible error path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is similar to one of previous patches.
When receiving stream (on virStorageVolUpload() and subsequent
virStreamSparseSendAll()) we may receive a hole. If the volume we
are saving the incoming data into is a regular file we just
lseek() and ftruncate() to create the hole. But this won't work
if the file is a block device. If that is the case we must write
zeroes so that any subsequent reader reads nothing just zeroes
(just like they would from a hole in a regular file).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When handling sparse stream, a thread is executed. This thread
runs a read() or write() loop (depending what API is called; in
this case it's virStorageVolDownload() and this the thread run
read() loop). The read() is handled in virFDStreamThreadDoRead()
which is then data/hole section aware, meaning it uses
virFileInData() to detect data and hole sections and sends
TYPE_DATA or TYPE_HOLE virStream messages accordingly.
However, virFileInData() does not work with block devices. Simply
because block devices don't have data and hole sections. What we
can do though, is to mimic being always in a DATA section.
Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For libvirt, the volume is just a binary blob and it doesn't
interpret data on volume upload/download. But as it turns out,
this unspoken assumption is not clear to our users. Document it
explicitly.
Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Right now we're unconditionally adding RPATH information to the
installed binaries and libraries, but that's not always desired.
autotools seem to be smart enough to only include that information
when targeting a non-standard prefix, so most distro packages
don't actually contain it; moreover, both Debian and Fedora have
wiki pages encouraging packagers to avoid setting RPATH:
https://wiki.debian.org/RpathIssuehttps://fedoraproject.org/wiki/RPath_Packaging_Draft
Implement RPATH logic that Does The Right Thing™ in the most
common cases, while still offering users the ability to override
the default behavior if they have specific needs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In a very distant past, we came around machines that has not
continuous node IDs. This made us error out when constructing
capabilities XML. We resolved it by utilizing strange behaviour
of numa_node_to_cpus() in which it returned a mask with all bits
set for a non-existent node. However, this is not the only case
when it returns all ones mask - if the node exists and has enough
CPUs to fill the mask up (e.g. 128 CPUs).
The fix consists of using nodemask_isset(&numa_all_nodes, ..)
prior to calling numa_node_to_cpus() to determine if the node
exists.
Fixes: 628c935747
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1860231
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There was a report on libvirt-users [1] about the domxml to/from
native converter in the Xen driver not handling PCI addresses
without a domain specification. This patch improves parsing of PCI
addresses in the converter and allows PCI addresses with only
bb:ss.f. xl.cfg(5) also allows either the dddd:bb:ss.f or bb:ss.f
format. A test has been added to check the conversion from xl.cfg
to domXML.
[1] https://www.redhat.com/archives/libvirt-users/2020-August/msg00040.html
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
After previous cleanups, some labels in some functions have
nothing but 'return' statement in them. Drop the labels and
replace 'goto'-s with respective return statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Again, instead of closing FDs explicitly, we can automatically
close them when they go out of their respective scopes.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
A cleanup function can be declared for virFDStreamMsg type so
that the structure doesn't have to be freed explicitly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All callers of virFDStreamMsgQueuePush() have the same pattern:
they explicitly set @msg passed to NULL to avoid freeing it later
on. Well, the function can take address of the pointer and clear
it for them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The buffer that allocated in the virFDStreamThreadDoRead() can be
automatically freed, or if saved into the message structure it
can be stolen.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far, only ENOENT is ignored (to deal with kernels without
devmapper). However, as reported on the list, under certain
scenarios a different error can occur. For instance, when libvirt
is running inside a container which doesn't have permissions to
talk to the devmapper. If this is the case, then open() returns
-1 and sets errno=EPERM.
Assuming that multipath devices are fairly narrow use case and
using them in a restricted container is even more narrow the best
fix seems to be to ignore all open errors BUT produce a warning
on failure. To avoid flooding logs with warnings on kernels
without devmapper the level is reduced to a plain debug message.
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When adding support for HMAT, in f0611fe883 I've introduced a
check which aims to validate /domain/cpu/numa/interconnects. As a
part of that, there is a loop which checks whether all <latency/>
with @cache attribute refer to an existing cache level. For
instance:
<cpu mode='host-model' check='partial'>
<numa>
<cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
<cache level='1' associativity='direct' policy='writeback'>
<size value='8' unit='KiB'/>
<line value='5' unit='B'/>
</cache>
</cell>
<interconnects>
<latency initiator='0' target='0' cache='1' type='access' value='5'/>
<bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
</interconnects>
</numa>
</cpu>
This XML defines that accessing L1 cache of node #0 from node #0
has latency of 5ns.
However, the loop was not written properly. Well, the check in
it, as it was always checking for the first cache in the target
node and not the rest. Therefore, the following example errors
out:
<cpu mode='host-model' check='partial'>
<numa>
<cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
<cache level='3' associativity='direct' policy='writeback'>
<size value='10' unit='KiB'/>
<line value='8' unit='B'/>
</cache>
<cache level='1' associativity='direct' policy='writeback'>
<size value='8' unit='KiB'/>
<line value='5' unit='B'/>
</cache>
</cell>
<interconnects>
<latency initiator='0' target='0' cache='1' type='access' value='5'/>
<bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
</interconnects>
</numa>
</cpu>
This errors out even though it is a valid configuration. The L1
cache under node #0 is still present.
Fixes: f0611fe883
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Both parsing and formatting of NBD migration jobs is QEMU specific and
since we're trying to create a hypervisor-agnostic module out of
qemu_domainjob.c, move the NBD XML handling bits to the qemu_domain
module instead. Additionally, move the respective NBD XML calls to
the 'parseJob'/'formatJob' callbacks of the
qemuDomainObjPrivateJobCallbacks structure.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Functions `qemuDomainRemoveInactiveJob` and
`qemuDomainRemoveInactiveJobLocked` had their declaration misplaced in
`qemu_domainjob` and were moved to `qemu_domain` where their definitions
reside.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Most of our augeas files are generated during meson setup into build
directory and we were running augeas tests only for these files.
However, we have some other augeas and config files that are not
modified during meson setup and they are only in source directories.
In order to run tests for these files we need to provide different path
to both source and build directories.
Reported-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This will be used later to specify different include directories for
augparse binary to run augeas tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case qemuDumpToFd() returns zero followed by a VIR_CLOSE(fd) fail,
we'd jump to the "cleanup" label with "ret=0", potentially resulting in
an unexpected success return value.
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In one of my latest patch (v6.6.0~30) I was trying to remove
libdevmapper use in favor of our own implementation. However, the
code did not take into account that device mapper can be not
compiled into the kernel (e.g. be a separate module that's not
loaded) in which case /proc/devices won't have the device-mapper
major number and thus virDevMapperGetTargets() and/or
virIsDevMapperDevice() fails.
However, such failure is safe to ignore, because if device mapper
is missing then there can't be any multipath devices and thus we
don't need to allow the deps in CGroups, nor create them in the
domain private namespace, etc.
Fixes: 2249455654
Reported-by: Andrea Bolognani <abologna@redhat.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The device mapper major is needed in virIsDevMapperDevice() which
determines whether given device is managed by device-mapper. This
number is obtained by parsing /proc/devices and then stored in a
global variable so that the file doesn't have to be parsed again.
However, as it turns out this logic is flawed - the major number
is not static and can change as it can be specified as a
parameter when loading the dm-mod module.
Unfortunately, I was not able to come up with a good solution and
thus the /proc/devices file is being parsed every time we need
the device mapper major.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Change the 'make check' reference after the switch to meson/ninja.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
GCC 10 complains about variables may be used uninitialized.
Even though it might be false positives, we can easily avoid them.
Avoiding
../src/storage/storage_backend_iscsi_direct.c:634:11: error: ‘nb_block’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
634 | while (lba < nb_block) {
| ^
../src/storage/storage_backend_iscsi_direct.c:619:14: note: ‘nb_block’ was declared here
619 | uint64_t nb_block;
| ^~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:637:16: error: ‘block_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
637 | task = iscsi_write16_sync(iscsi, lun, lba, data,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
638 | block_size * to_write,
| ~~~~~~~~~~~~~~~~~~~~~~
639 | block_size, 0, 0, 0, 0, 0);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:618:14: note: ‘block_size’ was declared here
618 | uint32_t block_size;
| ^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c: In function ‘virStorageBackendISCSIDirectRefreshPool’:
../src/storage/storage_backend_iscsi_direct.c:320:39: error: ‘nb_block’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
320 | vol->target.capacity = block_size * nb_block;
| ~~~~~~~~~~~^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:306:14: note: ‘nb_block’ was declared here
306 | uint64_t nb_block;
| ^~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:320:39: error: ‘block_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
320 | vol->target.capacity = block_size * nb_block;
| ~~~~~~~~~~~^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:305:14: note: ‘block_size’ was declared here
305 | uint32_t block_size;
| ^~~~~~~~~~
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
GCC 10 complains about "well_formed_uri" may be used uninitialzed.
Even though it is a false positive, we can easily avoid it.
Avoiding
../src/qemu/qemu_migration.c: In function ‘qemuMigrationDstPrepareDirect’:
../src/qemu/qemu_migration.c:2920:16: error: ‘well_formed_uri’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
2920 | if (well_formed_uri) {
| ^
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
sc_spacing-check FAIL reporting a case of "Curly brackets around
single-line body:" in a recent commit.
Fixes: d9c21f4b "apparmor: allow adding permanent per guest rules"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
With qemu 5.0 and libvirt 6.6 there are new apparmor denials:
apparmor="DENIED" operation="umount" profile="libvirtd"
name="/run/libvirt/qemu/1-kvmguest-groovy-norm.dev/" comm="rpc-worker"
These are related to new issues around devmapper handling [1] and the
error path triggered by these issues now causes this new denial.
There are already related rules for mounting and it seems right to
allow also the related umount.
[1]: https://www.redhat.com/archives/libvir-list/2020-August/msg00236.html
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The design of apparmor in libvirt always had a way to define custom
per-guest rules as described in docs/drvqemu.html and [1].
A fix meant to clean the profiles after guest shutdown was a bit
overzealous and accidentially removed this important admin feature as
well.
Therefore reduce the --delete option of virt-aa-helper to only delete
the .files that would be re-generated in any case.
Users/Admins are always free to clean the profiles themselve if they
prefer a clean directory - they will be regenerated as needed. But
libvirt should never remove the base profile meant to allow per-guest
overrides and thereby break a documented feature.
[1]: https://gitlab.com/apparmor/apparmor/-/wikis/Libvirt#advanced-usage
Fixes: eba2225b "apparmor: delete profile on VM shutdown"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If we don't have cgroups available and user tries to update blkio
parameters for running VM it will crash.
It should have been protected by the virCgroupHasController() check but
it was never called if the API was executed without any flags.
We call virDomainObjGetDefs() which sets `def` and `persistentDef` based
on the flags and these two variables should be used to figure out if we
need to update LIVE, CONFIG or both states.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808293
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
src/bhyve/bhyve_parse_command.c:437:9: warning: Either the condition
'!config' is redundant or there is possible null pointer dereference:
config. [nullPointerRedundantCheck]
src/bhyve/bhyve_parse_command.c:280:23: warning: Either the condition
'!separator' is redundant or there is pointer arithmetic
with NULL pointer. [nullPointerArithmeticRedundantCheck]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
BPF syscall BPF_MAP_GET_NEXT_KEY returns -1 if something fails but it
will also return -1 if trying to get next key using the last key in the
map with errno set to ENOENT.
If there are VMs running and libvirtd is restarted and user tries to
call some cgroup devices operation on a VM we need to get the count of
entries in BPF map and it fails which will result in error when trying
to attach/detech devices.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1833321
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since [1] qemu can after upgrade fall back to pre-upgrade modules
to still be able to dynamically load qemu-module based features.
The paths for these modules are pre-defined by the code and should
be allowed to be mapped and loaded from which will allow packagers
avoiding the inability of late feature load [2] after package upgrades.
[1]: https://github.com/qemu/qemu/commit/bd83c861
[2]: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange redhat com>
On some architectures (ppc, s390x, sparc, arm) qemu will read auxv
to detect hardware capabilities via qemu_getauxval.
Allow that access read-only for the entry owned by the current
qemu process.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Allow qemu to read @{PROC}/sys/vm/overcommit_memory.
This is read on guest start-up and (as read-only) not a
critical secret that has to stay hidden.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Jamie Strandboge <jamie@ubuntu.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When using xen through libxl in Debian/Ubuntu it needs to be able to
call pygrub.
This is placed in a versioned path like /usr/lib/xen-4.11/bin.
In theory the rule could be more strict by rendering the libexec_dir
setting pkg-config can derive from libbxen-dev. But that would make
particular libvirt/xen packages version-depend on each other. It seems
more reasonable to avoid these versioned dependencies and use a wildcard
rule instead as it is already in place for libxl-save-helper.
Note: This change was in Debian [1] and Ubuntu [2] for quite some time
already.
[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=931768
[2]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1326003
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
/etc/pki/qemu is a pki path recommended by qemu tls docs [1]
and one that can cause issues with spice connections when missing.
Add the path to the allowed list of pki paths to fix the issue.
Note: this is active in Debian/Ubuntu [1] for quite a while already.
[1]: https://www.qemu.org/docs/master/system/tls.html
[2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930100
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Following the rationale from commit
<2020c6af8a8e4bb04acb629d089142be984484c8> we should do the same thing
for iothread info as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit <6328da04285d9f65cb323d399f731c20caf63f5a> introduced
testDomainGetEmulatorPinInfo() into test driver but used
virHostCPUGetCount() function to get the number of host CPUs.
This would be correct for other drivers but in test driver we must not
depend on the host, we have to use hard-coded host representation that
we have in test driver.
Follows the logic of testDomainGetVcpuPinInfo().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit <2020c6af8a8e4bb04acb629d089142be984484c8> fixed an issue with
QEMU driver by reporting offline CPUs as well. However, doing so it
introduced a regression into libxl and test drivers by completely
ignoring the passed `hostcpus` variable.
Move the virHostCPUGetAvailableCPUsBitmap() out of the helper into QEMU
driver so it will not affect other drivers which gets the number of host
CPUs differently.
This was uncovered by running libvirt-dbus test suite which counts on
the fact that test driver has hard-coded host definition and must not
depend on the host at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is not expose in most historical versions of glibc, nor
non-glibc impls. We must use our wrapper API instead.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If a Ctrl-C arrives while we are in the middle of executing the
virDomainCreateXML call, we will have no "virDomainPtr" object
available, but QEMU may none the less be running.
This means we'll never try to stop the QEMU process before we
honour the Ctrl-C and exit.
To deal with this race we need to postpone quit of the event
loop if it is requested while in the middle of domain startup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a race between vir_event_thread_finalize and
virEventThreadWorker in releasing the last reference on
the GMainContext. If virEventThreadDataFree() runs after
vir_event_thread_finalize releases its reference, then
it will release the last reference on the GMainContext.
As a result g_autoptr cleanup on the GSource will access
free'd memory.
The race can be seen in non-deterministic crashes of the
virt-run-qemu program during its shutdown, but could
also likely affect the main libvirtd QEMU driver:
Thread 2 (Thread 0x7f508ffff700 (LWP 222813)):
#0 0x00007f509c8e26b0 in malloc_consolidate (av=av@entry=0x7f5088000020) at malloc.c:4488
#1 0x00007f509c8e4b08 in _int_malloc (av=av@entry=0x7f5088000020, bytes=bytes@entry=2048) at malloc.c:3711
#2 0x00007f509c8e6412 in __GI___libc_malloc (bytes=2048) at malloc.c:3073
#3 0x00007f509d6e925e in g_realloc (mem=0x0, n_bytes=2048) at gmem.c:164
#4 0x00007f509d705a57 in g_string_maybe_expand (string=string@entry=0x7f5088001f20, len=len@entry=1024) at gstring.c:102
#5 0x00007f509d705ab6 in g_string_sized_new (dfl_size=dfl_size@entry=1024) at gstring.c:127
#6 0x00007f509d708c5e in g_test_log_dump (len=<synthetic pointer>, msg=<synthetic pointer>) at gtestutils.c:3330
#7 0x00007f509d708c5e in g_test_log
(lbit=G_TEST_LOG_ERROR, string1=0x7f508800fcb0 "GLib:ERROR:ghash.c:377:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > 0)", string2=<optimized out>, n_args=0, largs=0x0) at gtestutils.c:975
#8 0x00007f509d70af2a in g_assertion_message
(domain=<optimized out>, file=0x7f509d7324a2 "ghash.c", line=<optimized out>, func=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", message=<optimized out>)
at gtestutils.c:2504
#9 0x00007f509d70af8e in g_assertion_message_expr
(domain=domain@entry=0x7f509d72d76e "GLib", file=file@entry=0x7f509d7324a2 "ghash.c", line=line@entry=377, func=func@entry=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", expr=expr@entry=0x7f509d732488 "hash_table->ref_count > 0") at gtestutils.c:2555
#10 0x00007f509d6d197e in g_hash_table_lookup_node (hash_table=0x55b70ace1760, key=<optimized out>, hash_return=<synthetic pointer>) at ghash.c:377
#11 0x00007f509d6d197e in g_hash_table_lookup_node (hash_return=<synthetic pointer>, key=<optimized out>, hash_table=0x55b70ace1760) at ghash.c:361
#12 0x00007f509d6d197e in g_hash_table_remove_internal (hash_table=0x55b70ace1760, key=<optimized out>, notify=1) at ghash.c:1371
#13 0x00007f509d6e0664 in g_source_unref_internal (source=0x7f5088000b60, context=0x55b70ad87e00, have_lock=0) at gmain.c:2103
#14 0x00007f509d6e1f64 in g_source_unref (source=<optimized out>) at gmain.c:2176
#15 0x00007f50a08ff84c in glib_autoptr_cleanup_GSource (_ptr=<synthetic pointer>) at /usr/include/glib-2.0/glib/glib-autocleanups.h:58
#16 0x00007f50a08ff84c in virEventThreadWorker (opaque=0x55b70ad87f80) at ../../src/util/vireventthread.c:114
#17 0x00007f509d70bd4a in g_thread_proxy (data=0x55b70acf3850) at gthread.c:784
#18 0x00007f509d04714a in start_thread (arg=<optimized out>) at pthread_create.c:479
#19 0x00007f509c95cf23 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Thread 1 (Thread 0x7f50a1380c00 (LWP 222802)):
#0 0x00007f509c8977ff in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f509c881c35 in __GI_abort () at abort.c:79
#2 0x00007f509d72a823 in g_mutex_clear (mutex=0x55b70ad87e00) at gthread-posix.c:1307
#3 0x00007f509d72a823 in g_mutex_clear (mutex=mutex@entry=0x55b70ad87e00) at gthread-posix.c:1302
#4 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:582
#5 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:541
#6 0x00007f50a08ffabb in vir_event_thread_finalize (object=0x55b70ad83180 [virEventThread]) at ../../src/util/vireventthread.c:50
#7 0x00007f509d9c48a9 in g_object_unref (_object=<optimized out>) at gobject.c:3340
#8 0x00007f509d9c48a9 in g_object_unref (_object=0x55b70ad83180) at gobject.c:3232
#9 0x00007f509583d311 in qemuProcessQMPFree (proc=proc@entry=0x55b70ad87b90) at ../../src/qemu/qemu_process.c:8355
#10 0x00007f5095790f58 in virQEMUCapsInitQMPSingle
(qemuCaps=qemuCaps@entry=0x55b70ad88010, libDir=libDir@entry=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=runUid@entry=107, runGid=runGid@entry=107, onlyTCG=onlyTCG@entry=false) at ../../src/qemu/qemu_capabilities.c:5409
#11 0x00007f509579108f in virQEMUCapsInitQMP (runGid=107, runUid=107, libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", qemuCaps=0x55b70ad88010)
at ../../src/qemu/qemu_capabilities.c:5420
#12 0x00007f509579108f in virQEMUCapsNewForBinaryInternal
(hostArch=VIR_ARCH_X86_64, binary=binary@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=107, runGid=107, hostCPUSignature=0x55b70ad01320 "GenuineIntel, Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz, family: 6, model: 85, stepping: 7", microcodeVersion=83898113, kernelVersion=0x55b70ad00d60 "4.18.0-211.el8.x86_64 #1 SMP Thu Jun 4 08:08:16 UTC 2020") at ../../src/qemu/qemu_capabilities.c:5472
#13 0x00007f5095791373 in virQEMUCapsNewData (binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", privData=0x55b70ad5b8f0) at ../../src/qemu/qemu_capabilities.c:5505
#14 0x00007f50a09a32b1 in virFileCacheNewData (name=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", cache=<optimized out>) at ../../src/util/virfilecache.c:208
#15 0x00007f50a09a32b1 in virFileCacheValidate (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", data=data@entry=0x7ffca39ffd90)
at ../../src/util/virfilecache.c:277
#16 0x00007f50a09a37ea in virFileCacheLookup (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/util/virfilecache.c:310
#17 0x00007f5095791627 in virQEMUCapsCacheLookup (cache=0x55b70ad5c030, binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/qemu/qemu_capabilities.c:5647
#18 0x00007f50957c34c3 in qemuDomainPostParseDataAlloc (def=<optimized out>, parseFlags=<optimized out>, opaque=<optimized out>, parseOpaque=0x7ffca39ffe18)
at ../../src/qemu/qemu_domain.c:5470
#19 0x00007f50a0a34051 in virDomainDefPostParse
(def=def@entry=0x55b70ad7d200, parseFlags=parseFlags@entry=258, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0)
at ../../src/conf/domain_conf.c:5970
#20 0x00007f50a0a464bb in virDomainDefParseNode
(xml=xml@entry=0x55b70aced140, root=root@entry=0x55b70ad5f020, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22520
#21 0x00007f50a0a4669b in virDomainDefParse
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., filename=filename@entry=0x0, xmlopt=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258) at ../../src/conf/domain_conf.c:22474
#22 0x00007f50a0a467ae in virDomainDefParseString
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., xmlopt=<optimized out>, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22488
#23 0x00007f50958ce112 in qemuDomainCreateXML
(conn=0x55b70acf9090, xml=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/qemu/qemu_driver.c:1744
#24 0x00007f50a0c268ac in virDomainCreateXML
(conn=0x55b70acf9090, xmlDesc=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/libvirt-domain.c:176
#25 0x000055b709547e7b in main (argc=<optimized out>, argv=<optimized out>) at ../../src/qemu/qemu_shim.c:289
The solution is to explicitly unref the GSource at a safe time instead
of letting g_autoptr unref it when leaving scope.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a fairly long standing race condition bug in glib which can hit
if you call g_source_destroy or g_source_unref from a non-main thread:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358
Unfortunately it is really common for libvirt to call g_source_destroy
from a non-main thread. This glib bug is the cause of non-determinstic
crashes in eventtest, and probably in libvirtd too.
To work around the problem we need to ensure that we never release
the last reference on a GSource from a non-main thread. The previous
patch replaced our use of g_source_destroy with a pair of
g_source_remove and g_source_unref. We can now delay the g_source_unref
call by using a idle callback to invoke it from the main thread which
avoids the race condition.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When COW is not explicitly requested to be disabled or enabled, the
function is supposed to do nothing on non-BTRFS file systems.
Fixes commit 7230bc95aa.
https://bugzilla.redhat.com/show_bug.cgi?id=1866157
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
and stop erroneously equating NULL with "". The latter means that the
element has empty content, while the former means there was an error
during parsing (either internal with the parser, or the content of the
XML was bad).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Many of our calls to xmlNodeGetContent() (which are now all via
virXMLNodeContentString() are failing to check for a NULL return. We
need to remedy that, but in order to make the remedy simpler, let's
log an error in virXMLNodeContentString(), so that the callers don't
all individually need to (since it would be the same error message for
all of them anyway).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainBlkioDeviceParseXML() calls xmlNodeGetContent() multiple
times in a loop, but can easily be refactored to call it once for all
element nodes, and then use the result of that one call in each of the
(mutually exclusive) blocks that previously each had their own call to
xmlNodeGetContent.
This is being done in order to reduce the number of changes needed in
an upcoming patch that will eliminate the lack of checking for NULL on
return from xmlNodeGetContent().
As part of the simplification, the while() loop has been changed into
a for() so that we can use "continue" without bypassing the
"node = node->next".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We should prevent inlining of symbols from the driver .so files that are
mocked, as well as those in the main libvirt.so
This isn't fixing any currently known problem, just trying to prevent
future issues.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We already allow controlling the initiator IQN for iSCSI based disks.
Add the same for host devices.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Whenever libvirt is upgraded on a Debian system, the user will be
prompted along the lines of
Configuration file '/etc/libvirt/qemu/networks/default.xml'
==> Modified (by you or by a script) since installation.
==> Package distributor has shipped an updated version.
What would you like to do about it ? Your options are:
Y or I : install the package maintainer's version
N or O : keep your currently-installed version
D : show the differences between the versions
Z : start a shell to examine the situation
The default action is to keep your current version.
*** default.xml (Y/I/N/O/D/Z) [default=N] ? d
--- /etc/libvirt/qemu/networks/default.xml 2020-08-04 12:57:25.450911143 +0200
+++ /etc/libvirt/qemu/networks/default.xml.dpkg-new 2020-08-03 22:47:15.000000000 +0200
@@ -1,19 +1,11 @@
-<!--
-WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
-OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
- virsh net-edit default
-or other application using the libvirt API.
--->
-
<network>
<name>default</name>
- <uuid>612a2cab-72fb-416d-92bc-4d9e597bfb63</uuid>
- <forward mode='nat'/>
- <bridge name='virbr0' stp='on' delay='0'/>
- <mac address='52:54:00:1f:03:79'/>
- <ip address='192.168.122.1' netmask='255.255.255.0'>
+ <uuid>d020b839-4379-492c-aa74-eab7365076e6</uuid>
+ <bridge name="virbr0"/>
+ <forward/>
+ <ip address="192.168.122.1" netmask="255.255.255.0">
<dhcp>
- <range start='192.168.122.2' end='192.168.122.254'/>
+ <range start="192.168.122.2" end="192.168.122.254"/>
</dhcp>
</ip>
</network>
The UUID situation should probably be handled the same way it is
in the spec file by stripping it, and in general we could behave
much better towards users, but one part of the diff that
immediately stands out is that some lines are highlighted not
because they are semantically different, but simply because they
use different types of quotes around attributes.
Since the canonical version of all libvirt XML documents (as
returned by the various vir*GetXMLDesc() APIs) as well as the
on-disk representations use single quotes, let's use the same
for configuration files we install as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If there's a list of mdevs to be assigned to a domain, but one of them
(NOT the first) is already assigned to a different domain we're going
to crash in the qemuProcessStop phase in
virMediatedDeviceListFindIndex, because some of the pointers in
mgr->activeMediatedHostdevs are dangling. This is due to
virMediatedDeviceListMarkDevices using cleanup instead of rollback when
we find out that a device is already taken.
Reproducer steps:
1. start vm1 with mdev1
2. start vm2 with mdev2, mdev1 (the order is important!)
Backtrace:
#0 0x0000ffffb8c36250 in strcmp
#1 0x0000ffffb9b80754 in virMediatedDeviceListFindIndex
#2 0x0000ffffb9b80870 in virMediatedDeviceListFind
#3 0x0000ffffb9c9e168 in virHostdevReAttachMediatedDevices
#4 0x0000ffff9949f724 in qemuHostdevReAttachMediatedDevices
#5 0x0000ffff9949f7f8 in qemuHostdevReAttachDomainDevices
#6 0x0000ffff994bcd70 in qemuProcessStop
#7 0x0000ffff994bf4e0 in qemuProcessStart
Signed-off-by: Binfeng Wu <wubinfeng@huawei.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The header log_manager.h doesn't use anything from log_protocol.h and
the only other place than logging using log_protocol.h is qemu_command.c
where we can include log_protocol.h directly to have enum value
VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE available.
Fixes race-condition compilation error with meson:
In file included from ../tests/qemuhotplugmock.c:21:
In file included from ../src/qemu/qemu_hotplug.h:25:
In file included from ../src/qemu/qemu_domain.h:42:
../src/logging/log_manager.h:25:10: fatal error: 'logging/log_protocol.h' file not found
^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Reported-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
During the switch to meson, one of the patches mistakenly changed the
runtime socket prefix for {libvirtd, virtproxyd} to "libvirtd-" from
the original "libvirt-". Not to be mistaken with the systemd unit name
which actually follows the daemon name, IOW the systemd unit name
remains as e.g. "libvirtd.socket", but the actual unix socket created
on the filesystem that the daemon binds to must be named "libvirt-sock"
and not "libvirtd-sock".
Fixes: dd4f2c73ad
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can use qemuDomainSetupInput() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupRNG() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupChardev() to obtain the path that we
need to unlink() from within domain's namespace. Note, while
previously we unlinked only VIR_DOMAIN_CHR_TYPE_DEV chardevs,
with this change we unlink some other types too - exactly those
types we created when plugging the device in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupMemory() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In my attempt to deduplicate the code, we can use
qemuDomainSetupHostdev() to obtain the list of paths to unlink
and then pass it to qemuDomainNamespaceUnlinkPaths() to unlink
them in a single fork.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, the only caller qemuDomainNamespaceUnlinkPath() will
always pass a single path to unlink, but similarly to
qemuDomainNamespaceMknodPaths() - there are a few callers that
would like to pass two or more files to unlink at once (held in a
string list). Make the @paths argument a string list then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Simirarly to qemuDomainAttachDeviceMknodHelper() which was
modified just a couple of commits ago, modify the unlink helper
which is called on device detach so that it can unlink multiple
files in one go instead of forking off for every single one of
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanup, creating /dev nodes from pre-exec hook is
no longer needed and thus can be removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain SEV into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain loader into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain RNGs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain inputs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain graphics (render node)
into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain TPM into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain chardevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain memory (nvdimms) into
daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain hostdevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain disks into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in previous commit, populating domain's namespace
from pre-exec() hook is dangerous. This commit moves population
of the namespace with basic /dev nodes (e.g. /dev/null, /dev/kvm,
etc.) into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Okay, here is the deal. Currently, the way we build namespace is
very fragile. It is done from pre-exec hook when starting a
domain, after we mass closed all FDs and before we drop
privileges and exec() QEMU. This fact poses some limitations onto
the namespace build code, e.g. it has to make sure not to keep
any FD opened (not even through a library call), because it would
be leaked to QEMU. Also, it has to call only async signal safe
functions. These requirements are hard to meet - in fact as of my
commit v6.2.0-rc1~235 we are leaking a FD into QEMU by calling
libdevmapper functions.
To solve this issue and avoid similar problems in the future, we
should change our paradigm. We already have functions which can
populate domain's namespace with nodes from the daemon context.
If we use them to populate the namespace and keep only the bare
minimum in the pre-exec hook, we've mitigated the risk.
Therefore, the old qemuDomainBuildNamespace() is renamed to
qemuDomainUnshareNamespace() and new qemuDomainBuildNamespace()
function is introduced. So far, the new function is basically a
NOP and domain's namespace is still populated from the pre-exec
hook - next patches will fix it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim to make it look as close to
qemuDomainNamespaceSetupDisk() as possible. The latter will call
the former and this change makes that diff easier to read.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every caller does the same - counts the number of items in a
string list they have, only to pass the number to
qemuDomainNamespaceMknodPaths(). This is needless - the function
can accept the string list and count the items itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the previous commit prepared the helper function run in a
forked off helper (with corresponding struct), this commit
modifies the caller, which now create all files requested in a
single process and does not fork off for every single path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, when attaching a device needs two or more /dev nodes
created into a domain, we fork off and run the helper for every
node separately. For majority of devices this is okay, because
they need no or one node created anyway. But the idea is to use
this attach code to build the namespace when starting a domain,
in which case there will be way more nodes than one.
To achieve this, the recursive approach for handling symlinks has
to be turned into an iterative one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When attaching a device into a domain, the corresponding /dev
node might need to be created in the domain's namespace. For some
types of files we call mknod(), for symlinks we call symlink(),
but for others - which exist in the host namespace - we need to
so called 'bind mount' them (which is a way of passing a
file/directory between mount namespaces). There is this condition
in qemuDomainAttachDeviceMknodRecursive() which decides whether a
bind mount will be used, move it into a separate function so that
it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This structure is going to be used from not only device attach
code, but also when building the namespace. Moreover, the code
lives in a separate file so the chances of clashing with another
name are minimal.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's not really a problem since this is a helper process that
dies as soon as the helper function returns, but the cleanup code
will be replaced with a function soon and this change prepares
the code for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While qemuDomainNamespaceMknodPaths() doesn't actually create
files in the namespace in one go (it forks for each path), it a
few commits time it will.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Functions that create a device node after domain startup (used
from hotplug) will get a list of paths they want to create and
eventually call qemuDomainNamespaceMknodPaths() which then checks
whether domain mount namespace is enabled in the first place.
Alternatively, on device hotunplug, we might want to delete a
path inside domain namespace in which case
qemuDomainNamespaceUnlinkPaths() checks whether the namespace is
enabled. While this is not dangerous, it certainly burns a couple
of CPU cycles needlessly.
Check whether mount namespace is enabled upfront.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is a lot of functions called from
qemuDomainBuildNamespace() that accept @cfg
(virQEMUDriverConfigPtr) as an argument and don't use it.
Historically, it was done so that all qemuDomainSetupAll*()
functions look the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The name of this function is not very helpful, because it doesn't
create anything, it just flips a bit in a bitmask when domain is
starting up. Move the function internals into qemu_process.c and
forget the function ever existed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemu_domain.c file is big as is and we should split it into
separate semantic blocks. Start with code that handles domain
namespaces.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
These variables are only used for assignment and have
no other effect.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In virCgroupV2BindMount there is an unused variable containing
what seem to be tmpfs mount options.
Delete it. Unlike with cgroups v1, we do not create a tmpfs
here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that everything uses g_strfreev, this function is no longer
needed.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Both accept a NULL value gracefully and virStringFreeList
does not zero the pointer afterwards, so a straight replace
is safe.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The g_strdupv function from GLib provides
the same functionality.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Also remove the temporary variable - even virStringListCopy
aborts on OOM now.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Last usage out of virlog.c was removed by
commit 91268c715c
node_device_udev: remove deprecated logging function
Also drop the virbuffer.h include - it seems it was never used
for anything else than the transitive stdarg.h include.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This function calls virLogVMessage. Move it below the definition
of virLogVMessage so it can call it even without a prototype.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The XML function is needed in the C file,
not in the header.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This file is using XML functions without including
the header.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It was needed for virAsprintf, which is now dropped.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 33ed622106
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We use an array of size VIR_NODE_MEMORY_STATS_FIELD_LENGTH
to store the string read from sysfs, but pass unbound "%s"
to sscanf.
Make the array larger by one and simply stringify that
constant as the field width specifier.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The 'dom' pointer is already dereferenced earlier.
src/vz/vz_sdk.c:249:24: warning: Either the condition 'if(dom)'
is redundant or there is possible null pointer dereference:
dom. [nullPointerRedundantCheck]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There is no distinction between Read/Write locks for resctrl from libvirt's
point of view any more.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It was created to get rid of conditional compilation in the resctrl code and
make it usable anywhere else. However this is not something that is going to be
used in other places because it is not portable and resctrl is just very
specific in this regard. And there is no reason why there could not be a
preprocessor conditional in the resctrl code. Also the interface of
virFileFlock() was very ambiguous which lead to some issues.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
That's the way it should've been all the time. It was originally the case, but
then the rework to virFileFlock() made the function ambiguous when it was
created in commit 5a0a5f7fb5, and due to that it was misused in commit
657ddeff23 and since then the lock being taken was shared rather than
exclusive.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The shmem 'name' specifies the shared memory path in '/dev/shm/',
however, we may need to change it to avoid filename conflict
when VM migrate to other host. This patch remove shmem name
consistency check.
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Role(master or peer) controls how the domain behaves on migration.
For more details about migration with ivshmem, see
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/system/ivshmem.rst;hb=HEAD
It's a optional attribute in libvirt, and qemu will choose default
role for ivshmem device if the user is not specified.
With device property 'role', the value can be 'master' or 'peer'.
- 'master' (means 'master=on' in qemu), the guest will copy
the shared memory on migration to the destination host.
- 'peer' (means 'master=off' in qemu), the migration is disabled.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Yang Hang <yanghang44@huawei.com>
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-export-dynamic is provided by src_dep
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
This introduces intermediate static library that is required for
following remote protocol check.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
We have to compile the libvirt-admin.so outside of src/admin directory
because it depends on libvirt.so.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
There are two extra dependencies compared to automake, apparmor and
selinux. It looks like libtool is doing some magic and inheriting
dependencies from libvirt.la which inherits these dependencies from
other libraries.
Without these two dependencies in meson the compilation fails.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Drop automake like print from scripts/hyperv_wmi_generator.py as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Drop automake like print from scripts/esx_vi_generator.py as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
WARN_FLAGS are not relevant for meson as all warning flags are set to
the whole project using add_project_arguments().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Meson doesn't use .libs directory, everything is placed directly into
directories where meson.build file is used.
In order to have working tests and running libvirt directly from GIT we
need to fix all the paths pointing '.libs' directory.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
With meson we no longer have .libs directory with the actual binary so
we have to take a different approach to detect if running from build
directory.
This is not as robust as for autotools because if you select --prefix
in the build directory it will incorrectly enable the override as well
but nobody should do that.
We have to modify some of the tests to not add current build path into
PATH variable and use the full path for virsh instead. Otherwise it
would be impossible to figure out that we are running virsh from build
directory.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
There is no point of having this option in libvirt because the debug
logs can be configured using log filters.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
EXTRA_DIST is not relevant because meson makes a git copy when creating
dist archive so everything tracked by git is part of dist tarball.
The remaining ones are not converted to meson files as they are
automatically tracked by meson.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
After the switch to libnl these are no longer used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 77e7c13b2e
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
After dropping support for sanlock < 2.4,
this constant is no longer used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: c495169478
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
When trying to parse an XML with overlapping iothread scheduler
settings, the error message was rather confusing:
error: iothreadssched attributes 'vcpus' must not overlap
Pass the correct element name.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pass the scheduler element name instead of trying to reconstructing
it from the attribute name.
This has the benefit of not mixing '%s' with regular text in
translatable strings as well as preventing the confusion when
the 's' marking the plural in the element name ('vcpus') is taken
as a first letter of the 'sched' suffix.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7ea55a481d
Fixes: 99c5fe0e7c
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virDomainThreadSchedParseHelper is used for parsing both iothread
and vcpu scheduling settings. Rename its 'name' attribute to
make it obvious this refers to the attribute name, not the name of
the element (which is currently constructed from the attribute name).
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When doing an external snapshot we migrate memory to a file as a form of
taking the memory state. This creates a problem as qemu deactivates all
active bitmaps after a successful migration. This means that calling
'query-named-block-nodes' will return an empty list of bitmaps for
devices. We use the bitmap list to propagate the active bitmaps into the
overlay files being created which is required for backups to work after
a snapshot. Since we wouldn't propagate anything a subsequent backup
will fail with:
invalid argument: missing or broken bitmap 'testchck' for disk 'vda'
To fix this, we can simply collect the bitmap list prior to the
migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1862472
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
virPCIDeviceAddressPtr 'addr' is forgotten to be freed in the branch
'VIR_APPEND_ELEMENT() < 0'. Use g_autoptr instead.
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr to free the temporary virDomainDef object created by
qemuDomainSaveInternal() when xmlin is non-NULL. Leak was added in
commit 0ea479f8f6, first appearing in libvirt 0.9.4 in August 2011.
Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The term number is used for other stats and even for hugetlb
stats in virsh man page. The term number is also more clear.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
CVE-2020-14339
When building domain's private /dev in a namespace, libdevmapper
is consulted for getting full dependency tree of domain's disks.
The reason is that for a multipath devices all dependent devices
must be created in the namespace and allowed in CGroups.
However, this approach is very fragile as building of namespace
happens in the forked off child process, after mass close of FDs
and just before dropping privileges and execing QEMU. And it so
happens that when calling libdevmapper APIs, one of them opens
/dev/mapper/control and saves the FD into a global variable. The
FD is kept open until the lib is unlinked or dm_lib_release() is
called explicitly. We are doing neither.
However, the virDevMapperGetTargets() function is called also
from libvirtd (when setting up CGroups) and thus has to be thread
safe. Unfortunately, libdevmapper APIs are not thread safe (nor
async signal safe) and thus we can't use them. Reimplement what
libdevmapper would do using plain C (ioctl()-s, /proc/devices
parsing, /dev/mapper dirwalking, and so on).
Fixes: a30078cb83
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858260
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since we have VIR_AUTOSTRINGLIST we can use it to free string
lists used in the function automatically.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are two distinct WITH_DEVMAPPER sections in the file, for
different functions each. Rearrange the code to make some of
future commits smaller.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The storage pool code now attempts to disable COW by default on btrfs,
but management applications may wish to override this behaviour. Thus we
introduce a concept of storage pool features:
<features>
<cow state='yes|no'/>
</features>
If the <cow> feature policy is set, it will be enforced. It will always
return an hard error if COW cannot be explicitly set or unset.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This calls virFileSetCOW when building a pool with a request to attempt,
but not require, COW to be disabled. The effect is that nothing changes
on non-btrfs filesystems, but btrfs will get COW disabled on the
directory. This setting is then inherited by all newly created files in
the pool, avoiding the need for mgmt app to set "nocow" on a per-volume
basis.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When disabling COW on individual files, we now use the virFileSetCOW
method. Note that this change has a slight semantic difference to the
old implementation.
The original code reported errors but returned success when disabling
COW failed.
With this new code, we will always report an error if the user requested
disabling of COW and we could not honour it, either because btrfs
returned an error, or because the filesystem is not btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
btrfs defaults to performing copy-on-write for files. This is often
undesirable for VM images, so we need to be able to control whether this
behaviour is used.
The virFileSetCOW() will allow for this. We use a tristate, since out of
the box, we want the default behaviour attempt to disable cow, but only
on btrfs, silently do nothing on non-btrfs. If someone explicitly asks
to disable/enable cow, then we want to raise a hard error on non-btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is only used in the ESX driver where, when set to "no", it will
ignore all the checks libvirt does about the origin of the MAC address
(whether or not it's in a VMWare OUI) and forward the original one to
the ESX server telling it not to check it either.
This allows keeping a deterministic MAC address which can be useful for
licensed software which might dislike changes.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
VMX conversion parts rewritten to apply on top of previously merged
support for type='generated|static'
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When support for MAC addresses having a type='static|generated'
attribute was added in:
commit 454e5961ab
Author: Bastien Orivel <bastien.orivel@diateam.net>
Date: Mon Jul 13 16:28:53 2020 +0200
Add a type attribute on the mac address element
the VMX -> XML parser was not updated. As a result while we
accept the 'type' attribute on input, we never show it again
on 'output', so we loose information during the roundtrip.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the current formatter, the XML snippets:
<interface type='bridge'>
<mac address='00:0c:29:dd:ee:fe' type='static'/>
<source bridge='br1'/>
</interface>
<interface type='bridge'>
<mac address='aa:bb:cc:dd:ee:fd' type='generated'/>
<source bridge='br2'/>
</interface>
result in
ethernet1.present = "true"
ethernet1.networkName = "br1"
ethernet1.connectionType = "bridged"
ethernet1.addressType = "static"
ethernet1.address = "00:0c:29:dd:ee:fe"
ethernet1.checkMACAddress = "false"
ethernet2.present = "true"
ethernet2.networkName = "br2"
ethernet2.connectionType = "bridged"
ethernet2.addressType = "static"
ethernet2.address = "aa:bb:cc:dd:ee:fd"
ethernet2.checkMACAddress = "false"
which is flawed, as both type='static' and type='generated' in the XML
turn into 'static' in the VMX config.
The existence of the 'static' attribute is further overriding whether
the checkMACAddress config option is set as a side effect.
Both these pieces of flawed logic were introduced in
commit 454e5961ab
Author: Bastien Orivel <bastien.orivel@diateam.net>
Date: Mon Jul 13 16:28:53 2020 +0200
Add a type attribute on the mac address element
which intentionally added the 'checkMACAddress' side effect based on
the 'type' attribute.
With this change, we're reverting the handling of checkMACAddress
to match what existed historically. The 'type' attribute now directly
maps to the addressType attribute, so the above config becomes:
ethernet1.present = "true"
ethernet1.networkName = "br1"
ethernet1.connectionType = "bridged"
ethernet1.addressType = "static"
ethernet1.address = "00:0c:29:dd:ee:fe"
ethernet2.present = "true"
ethernet2.networkName = "br2"
ethernet2.connectionType = "bridged"
ethernet2.addressType = "generated"
ethernet2.generatedAddress = "aa:bb:cc:dd:ee:fd"
ethernet2.generatedAddressOffset = "0"
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gcc 10.1.0 on Debian sid has a bug where the bounds checking gets
confused beteen two branches:
In file included from /usr/include/string.h:495,
from ../../src/internal.h:28,
from ../../src/util/virsocket.h:21,
from ../../src/util/virsocketaddr.h:21,
from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
In function 'memcpy',
inlined from 'virNetDevGetifaddrsAddress' at ../../src/util/virnetdevip.c:914:13,
inlined from 'virNetDevIPAddrGet' at ../../src/util/virnetdevip.c:962:16:
/usr/include/arm-linux-gnueabihf/bits/string_fortified.h:34:10: error: '__builtin_memcpy' offset [16, 27] from the object at 'addr' is out of the bounds of referenced subobject 'inet4' with type 'struct sockaddr_in' at offset 0 [-Werror=array-bounds]
34 | return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
../../src/util/virnetdevip.c: In function 'virNetDevIPAddrGet':
../../src/util/virsocketaddr.h:29:28: note: subobject 'inet4' declared here
29 | struct sockaddr_in inet4;
| ^~~~~
cc1: all warnings being treated as errors
Note the source location is pointing to the "inet6" / AF_INET6 branch of
the "if", but is complaining about bounds of the "inet4" field. Changing
the code into a switch() is sufficient to avoid triggering the bug and
is arguably better code too.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are races condiction to make '/run/libvirt/qemu/dbus' directory in
virDirCreateNoFork() while concurrent start VMs, and get "failed to create
directory '/run/libvirt/qemu/dbus': File exists" error message. pre-create the
dbus directory in qemuStateInitialize.
Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove the superfuous break, as there is a 'return' before it.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Since active domains which do not have the attribute already set were
not started by libvirt that probed for CPU migratable property, we need
to check this property on reconnect and update the domain definition
accordingly.
https://bugzilla.redhat.com/show_bug.cgi?id=1857967
Reported-by: Mark Mielke <mark.mielke@gmail.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit v6.4.0-61-g201bd5db63 started to fill the default value for
//cpu/@migratable attribute according to QEMU support. However, active
domains either have the migratable attribute already set or the
capabilities we use for checking the QEMU support were created by older
libvirt which didn't probe for this specific capability. Thus we should
leave active domains alone when parsing their XMLs.
https://bugzilla.redhat.com/show_bug.cgi?id=1857967
Reported-by: Mark Mielke <mark.mielke@gmail.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use g_auto* on pointers to avoid using the 'cleanup' label.
In theory the 'ret' variable can also be discarded if the flow
of the logic is reworked. Perhaps another time.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-5-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use g_autoptr() on pointers and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-4-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-3-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Next patches will use g_autoptr() in qemuProcessQMPPtr pointers
for some cleanups in QMP code.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-2-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Refactor networkSetIPv6Sysctls to remove repetition and reuse
of the 'field' variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ignore errors from creating "libvirt-tmp-activewrite" bitmap. This
prevents failures of finishing blockjobs if the bitmap already exists.
Note that if the bitmap exists, the worst case that can happen is that
more bits are marked as dirty in the resulting merge.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
There are two possible 'transaction' command arguments in the function.
Rename 'actions' as they deal with creating bitmaps only.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The 'libvirt-tmp-activewrite' bitmap is added during the 'pivot'
operation of block copy and active layer block commit operations
regardless of whether there are any bitmaps to merge, but was not
removed unless a bitmap was merged. This meant that subsequent attempts
to merge into the same image would fail.
Fix it by checking whether the 'libvirt-tmp-activewrite' would be used
by the code and don't skip the code which would delete it.
This is a regression introduced when we switched to the new code for
block commit in <20a7abc2d2d> and for block copy in <7bfff40fdfe5>. The
actual bug originates from <4fa8654ece>.
https://bugzilla.redhat.com/show_bug.cgi?id=1857735
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Commit 20a7abc2d2 tried to delete the possibly leftover bitmap but
neglected to call the actual monitor to do so.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The handler finalizing the active layer block commit doesn't actually
reopen the file for active layer block commit, so the comment and check
are invalid.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The infrastructure supports setting the threshold also for the <mirror>.
Mention it in the docs.
https://bugzilla.redhat.com/show_bug.cgi?id=1807741
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When doing a block copy, there is another chain of images attached to a
disk. Consider them as well when looking up a disk using nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Top level image may get two events, one with the disk target (vda) and
one with disk target with index (vda[3]) if the top level image has an
index.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The index returned by qemuDomainDiskLookupByNodename is the position in
the backing chain rather than the index we report in the XML.
Since with -blockdev they differ now and additionally the disk source
also has an index we need to fix the 'threshold' events we report:
1) If it's the top level image we must always trigger the event without
any suffix as we did until now
2) We must report the correct index
3) We must report the correct index also for the top level image, when
blockdev is used.
This means that we need to potentially emit 2 events, one for the device
without the index and then when blockdev is used and the top level image
has an index we must do it also with the index.
This will fix it for blockdev cases, while also not removing previous
semantics.
https://bugzilla.redhat.com/show_bug.cgi?id=1857204
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Rather than having labels named exit, done, exit_snooprequnlock,
skip_rename, etc, use the standard "cleanup" label. And instead of
err_exit, malformed, tear_down_tmpebchains, use "error".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This rewrite of a nested conditional produces the same results, but
eliminate a goto and corresponding label.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's possible/probable the callers to virNWFilterInstReset() make it
unnecessary to set the object's nrules to 0 after freeing all its
rules, but that same function is setting nfilters to 0, so let's do
the same for the sake of consistency.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On failure, this function would clear out and free the list of
subchains it had been called with. This is unnecessary, because the
*only* caller of this function will also clear out and free the list
of subchains if it gets a failure from ebtablesGetSubChainInsts().
(It also makes more logical sense for the function that is creating
the entire list to be the one freeing the entire list, rather than
having a function whose purpose is only to create *one item* on the
list freeing the entire list).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko redhat com>
All these cleanup/error labels were reduced to having just "return
ret" by a previous patch, so get rid of them and return directly.
This patch coincidentally fixes a bug in
networkFindUnusedBridgeName(), where we would log an error yet still
return success if we failed to find a single unused "virbrNNN" name
after checking all values of "N" from 0 - 256. Said bug was introduced
when that function was originally written, in commit a28d3e485f
(libvirt 1.2.15, 2015)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This includes standard g_autofree() as well as other objects that have
a cleanup function defined to use via g_autoptr (virCommand,
virJSONValue)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the
public API file libvirt-network.h, and we can't pollute that with glib
macro invocations, so put this in src/datatypes.h next to the other
virNetwork items.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
g_new() is used in only 3 places. Switching them to g_new0() will do
no harm, reduces confusion, and helps me sleep better at night knowing
that all allocated memory is initialized to 0 :-) (Yes, I *know* that
in all three cases the associated memory is immediately assigned some
other value. Today.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To remove dependecy of `qemuDomainJob` on job specific
paramters, a `privateData` pointer is introduced.
To handle it, structure of callback functions is
also introduced.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In 9536379da4 and 8b0cb0e666 I've tried to call
virNetSocketCheckProtocolByLookup() only if we are suspecting the
host is IPv4 or IPv6 capable (because we've found an interface
with such address). However, the code was missing dereference of
the boolean variables and thus was comparing pointers against
NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This partially reverts commit 5331c4804f.
The original commit mistakenly thought virFileCacheLookup did not set
an error. In fact the only case it doesn't set an error for is when
the cache key is NULL. This in fact the fault of the caller for passing
an invalid cache key, so doesn't need to be handled.
This caller bug was fixed by checking for a NULL binary in the
virQEMUCapsCacheLookupDefault method.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Conver the code to the new approach which uses XPath to fetch known
elements rather than looping through all XML children.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a switch statement which will not be omitted when adding potential
new types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the mdev subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the vHBA subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split up formatting of the '<address>' element rather that trying to
optimize it with formatting string hacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the SCSI subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the PCI subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate out bits related to USB so that the logic isn't entangled in
multiple conditional statements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the formatter to the new multiple buffer based approach so that
we can easily separate it into formatters per subsys type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commits, modify the hostdev detach code to use
blockdev infrastructure to detach (i)SCSI hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to command line creation, use the blockdev helpers when
hotplugging an (i)SCSI hostdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In preparation for instantiating (i)SCSI hostdevs via -blockdev,
refactor qemuBuildHostdevSCSICommandLine to use the new infrastructure
which will do it automatically.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add convertor for creating qemuBlockStorageSourceAttachData which will
allow reusing the infrastructure which we have for attaching disks also
for hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We want to instantiate hostdevs via -blockdev too. Add a separate
capability for them for a clean transition. The new capability will be
enabled when QEMU_CAPS_BLOCKDEV is present once all code is prepared.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't (re)generate the backend alias (alias of the -drive backend for
now) internally but rather pass it in. Later on it will be replaced by
the nodename when blockdev is used depending on the capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move all (i)SCSI related code into a new function named
'qemuBuildHostdevSCSICommandLine'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now store the alias of the secrets in the status XML so there's no
need to generate it again.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When upgrading from a libvirt which didn't format private data of a
virStorageSource representing an iSCSI hostdev source, we might need to
generate some internal data so that the code still works as if it was
present in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need the alias to deal with hot-unplug of the hostdev. Use
qemuDomainSecretInfoDestroy which clears only the secrets and not the
alias. The same function is used also for handling disk secrets.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We store the config of an iSCSI hostdev in a virStorageSource structure.
Parse the private data portion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
iSCSI subsystem hostdevs store the data as a virStorageSource. Format
the private data part of the virStorageSource in the appropriate place.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
SCSI hostdevs don't have a virStorageSource associated with the backend
in certain cases. Adding a separate field to hold memory for a copy of
the nodename of the storage backend will allow reusing the blockdev
machinery also for SCSI hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It doesn't make sense to format "discard" when doing a -blockdev backend
of scsi-generic used with SCSI hostdevs. Add a way to skip it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming commit will need to add another flag for the function so
convert it to a bitwise-or'd array of flags to prevent having 4
booleans.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, if we found an interface with an IPv6 address we
did not blindly trust that host is IPv6 capable (as in we can
successfully translate IPv4 addresses) but used getaddrinfo() to
confirm it. Turns out, we have use the same argument for IPv4.
For instance, in an namespace created by the following steps,
getaddrinfo("127.0.0.1", ...) fails (demonstrating by "Socket
TCP/IPv4 Accept" test case failing in virnetsockettest):
unshare -n
ip link set lo up
ip link add dummy0 type dummy
ip link set dummy0 up
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is not much sense trying to disprove host is IPv6 capable
if we know after first round (getifaddrs()) that is is not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virNetSocketCheckProtocols() function is supposed to tell
caller whether IPv4 and/or IPv6 is supported on the system. In
the initial round, it uses getifaddrs() to see if an interface
has IPv4/IPv6 address assigned and then to double check IPv6 it
uses getaddrinfo() to lookup IPv6 loopback address. Separate out
this latter code because it is going to be reused.
Since the original code lived under an #ifdef and the new
function doesn't it is marked as unused - because on some systems
it may be so.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, we've used security_context_t for variables passed
to libselinux APIs. But almost 7 years ago, libselinux developers
admitted in their API that in fact, it's just a 'char *' type
[1]. Ever since then the APIs accept 'char *' instead, but they
kept the old alias just for API stability. Well, not anymore [2].
1: 9eb9c93275
2: 7a124ca275
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Choose a TPM 2 device for the backend as default for the CRB interface
since TPM 1.2 would not work.
This patch addresses BZ 1781913: https://bugzilla.redhat.com/show_bug.cgi?id=1781913
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so
prevent the choice of TPM 1.2 when the SPAPR device model is chosen
and use a default of '2.0' (TPM 2) for the backend.
This patch addresses BZ 1781913: https://bugzilla.redhat.com/show_bug.cgi?id=1781913
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Move setting the TPM default version out of the validation function into
the post parse function.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Functions `qemuDomainObjPrivateXMLParseJob` and
`qemuDomainObjPrivateXMLFormatJob` are moved from
`qemu_domain` to `qemu_domainjob`.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`qemuDomainObjPrivatePtr` parameter was avoided being passed
as a paramter in functions `qemuDomainObjPrivateXMLParseJob`
and `qemuDomainObjPrivateXMLFormatJob`, as we already pass
`virDomainObjPtr`, which can be used to get `privateData`
pointer.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During disk hot plugging, qemuDomainAttachDeviceLive() adds the new
disk to the device list of the VM object. However, hot plugging
cdroms and floppies only updates the src variable of the original
disk device, so the newly generated disk object needs to be freed.
Signed-off-by: Jin Yan <jinyan12@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is only used in the ESX driver where, when set to "static", it will
ignore all the checks libvirt does about the origin of the MAC address
(whether or not it's in a VMWare OUI) and forward the original one to
the ESX server telling it not to check it either.
This allows keeping a deterministic MAC address which can be useful for
licensed software which might dislike changes.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove both 'error' and 'cleanup' labels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the obsolete 'error' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Next patch will use g_autoptr() in a qemuMigrationCookiePtr pointer to
modernize qemuMigrationSrcBeginPhase().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the now obsolete 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove both 'cleanup' and 'error' labels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use variable autocleanup and remove the now obsolete 'cleanup'
label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() on pointers and remove the unneeded 'cleanup'
label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree on strings and remove the 'done' label since it's
now unneeded.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add headers with declarations of geteuid/getegid
and virGetUserName/virGetGroupName.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
While moving the code, qemuDomainNamespace also was moved
to `qemu_domainjob`. Hence it is moved back to `qemu_domain`
where it will be more appropriate.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With meson introduction which is using the same CFLAGS for the whole
project some compilation errors were discovered. The wireshark plugin
library is the only one in tools directory that is not using AM_CFLAGS.
With the AM_CFLAGS we get these errors:
../../tools/wireshark/src/packet-libvirt.c: In function 'dissect_libvirt_fds':
../../tools/wireshark/src/packet-libvirt.c:348:31: error: unused parameter 'tvb' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~~~~~~^~~
../../tools/wireshark/src/packet-libvirt.c:348:41: error: unused parameter 'start' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~^~~~~
../../tools/wireshark/src/packet-libvirt.c:348:55: error: unused parameter 'nfds' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~~~^~~~
At top level:
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_bool' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:88:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
88 | XDR_PRIMITIVE_DISSECTOR(bool, bool_t, boolean)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_float' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:86:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
86 | XDR_PRIMITIVE_DISSECTOR(float, gfloat, float)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_short' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:80:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
80 | XDR_PRIMITIVE_DISSECTOR(short, gint16, int)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c: In function 'dissect_libvirt_message':
../../tools/wireshark/src/packet-libvirt.c:423:34: error: null pointer dereference [-Werror=null-dereference]
423 | vir_xdr_dissector_t xd = find_payload_dissector(proc, type, get_program_data(prog, VIR_PROGRAM_DISSECTORS),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
424 | *(gsize *)get_program_data(prog, VIR_PROGRAM_DISSECTORS_LEN));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The current code to check XDR support was obsolete and way to
complicated.
On linux we can use pkg-config to check for libtirpc and have
the CFLAGS and LIBS configured by it as well.
On MinGW there is portablexdr library which installs header files
directly into system include directory.
On FreeBSD and macOS XDR functions are part of libc so there is
no library needed, we just need to call AM_CONDITIONAL to silence
configure which otherwise complains about missing WITH_XDR.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All of the listed functions are available in libselinux version 2.2.
Our supported OSes start with version 2.5 so there is no need to check
it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Just like in the previous commit, the stdin_path argument of
virSecurityManagerSetAllLabel() is renamed to incomingPath.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
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>
The only consumer was removed in the previous commit.
This reverts commit f03a38bd1d.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
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().
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1851016
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These APIs are are basically
virSecuritySELinuxDomainSetPathLabelRO() and
virSecuritySELinuxDomainRestorePathLabel().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
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.
This reverts bd22eec903.
This partially reverts 4ccbd207f2.
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.
1: https://bugzilla.redhat.com/show_bug.cgi?id=1851016
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
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>
In the light of recent commit of 9d83281382 fix the comment that
says directories can't be locked. Well, in general they can, but
not in our case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit <5b816e16968ba02def56f067774ecd9a8c8d44d7> removed hard-coded
sysconfdir path from *.conf files but missed virtproxyd.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
It was introduced by commit <c606671aaad10a9bc87f226bc473a091e00a9629>
as a gnulib ldexp module and later removed by commit
<09fe607b4de8eb883c966e90aaf5563299a22738>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Fixes inconsistency with macro names for external programs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <0985a9597bb0348d46c0d18dc548a676bf0ad8e2> added unused variable
so remove it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
Commit <fd3b8fe7ad491c77c0b3f57110adaf64f743855e> removed objectlocking
test but forgot to remove all of the usages of LOCK_CHECKING_CFLAGS.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function sanlock_write_lockspace() was introduced in 2.7 version which
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>
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>
SANLK_INQ_WAIT was introduced in sanlock 2.4 which 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>
This check was introduced by commit
<96a02703daad4dc6663165adbc0feade9900cebd> to guard calling
sanlock_inq_lockspace() function but it used SANLK_INQ_WAIT as a
parameter which was introduced later. This was eventually fixed by
commit <238dba0f9c925359cb3b8beddd8c8ae739cb4e06>.
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>
This was introduced together with clock-time gnulib module by commit
<d74e5a4dfc434d3a1d01856d013a7f50d910fa95> and removed from libvirt
by commit <86d223a762990c9d529065a2d3b30b6a00ea63dd>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set FLAT_NAMESPACE_FLAGS to -Wl,-flat_namespace in configure only for
macOS and use it unconditionally in Makefiles.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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.
It's mandatory since <5aec02dc37623bf739d1edd8f2be3e4ad9f94ff5>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virFileIsAccessible does not return true on accessible
directories. Check whether it set EISDIR and only
then assume the directory is inaccessible.
Return 0 (not found) instead of 1 (found),
since the bridge driver taints the network based on
this return value, not whether the hook actually ran.
Remove the bogus check from virHookCall, since it already
checks the virHooksFound bitmap that was filled before
by virHookCheck.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7fa7f7eeb6
Closes: https://gitlab.com/libvirt/libvirt/-/issues/47
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
- 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 18dca21a32 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.
https://bugzilla.redhat.com/1852741
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
All modified functions are similar, in all cases "cleanup" label is removed,
along with all the "goto" calls.
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com>
Reviewed-by: Laine Stump <laine@redhat.com>
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>
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>
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>
