Remove the overlay if the disk was <transient/>. Note that even if we'd
forbid unplug of such a disk through the API, the disk can still be
ejected from the guest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Block migration when transient disk option is enabled to simplify the
handling of the overlay files.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
For now we disable disk hotplug of transient disk as it requires
creating an overlay prior to adding the frontend.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
For now we disallow blockjobs with transient disks to avoid dealing with
obsoleted overlays.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
To implement <transient/> disks we'll need to install an overlay on top
of the original disk image which will be discarded after the VM is
turned off. This was initially implemented by qemu but libvirt never
picked up this option as the overlays were created by qemu without
libvirt involvment which didn't work with SELinux.
With blockdev the qemu feature became unsupported so we need to do this
via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk
definition for a disk which is configured as <transient/> and use it to
create a snapshot (without actually modifying metadata or persistent
def).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Later patches will implement support for <transient/> disks in libvirt
by installing an overlay on top of the configured image. This will
require cleanup after the VM will be stopped so that the state is
correctly discarded.
Since the overlay will be installed only during the startup phase of the
VM we need to ensure that qemuProcessStop doesn't delete the original
file on some previous failure. This is solved by adding
'inhibitDiskTransientDelete' VM private data member which is set prior
to any startup step and will be cleared once transient disk overlays are
established.
Based on that we can then delete the overlays for any <transient/> disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Allow using the function for creating temporary snapshot disk
definitions for creating <transient/> disk overlays.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
CVE-2020-25637
Add a requirement for domain:write if source is set to
VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
CVE-2020-25637
Add a new field to @acl annotations for filtering by
unsigned int parameters.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
CVE-2020-25637
Prepare for omission of the <flagname> in remote_protocol.x
@acl annotations:
@acl: <object>:<permission>:<flagname>
so that we can add more fields after, e.g.:
@acl: <object>:<permission>::<field>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The refactoring in commit de49d5bad3 accidentally dropped the statement
setting def to NULL after successfully adding it to the virDomainObjList,
causing it to be freed while still in use. The resulting memory
corruption caused unpredictable behavior, often resulting in a libvirtd
crash.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Like other distros, openSUSE Tumbleweed recently changed libexecdir from
/usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper
and pygrub.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Allow to match with CCW addresses in addition to PCI addresses
(and MAC addresses).
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
'ndd' tracks the actual number of snapshot disks filled into the
structure and is incremented by the functions filling the context, thus
it must not be set when initializing the context.
Fixes: 8c2ecdf131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The container itself needs to be freed too.
Fixes: 8c2ecdf131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This flag was added by Linux with:
commit f43798c27684ab925adde7d8acc34c78c6e50df8
Author: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu Jul 3 03:48:02 2008 -0700
tun: Allow GSO using virtio_net_hdr
so we can assume all Linux distros we support have this flag available
and thus the compile time check is sufficient.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Make it obvious that the snapshot is prepared for the active external
snapshot case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the code which invokes the monitor and finalizes the snapshot
into a separate function for easier reuse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a container struct which holds all data needed to create and clean
up after a (for now external) snapshot. This will aggregate all the
'qemuSnapshotDiskDataPtr' the 'actions' of a transaction QMP command and
everything needed for cleanup at any given point.
This aggregation allows to simplify the arguments of the functions which
prepare the snapshot data and additionally will simplify the code
necessary for creating overlays on top of <transient/> disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Be more specific about the role of the function. It's creating the disk
portion of an external active snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Most of the variables were reinitialized on every iteration.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It was made pointless by:
commit c25c18f71b
Convert capabilities / domain_conf to use virArch
and unused by:
commit 8db1f2d228
Fix libxl driver for virArch changes
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
cppcheck reports:
src/vbox/vbox_XPCOMCGlue.c:226:21: style:
The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
logically equivalent to 'hVBoxXPCOMC=NULL'.
[duplicateConditionalAssign]
It does not matter anyway because this function
is never called.
Fixes: e1506cb4eb
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When the xen driver loads, it probes libxl for some info about dom0 and
adds it to the virDomainObjList. The driver then looks for any domains
in stateDir and if they are still alive adds them to the list as well.
This logic is a bit flawed wrt handling driver reload and causes the
following error
internal error: unexpected domain Domain-0 already exists
A simple fix is to load all domains from stateDir first and then only
add dom0 if needed.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
