Add a function calculating which bitmaps to copy to the mirror during
a block-copy operation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a validator which checks that a bitmap spanning multiple backing
chain members doesn't look broken. The current rules are that no
intermediate birmaps are missing (unfortunately it's hard to know
whether the topmost or bottommost bitmap is missing) and none of the
components is inconsistent.
We can obviously improve it over time.
The validator is also tested against the existing bitmap data we have
for the backup merging test as well as some of the existing broken
bitmap synthetic test cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The flags may control important aspects of the block job which may
influence also the termination of the job. Store the 'flags' for all
the block job types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a variable which will store the contents of the 'flags' variable as
passed in by the individual block jobs. Since the flags may influence
behaviour of the jobs it's important to preserve them to the
finalization steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_new0 and skip checking of the return value of keyCopy callback
as both are bound to return a valid pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tweak the return value expectation comment so that it doesn't
necessarily require to allocate memory and refactor the implementations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the glib allocation function that never returns NULL and remove the
now dead-code checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a helper that concatenates the second array into the first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Create a wrapper for qemuBlockGetNamedNodeData named
qemuBlockGetNamedNodeData. The purpose of the wrapper is to integrate
the monitor handling functionality and in the future possible
qemuCaps-based flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow qemu access to modify backing files in case when we want to delete
a checkpoint.
This patch adds tracking of which images need to be relabelled when
calculating the transaction, the code to relabel them and rollback.
To verify that stuff works we also output the list of images to relabel
into the test case output files in qemublocktest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow deleting of checkpoints when snapshots were created along. The
code tracks and modifies the checkpoint list so that backups can still
be taken with such a backing chain. This unfortunately requires to
rename few bitmaps (by copying and deleting them) in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The configure script allows users to specify different paths for
/etc/, /usr/sbin/, /var/run/ and /usr/libexec/. Instead of
assuming user will pass expected value, generate the apparmor
profiles using the actual values.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This requires stealing one cmd pointer before returning it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Mark eligible declarations as g_autofree and remove
the corresponding VIR_FREE calls.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Always trim the full specified suffix.
All of the callers outside of tests were passing either
strlen or the actual length of the string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Replace all the cases that only supply the length
and do not care about matching a suffix, as well
as that one test case that does.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Just like the existing virBufferTrim, but only
does one thing at a time.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As discussed on the developer list, parallel migration connections
are not compatible with tunneled migration
https://www.redhat.com/archives/libvir-list/2020-January/msg00463.html
Prohibit the concurrent use of parallel and tunneled migration options.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Domain definition is useless now inside network structure. This pointer
was required because new network definition was being added each time
that a new network type appeared. So, this should be processed into
old function `lxcNetworkParseDataType()`. Now, as it was moved to an
array, it can be handle together each interface pointer.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
LXC version 3 or higher introduced indexes for network interfaces.
Libvirt should be able to parse entries like `lxc.net.2.KEY`. This
commit adds functions to parse this type of field. That's why array
structures are so important this time.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Struct lxcNetworkParseData is being used as a single pointer which
iterates through LXC config lines. It means that it will be applied as a
network each time that a new type appears. After, the same struct is
used to populate a new network interface. This commit changes this logic
to multiple lxcNetworkParseData to move this strcuture to an array. It
makes more sense if we are using indexes to fill interface settings.
This is better to improve code clarity.
This commit still introduces *Legacy() functions to keep support of
network old style definitions.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now, that every use of virAtomic was replaced with its g_atomic
equivalent, let's remove the module.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of calling virAtomicIntAdd(&var, 1); we can call
g_atomic_int_add() directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.
Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:
$ virsh list
Id Name State
--------------------------
1 f28live running
1 f28live1 running
Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.
This also restores the usual numbering from 1 instead of 0.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7b9645a7d1
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Starting on commit 1f43393283, qemuDomainFillDeviceIsolationGroup()
returns 0 in all circunstances. Let's turn it to 'void' make it
clearer that the function will not fail. This also spares a
check for < 0 return in qemu_hotplug.c. The
qemuDomainFillDeviceIsolationGroupIter() callback now returns
0 at all times - which is already happening anyway.
Refer to 1f43393283 commit message for more details on why
the function was changed to never return an error.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuDomainChrDefDropDefaultPath() returns an int, but it's
always returning 0. Callers are checking for result < 0 to
run their cleanup code needlessly.
Turn the function to 'void' and adjust the callers.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Avoid some of the virObjectUnref() calls by using g_autoptr.
Aside from the 'cleanup' label in qemuDomainSetFakeReboot(),
all other now deprecated cleanup labels will be removed in
the next patch.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree to remove VIR_FREE() calls used for cleanups.
Labels that became deprecated will be removed in a later
patch.
In qemuDomainSetupDisk(), the 'dst' variable is not used at
all and could be removed.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'caps' variable in qemuDomainObjPrivateXMLParseAutomaticPlacement()
is set to auto clean via g_autoptr(), but a 'virObjectUnref(caps)' is
being executed in the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previous patch used 'g_autofree' to eliminate instances of
VIR_FREE(), making some cleanup labels obsolete. This
patch removes them.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree in strings when possible to spare a VIR_FREE()
call. Unneeded 'cleanup' labels will be taken care of in the
next patch.
The 'str' string in virDomainVirtioSerialAddrReserve() was
never used by the logic, only being used in cleanup by
VIR_FREE(). Let's remove it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With -blockdev we must look up via the nodename rather than the 'drive'
alias which is not present any more.
This fixes the pre-creation of storage volumes on migration with
non-shared storage.
https://bugzilla.redhat.com/show_bug.cgi?id=1793263
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Swithc to the helper which doesn't require checking of the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The data is gathered only once so we can move the whole block which
fetches the data out of the loop and get rid of the logic which
prevents multiple calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor the logic to skip the body of the function if there's nothing
to do.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are two calls to virHashNew which check the return value. It's not
necessary any more as virHashNew always returns a valid pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This module has last two direct checks whether the value returned by
virHashCreateFull is NULL. Remove them so that static analyzers don't
get the false idea that checking the value is necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the glib helpers and remove the mention of returning NULL on failure
of virHashNew, virHashCreate and virHashCreateFull.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since a3ab6d42 "apparmor: convert libvirtd profile to a named profile"
the detection of the subelement for qemu_bridge_helper is wrong.
In combination with the older 123cc3e1 "apparmor: allow
/usr/lib/qemu/qemu-bridge-helper" it now detects qemu-bridge-helper no
more with its path, but instead as a proper subelement of the named profile
like: label=libvirtd//qemu_bridge_helper
In the same fashion the reverse rule in the qemu_bridge_helper
sub-profile still uses the path and not the named profile label.
Triggering denies like:
apparmor="DENIED" operation="file_inherit"
profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel"
family="unix" sock_type="stream" protocol=0 requested_mask="send receive"
denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"
This patch fixes the unix socket rules for the communication between
libvirtd and qemu-bridge-helper to match that.
Fixes: a3ab6d42d8
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1655111
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Our nwfilter code doesn't set any timeout on the pcap packet buffer which
means that when DHCP snooping is enabled on a guest interface and
libvirt is trying to learn the IP address from guest's DHCP traffic, it
takes up to 4x longer to ping a guest successfully compared to a case
where nwfilter isn't enabled at all or libvirt uses the cached nwfilter
leases to populate the corresponding rules to ebtables.
With the pcap filter and rate limiting already in place, we should be
able to afford enabling the immediate packet delivery, FWIW immediate
mode was actually the default prior libpcap-1.5.0 (CentOS 6) regardless
of whether a buffer was requested.
The lack of any kind of timeout on the pcap buffer messed with the
libvirt TCK test suite which, even with a generous timeout in place,
timeouts every single time simply because it takes a while until
guest actually starts producing any kind of traffic to fill up
the buffer in place (apart from the DHCP traffic which happens fairly
early on).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
At the beginning of each profile we have a comment that says when
the profile was last updated. In theory, it makes sense because
one can see immediately if they are using an outdated profile.
However, we don't do a good job in keeping the comments in sync
with reality and also sysadmins should rather use their package
manager to find out libvirt version which installed the profiles.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
There are two more paths that we are missing in the default
domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/.
These exist on my Gentoo box and contain UEFI and BIOS images
respectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Even though we construct a domain specific profile for each
domain we start (which should cover domain specific paths), there
is also another file that is included from the profile and which
contains domain agnostic paths (e.g. to cover libraries that qemu
links with). The paths in the file are split into blocks divided
by comments. Sort the paths in each block individually (ignoring
case sensitivity).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
With Credit2 being Xen default scheduler, it's definitely the case to
allow Credit2's scheduling parameters to be get and set via libvirt.
This is easy, as Credit and Credit2 have (at least as of now) the very
same parameters ('weight' and 'cap'). So we can just let credit2 pass
the scheduler-type check and the same code will work for both.
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Aside from itinerant error (actually warning) messages due to an
unrecognized response from qemu, this isn't even necessary - the
migration proceeds successfully to completion anyway.
(I'm not sure where to see this status reported in the API though - do
we need to add an extra state, or recognition of a new event somewhere?)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Normally a PCI hostdev can't be migrated, so
qemuMigrationSrcIsAllowedHostdev() won't permit it. In the case of a a
hostdev network interface that has <teaming type='transient'/> set,
QEMU will automatically unplug the device prior to migration, and
re-plug a corresponding device on the destination. This patch modifies
qemuMigrationSrcIsAllowedHostdev() to allow domains with those devices
to be migrated.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver uses the <teaming type='persistent|transient'
persistent='blah'/> element to setup a "failover" pair of devices -
the persistent device must be a virtio emulated NIC, with the only
extra configuration being the addition of ",failover=on" to the device
commandline, and the transient device must be a hostdev NIC
(<interface type='hostdev'> or <interface type='network'> with a
network that is a pool of SRIOV VFs) where the extra configuration is
the addition of ",failover_pair_id=$aliasOfVirtio" to the device
commandline. These new options are supported in QEMU 4.2.0 and later.
Extra qemu-specific validation is added to ensure that the device
type/model is appropriate and that the qemu binary supports these
commandline options.
The result of this will be:
1) The virtio device presented to the guest will have an extra bit set
in its PCI capabilities indicating that it can be used as a failover
backup device. The virtio guest driver will need to be equipped to do
something with this information - this is included in the Linux
virtio-net driver in kernel 4.18 and above (and also backported to
some older distro kernels). Unfortunately there is no way for libvirt
to learn whether or not the guest driver supports failover - if it
doesn't then the extra PCI capability will be ignored and the guest OS
will just see two independent devices. (NB: the current virtio guest
driver also requires that the MAC addresses of the two NICs match in
order to pair them into a bond).
2) When a migration is requested, QEMu will automatically unplug the
transient/hostdev NIC from the guest on the source host before
starting migration, and automatically re-plug a similar device after
restarting the guest CPUs on the destination host. While the transient
NIC is unplugged, all network traffic will go through the
persistent/virtio device, but when the hostdev NIC is plugged in, it
will get all the traffic. This means that in normal circumstances the
guest gets the performance advantage of vfio-assigned "real hardware"
networking, but it can still be migrated with the only downside being
a performance penalty (due to using an emulated NIC) during the
migration.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The subelement <teaming> of <interface> devices is used to configure a
simple teaming association between two interfaces in a domain. Example:
<interface type='bridge'>
<source bridge='br0'/>
<model type='virtio'/>
<mac address='00:11:22:33:44:55'/>
<alias name='ua-backup0'/>
<teaming type='persistent'/>
</interface>
<interface type='hostdev'>
<source>
<address type='pci' bus='0x02' slot='0x10' function='0x4'/>
</source>
<mac address='00:11:22:33:44:55'/>
<teaming type='transient' persistent='ua-backup0'/>
</interface>
The interface with <teaming type='persistent'/> is assumed to always
be present, while the interface with type='transient' may be be
unplugged and later re-plugged; the persistent='blah' attribute (and
in the one currently available implementation, also the matching MAC
addresses) is what associates the two devices with each other. It is
up to the hypervisor and the guest network drivers to determine what
to do with this information.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Presence of the virtio-net-pci option called "failover" indicates
support in a qemu binary of a simplistic bonding of a virtio-net
device with another PCI device. This feature allows migration of
guests that have a network device assigned to a guest with VFIO, by
creating a network bond device in the guest consisting of the
VFIO-assigned device and a virtio-net-pci device, then temporarily
(and automatically) unplugging the VFIO net device prior to migration
(and hotplugging an equivalent device on the migration
destination). (The feature is called "failover" because the bond
device uses the vfio-pci netdev for normal guest networking, but
"fails over" to the virtio-net-pci netdev once the vfio-pci device is
unplugged for migration.)
Full functioning of the feature also requires support in the
virtio-net driver in the guest OS (since that is where the bond device
resides), but if the "failover" commandline option is present for the
virtio-net-pci device in qemu, at least the qemu part of the feature
is available, and libvirt can add the proper options to both the
virtio-net-pci and vfio-pci device commandlines to indicate qemu
should attempt doing the failover during migration.
This patch just adds the qemu capabilities flag "virtio-net.failover".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
* chown: use is conditionally compiled
* configmake: functionality imported to Makefile.am
* getaddrinfo: we have no portability problems
with Windows impl
* getpass: simplified impl is imported
* mgetgroups: getgrouplist is used directly
* net_if: header includes are conditionalized
* netdb: header includes are conditionalized
* passfd: simplified impl is imported
* posix-shell: functionality was unused & removed
* sigaction: usage is conditionalized
* sigpipe: usage is conditionalized
* stat-time: struct stat is used directly
* strchrnul: usage is eliminated
* strtok_r: usage is not a portability problem
* sys_stat: usage is conditionalized
* uname: rewritten to use native Win32 function to
get host arch
* waitpid: usage is conditionalized
* wcwidth: rewritten using g_unichar APIs
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The EAI_ADDRFAMILY constant has been removed from FreeBSD
headers, supposedly because it is deprecated by new RFC
drafts.
Previously GNULIB was providing a replacement because
MinGW lacked it too. The replacement provided for MinGW
was thus being used on FreeBSD too, but with a completely
bogus integer value.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are a large number of different header files that
are related to the sockets APIs. The virsocket.h header
includes all of the relevant headers for Windows and UNIX
in one convenient place. If virsocketaddr.h is already
included, then there's no need for virsocket.h
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
chown and some stat constants are not available on
the Windows platform.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_BINARY flag is not defined on all platforms so we must
conditionalize its use once we remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_DIRECT flag is not available on all platforms, so we
must introduce a compat define the same way gnulib does.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The F_DUPFD_CLOEXEC functionality is not available on
some platformms. We must thus explicitly call the
virSetCloexec function once we remove GNULIB's equiv
fix for this.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Windows uses _O_NOINHERIT as the name for its O_CLOEXEC
equivalent. Define O_CLOEXEC to match this to fix
portability when we remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_DIRECTORY flag causes open() to return an error
if the filename is a directory. There's no obvious
reason why resctrl needs to use this, while the rest of
libvirt code does not. Removing it avoids build issues
on platforms where O_DIRECTORY is not defined, once we
remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The S_ISSOCK macro is not available on Windows platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The net/if.h is not portable so we must check for its
existance and avoid using it when missing. Some use
of net/if.h was redundant and could be removed.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Many of the virProcess APIs are relying on GNULIB providing
POSIX API stubs. Even with these stubs the APIs don't do
anything useful once compiled. We can thus conditionalize
the code so that we don't compile anything at all.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Cygwin is not a supported build platform for libvirt and
has no testing coverage in our CI systems. Stop pretending
the code is usable and remove it so there is less to port
to Meson.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A large part of the virCommand code is still built on
WIN32, despite the fact that the core fork() & execve()
functions are not available. So despite succesfully
building most of the code, at runtime the APIs are
none the less unusuable. With the elimination of GNULIB
many of the APIs being used in this code no longer have
portability wrappers/shims for Windows.
Rather than try to add portability wrappers, or do tests
for each individual function, it is clearer to conditionalize
nearly all of the code using #ifdef WIN32.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
g_mkdir() provides portability to Windows platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The sys/uio.h header is only needed when building logging
code with journald support enabled. Conditionally include
it so that we avoid break on platforms which lack this
header.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The Windows platform does not have the signal handling
support we need, so it must be disabled in several parts
of the codebase.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a simplified variant of gnulib's passfd module
without the portability code that we do not require.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The gnulib stat-time.h header provides wrapper functions
to hide the differences in 'struct stat' between various
platforms.
Linux and FreeBSD support the same names, except for
birthtime which Linux only provides in the new 'statx()'
syscall we're not using. macOS has completely different
naming. Since we only rely on this code in one place
we just use conditionals at time of need.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virProcess code relies on windows.h and is getting it
indirectly via some GNULIB header fixes. This dependancy
needs to be made explicit.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The mgetgroups function is a GNULIB custom wrapper around
getgrouplist(). This implements a simplified version of
that code directly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The strchrnul function doesn't exist on Windows and rather
than attempt to implement it, it is simpler to just avoid
its usage, as any callers are easily adapted.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This imports a simpler version of GNULIB's getpass() function
impl for Windows. Note that GNULIB's impl was buggy as it
returned a static string on UNIX, and a heap allocated string
on Windows. This new impl always heap allocates.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of relying on GNULIb's uname() impl, directly use the
Windows API for determining CPU architecture.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Files inside /dev/vfio/ can't be opened more than once, meaning
that any subsequent open calls will fail. This behavior was
introduced in kernel v3.11, commit 6d6768c61b39.
When using the VFIO driver, we open a FD to /dev/vfio/N and
pass it to QEMU. If any other call attempt for the same
/dev/vfio/N happens while QEMU is still using the file, we are
unable to open it and QEMU will report -EBUSY. This can happen
if we hotplug a PCI hostdev that belongs to the same IOMMU group
of an existing domain hostdev.
The problem and solution is similar to what we already dealt
with for TPM in commit 4e95cdcbb3. This patch changes both
DAC and SELinux drivers to disable 'remember' for VFIO hostdevs
in virSecurityDACSetHostdevLabelHelper() and
virSecurityDACSetHostdevLabel(), and 'recall'
in virSecurityDACRestoreHostdevLabel() and
virSecuritySELinuxRestoreHostdevSubsysLabel().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is a case in which we do not want 'remember' to be
set to true in SetOwnership() calls inside the
HostdevLabelHelper() functions of both DAC and SELinux drivers.
Next patch will explain and handle that scenario.
For now, let's make virSecurityDACSetOwnership() and
virSecuritySELinuxSetHostdevLabelHelper() accept a 'remember'
flag, which will be used to set the 'remember' parameter
of their respective SetOwnership() calls. No functional
change is made.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently when disk is removed from iotune group (by setting
all tunables to zero) group name is leaved in config. Let's fix
it.
Given iotune defaults are taken from the destination group setting
tunables to zero may require different set of zero settings in API
call. Let's prohibit removing from group while specifying different
group name then current for the sanity sake.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For example if disk is not in the group and we want to move it
there then it makes sense to specify only the group name in API call.
Currently the destination group iotune settings will be overwritten
with the disk settings which I would say is not what one would expect.
Thus let's get defaults from the group we are moving to.
And if we are moving the brand new group then is makes sense to
copy the current disk iotune settings to the group.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainSetBlockIoTune not simply sets the iotune params given in API
but use current settings for all the omitted params. Unfortunately
it uses current settings for active config when setting inactive
params. Let's fix it.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently it is possible to start a domain which have disks
in same iotune group and at the same time having different iotune
params. Both params set are passed to qemu in command line and the one
that is passed later down command line is get actually set.
Let's prohibit such configurations.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently upon successfull call to qemu's implementation of
virDomainSetBlockIoTune iotune settings are changed only for the
disk given in API if the disk is in iotune group while we need
to change the settings for all disks in the group.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, if only iotune group name is given for some disk and
no any params then later start of domain will fail. I guess it
will be convenient to allow such configuration if there is
another disk in the same iotune group with iotune params set. The
meaning is that the first disk have same iotunes and the latter.
Thus one can easily add a disk to iotune group - just add group
name parameter and no need to copy all the params.
Also let's expand iotunes params in the described case so we don't
need to refer to another disk to know iotunes and this will make
logic in many places simple.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
And introduce virDomainBlockIoTuneInfoHasAny.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some *ParseXML functions have comments stating what kind of device
they parse with an outdated list of parameters, with the exception
of virDomainFSDefParseXML which claims to parse a disk.
Remove them, assuming the function names are descriptive enough.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All the callees return either 0 or -1 so there is no need
for propagating the value. And we bail on the first error.
Remove the variable to make the function simpler.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We have a helper variable to make the code more concise,
use it consistently.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that the cleanup section is empty, eliminate the cleanup
label as well as the 'ret' variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the g_auto macros wherever possible to eliminate the cleanup
section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the virXMLFormatElement helper to format the driver element
to simplify adding further sub-elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The template still references libvirt-qemu-shim, which was at one
point the name used to refer to what we now know as virt-qemu-run.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
A recent commit added an error check for too-nested backing chains
followed by a return, even though errors above jump to cleanup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: b168fa88b8
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Remove bogus G_GNUC_UNUSED attribute and add a missing space.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: d600667278
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The algorithm is used in two places to find the parent checkpoint object
which contains given disk and then uses data from the disk. Additionally
the code is written in a very non-obvious way. Factor out the lookup of
the disk into a function which also simplifies the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The function has no users now and there's no need for it as the common
pattern is to look up the whole disk object anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If a disk is unplugged and then the user tries to delete a checkpoint
the code would try to use NULL node name as it was not checked.
Fix this by fetching the whole disk definition object and verifying it
was found.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Lookup the whole disk definition rather than just the node name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Upcoming patches will also use the domain disk definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Upcoming patches will also use the domain disk definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuCheckpointDiscard is a massive function that can be separated into
smaller bits. Extract the part that actually modifies the disk from the
metadata handling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Test code will need to know whether the virQEMUCaps object contains any
machine types already. Add a helper and expose it via 'qemu_capspriv.h'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The previous approac of just purging the alias combined with the fact
that we filled in fake machine types in the test data meant that if a
test case used an alias machine type such as 'pc' or 'q35' it would not
properly resolve to the actual data returned by qemu.
This started to be a problem since the CPU driver now looks at the
default CPU reported with the machine type.
This patch replaces the original approach of just removing the alias by
replacing it with a copy of the machine type data which the type would
alias to. This means that we are using the real data while we don't
modify the test output after every qemu upgrade.
Additionally this change will allow us to drop adding the fake machine
types later.
The test fallout is from actually excercising the CPU driver with
actual data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Separate out the internals as they will become more complex soon.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Every supported qemu is able to return the list of machine types it
supports so we can start validating it against that list. The advantage
is a better error message, and the change will also prevent having stale
test data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Debian/Ubuntu linkers are more strict that other distros requiring glib
to be linked explicitly.
macOS needs -export-dynamic instead of -Wl,--export-dynamic
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Similarly to 510d154a0b we need to prevent
doing too deeply nested backing chains and reject them with a sane error
message.
Add a loop to go through the snapshots prior to attempting actually
creating them to prevent some possible inconsistent scenarios.
We don't need to do it when reusing backing chains as we'll be
re-detecting the backing chain in that case anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Don't adopt the backing store data when reusing images provided by the
user. This will force a backing chain re-probe as users might have
passed in something unexpected in the overlay where our view of the
backing chain would not correspond.
This is done only for inactive snapshots as there we have way less
verification.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The previous "QEMU shim" proof of concept was taking an approach of only
caring about initial spawning of the QEMU process. It was then
registered with the libvirtd daemon who took over management of it. The
intent was that later libvirtd would be refactored so that the shim
retained control over the QEMU monitor and libvirt just forwarded APIs
to each shim as needed. This forwarding of APIs would require quite alot
of significant refactoring of libvirtd to achieve.
This impl thus takes a quite different approach, explicitly deciding to
keep the VMs completely separate from those seen & managed by libvirtd.
Instead it uses the new "qemu:///embed" URI scheme to embed the entire
QEMU driver in the shim, running with a custom root directory.
Once the driver is initialization, the shim starts a VM and then waits
to shutdown automatically when QEMU shuts down, or should kill QEMU if
it is terminated itself. This ought to use the AUTO_DESTROY feature but
that is not yet available in embedded mode, so we rely on installing a
few signal handlers to gracefully kill QEMU. This isn't reliable if
we crash of course, but you can restart with the same root dir.
Note this program does not expose any way to manage the QEMU process,
since there's no RPC interface enabled. It merely starts the VM and
cleans up when the guest shuts down at the end. This program is
installed to /usr/bin/virt-qemu-run enabling direct use by end users.
Most use cases will probably want to integrate the concept directly
into their respective application codebases. This standalone binary
serves as a nice demo though, and also provides a way to measure
performance of the startup process quite simply.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enables support for running the secret driver embedded to the
calling application process using a URI:
secret:///embed?root=/some/path
When using the embedded mode with a root=/var/tmp/embed, the
driver will use the following paths:
configDir: /var/tmp/embed/etc/secrets
stateDir: /var/tmp/embed/run/secrets
These are identical whether the embedded driver is privileged
or unprivileged.
This compares with the system instance which uses
configDir: /etc/libvirt/secrets
stateDir: /var/lib/libvirt/secrets
When an embedded instance of the secret driver is open, any other
embedded drivers will automatically use the embedded secret driver.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enables support for running QEMU embedded to the calling
application process using a URI:
qemu:///embed?root=/some/path
Note that it is important to keep the path reasonably short to
avoid risk of hitting the limit on UNIX socket path names
which is 108 characters.
When using the embedded mode with a root=/var/tmp/embed, the
driver will use the following paths:
logDir: /var/tmp/embed/log/qemu
swtpmLogDir: /var/tmp/embed/log/swtpm
configBaseDir: /var/tmp/embed/etc/qemu
stateDir: /var/tmp/embed/run/qemu
swtpmStateDir: /var/tmp/embed/run/swtpm
cacheDir: /var/tmp/embed/cache/qemu
libDir: /var/tmp/embed/lib/qemu
swtpmStorageDir: /var/tmp/embed/lib/swtpm
defaultTLSx509certdir: /var/tmp/embed/etc/pki/qemu
These are identical whether the embedded driver is privileged
or unprivileged.
This compares with the system instance which uses
logDir: /var/log/libvirt/qemu
swtpmLogDir: /var/log/swtpm/libvirt/qemu
configBaseDir: /etc/libvirt/qemu
stateDir: /run/libvirt/qemu
swtpmStateDir: /run/libvirt/qemu/swtpm
cacheDir: /var/cache/libvirt/qemu
libDir: /var/lib/libvirt/qemu
swtpmStorageDir: /var/lib/libvirt/swtpm
defaultTLSx509certdir: /etc/pki/qemu
At this time all features present in the QEMU driver are available when
running in embedded mode, availability matching whether the embedded
driver is privileged or unprivileged.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The driver URI scheme:
"$drivername:///embed?root=/some/path"
enables a new way to use the drivers by embedding them directly in the
calling process. To use this the process must have a thread running the
libvirt event loop. This URI will then cause libvirt to dynamically load
the driver module and call its global initialization function. This
syntax is applicable to any driver, but only those will have been
modified to support a custom root directory and embed URI path will
successfully open.
The application can now make normal libvirt API calls which are all
serviced in-process with no RPC layer involved.
It is required to specify an explicit root directory, and locks will be
acquired on this directory to avoid conflicting with another app that
might accidentally pick the same directory.
Use of '/' is not explicitly forbidden, but note that the file layout
used underneath the embedded driver root does not match the file
layout used by system/session mode drivers. So this cannot be used as
a backdoor to interact with, or fake, the system/session mode drivers.
Libvirt will create arbitrary files underneath this root directory. The
root directory can be kept untouched across connection open attempts if
the application needs persistence. The application is responsible for
purging everything underneath this root directory when finally no longer
required.
Even when a virt driver is used in embedded mode, it is still possible
for it to in turn use functionality that calls out to other secondary
drivers in libvirtd. For example an embedded instance of QEMU can open
the network, secret or storage drivers in the system libvirtd.
That said, the application would typically want to at least open an
embedded secret driver ("secret:///embed?root=/some/path"). Note that
multiple different embedded drivers can use the same root prefix and
co-operate just as they would inside a normal libvirtd daemon.
A key thing to note is that for this to work, the application that links
to libvirt *MUST* be built with -Wl,--export-dynamic to ensure that
symbols from libvirt.so are exported & thus available to the dynamically
loaded driver module. If libvirt.so itself was dynamically loaded then
RTLD_GLOBAL must be passed to dlopen().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The intent here is to allow the virt drivers to be run directly embedded
in an arbitrary process without interfering with libvirtd. To achieve
this they need to store all their configuration & state in a separate
directory tree from the main system or session libvirtd instances.
This can be useful for doing testing of the virt drivers in "make check"
without interfering with the user's own libvirtd instances.
It can also be used for applications using KVM/QEMU as a piece of
infrastructure to build an service, rather than for general purpose
OS hosting. A long standing example is libguestfs, which would prefer
if its temporary VMs did show up in the main libvirtd VM list, because
this confuses apps such as OpenStack Nova. A more recent example would
be Kata which is using KVM as a technology to build containers.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If a domain is configured to have an egl-headless display and a virtio
video device, virgl will be enabled automatically within the guest, even
if the video device is configured with accel3d='no'.
In this case we should explicitly pass 'virgl=off' to qemu.
See https://bugzilla.redhat.com/show_bug.cgi?id=1791236 for more
information.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since v4.2-rc0, QEMU introduced a builtin rng backend that uses
getrandom() syscall to generate random. Add it to libvirt with the
backend model 'builtin'.
https://bugzilla.redhat.com/show_bug.cgi?id=1785091
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'builtin' rng backend model can be used as following:
<rng model='virtio'>
<backend model='builtin'/>
</rng>
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For qemu object like rng-builtin, there are no properties after id
property. We should always set comma after object id. Otherwise it will
cause trailing comma on object:
-object rng-builtin,id=ID,
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It is used to check if qemu is capable of rng-builtin object.
This object is added since qemu-4.2.0-rc0, commit 6c4e9d48.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since v5.6.0-48-g270583ed98 we try to cache domain capabilities,
i.e. store filled virDomainCaps in a hash table in virQEMUCaps
for future use. However, there's a race condition in the way it's
implemented. We use virQEMUCapsGetDomainCapsCache() to obtain the
pointer to the hash table, then we search the hash table for
cached data and if none is found the domcaps is constructed and
put into the table. Problem is that this is all done without any
locking, so if there are two threads trying to do the same, one
will succeed and the other will fail inserting the data into the
table.
Also, the API looks a bit fishy - obtaining pointer to the hash
table is dangerous.
The solution is to use a mutex that guards the whole operation
with the hash table. Then, the API can be changes to return
virDomainCapsPtr directly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1791790
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When fixing [1] I've ran attached reproducer and had it spawn
1024 threads and query capabilities XML in each one of them. This
lead libvirtd to hit the RLIMIT_NOFILE limit which was kind of
expected. What wasn't expected was a subsequent segfault. It
happened because virCPUProbeHost failed and returned NULL. We've
taken the NULL and passed it to virCapabilitiesHostNUMARef()
which dereferenced it. Code inspection showed the same flas in
virQEMUDriverGetHostNUMACaps(), so I'm fixing both places.
1: https://bugzilla.redhat.com/show_bug.cgi?id=1791790
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virCapabilitiesGetNodeInfo() function has the usual return
value semantics for integeres: a negative value means an error,
zero or a positive value means success. However, the function
call done in virCPUProbeHost() doesn't check for the return value
accordingly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Don't use ERANGE as it doesn't make much sense in the error message.
Also point out that the reply from qemu was too large which is not
obvious from the original error:
error: No complete monitor response found in 10485760 bytes: Numerical result out of range
The new message will read:
error: internal error: QEMU monitor reply exceeds buffer size (10485760 bytes)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
libvirt treats 'luks' images as raw+encryption. The logic in
qemuBlockStorageSourceCreateFormat skipped the creation if the requested
image was raw but didn't take into account the encryption.
This manifested itself e.g. when attempting to do a virsh blockcopy with
the following XML:
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/tmp/enccpy'>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</encryption>
</source>
</disk>
Where qemu would report the following error:
unable to execute QEMU command 'blockdev-add': Volume is not in LUKS format
rather than actually formatting the image first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If we get a user reporting this error message being shown it's pretty
useless in terms of actually debugging it since we don't know which hash
and which key are actually subject to the error.
This patch adds a new hash table callback which formats the
user-readable version of the hash key and reports it in the new message
which will look like:
"Duplicate hash table key 'blah'"
That way we will at least have an anchor point where to start the
search.
There are two special implementations of keys which are numeric so we
add specific printer functions for them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the user-configured name of the bitmap when merging the appropriate
bitmaps for an incremental backup so that the user can see it as
configured. Additionally expose the default bitmap name if nothing is
configured.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Pass the exportname as configured when exporting the image via NBD and
fill it with the default if it's not configured.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When using blockdev configurations the 'device' argument of
'blockdev-commit' must correspond to the topmost node in the block node
graph. Libvirt didn't do this properly in case when 'copy_on_read'
option was enabled on the disk.
Use qemuDomainDiskGetTopNodename to fix it when calling block-commit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When using blockdev configurations the 'device' argument of
'blockdev-mirror' must correspond to the topmost node in the block node
graph. Libvirt didn't do this properly in case when 'copy_on_read'
option was enabled on the disk.
Use qemuDomainDiskGetTopNodename to fix it for the blockdev-mirror calls
in qemuDomainBlockCopy and the non-shared-storage migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
There are more places which require getting the topmost nodename to be
passed to qemu. Separate it out into a new function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If a mirror job fails to start in -blockdev mode we'd not unplug the
backing files we added first because the code on the error path checked
the wrong value. 'rc' is used as status of the code which added the
images, but the state of the 'block(dev)-mirror' call is stored in 'ret'
at that point.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The virConnectGetDomainCapabilities API accepts either a binary path
to the emulator, or desired guest arch. If guest arch is not given,
then the host arch is assumed.
In the case where the binary is not given, the code tried to find the
emulator binary in the existing list of cached emulator capabilities.
This is not valid since we switched to lazy population of the cache in:
commit 3dd91af01f
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 2 13:04:26 2019 +0000
qemu: stop creating capabilities at driver startup
As a result of this change, if there are no persistent guests defined
using the requested guest architecture, virConnectGetDomainCapabilities
will fail to find an emulator binary.
The solution is to stop relying on the cached capabilities to find the
binary and instead use the same logic we use to pick default a binary
per arch when populating capabilities.
Tested-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "ps2" bus is only available on certain machines like x86. On
machines like s390x, we should refuse to add a device to this bus
instead of silently ignoring it.
Looking at the QEMU sources, PS/2 is only available if the QEMU binary
has the "i8042" device, so let's check for that and only allow "ps2"
devices if this QEMU device is available, or if we're on x86 anyway
(so we don't have to fake the QEMU_CAPS_DEVICE_I8042 capability in
all the tests that use <input ... bus='ps2'/> in their xml data).
Reported-by: Sebastian Mitterle <smitterl@redhat.com>
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1763191
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
LXC driver is not able to retrieve IP addresses from domains. This
function was not implemented yet. It can be done using DHCP lease and
ARP table. Different from QEMU, LXC does not have an agent to fetch
this info, but other sources can be used.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU driver has two functions: qemuGetDHCPInterfaces() and
qemuARPGetInterfaces() that are being used inside only one single
function. They can be turned into generic functions that other drivers
can use. This commit move both from QEMU driver tree to domain conf
tree.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simplify function logic by using g_autofree to free local variables so
that we can remove some goto statements that are used for cleanup.
Introduce a g_autoptr cleanup function for virNodeDeviceDef.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since commit <60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24> we require
GnuTLS and since commit <ac0d21c762351f58dd5d2dafa2014ed48a8b49f3>
we can actually drop the usage of WITH_GNUTLS.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The libxl driver already tries to call shutdown inhibit callback in the
right places, but only if it's set. That last part was missing,
resulting in premature shutdown when running libvirtd
--timeout=...
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The function virSecretGetSecretString calls into secret driver and is
used from other hypervisors drivers and as such makes more sense in
util.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove many imports of sys/ioctl.h which are redundant,
and conditionalize remaining usage that needs to compile
on Windows platforms.
The previous change to remove the "nonblocking" gnulib
module indirectly caused the loss of the "ioctl" gnulib
module that we did not explicitly list in bootstrap.conf
despite relying on.
Rather than re-introduce the "ioctl" module this patch
makes it redundant.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This fixes a build bug introduced by
commit fbf27730a3
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 16 11:16:51 2019 +0000
conf: add support for specifying CPU "dies" parameter
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When parsing legacy NBD backing file strings such as
'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to
VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we
actually started to generate config of the backing store on the command
line with -blockdev as the JSON code would try to format it as TCP and
fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
https://bugzilla.redhat.com/show_bug.cgi?id=1791614
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reported-by: Ming Xie <mxie@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
gmtime_r/localtime_r are mostly used in combination with
strftime to format timestamps in libvirt. This can all
be replaced with GDateTime resulting in simpler code
that is also more portable.
There is some boundary condition problem in parsing POSIX
timezone offsets in GLib which tickles our test suite.
The test suite is hacked to avoid the problem. The upsteam
GLib bug report is
https://gitlab.gnome.org/GNOME/glib/issues/1999
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GNULIB termios module ensures termios.h exists (but
is none the less empty) when building for Windows. We
already exclude usage of the functions that would exist
in a real termios.h, so having an empty termios.h is
not especially useful.
It is simpler to just put all use of termios.h related
functions behind a "#ifndef WIN32" conditional.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
G_STATIC_ASSERT() is a drop-in functional equivalent of
the GNULIB verify() macro.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirt's original atomic ops impls were largely copied
from GLib's code at the time. The only API difference
was that libvirt's virAtomicIntInc() would return a
value, but g_atomic_int_inc was void. We thus use
g_atomic_int_add(v, 1) instead, though this means
virAtomicIntInc() now returns the original value,
instead of the new value.
This rewrites libvirt's impl in terms of g_atomic_int*
as a short term conversion. The key motivation was to
quickly eliminate use of GNULIB's verify_expr() macro
which is not a direct match for G_STATIC_ASSERT_EXPR.
Long term all the callers should be updated to use
g_atomic_int* directly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need all the platforms gnulib deals with, so
this is a cut down version of GNULIB's physmem.c
code. This also allows us to integrate libvirt's
error reporting functions closer to the error cause.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Convert to use socket wrappers. Aside from the header file
include change, this requires changing close -> closesocket
since our portability isn't trying to replace the close
function.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Windows sockets take a SOCKET HANDLE object instead of a
file descriptor. Wrap them in the same way that gnulib
does so that they use C runtime file descriptors.
While we could in theory use GSocket, it is hard to get
the exact same semantics libvirt has for its current
socket usage. Wrapping the Winsock2 APIs is thus the
easiest approach in the short term.
In changing the socke wrappers we need to re-implement
the nonblocking function too, since the GNULIB impl
expects to be used with the GNULIB sockets wrappers.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All UNIX platforms we care about have openpty() in the libutil
library. Use of pty.h must also be made conditional, excluding
Win32.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GLib g_size_checked_mul() function is not quite the
same signature, and gives compiler warnings due to not
correctly casting from gsize to guint64/32. Implementing
a replacement for INT_MULTIPLY_OVERFLOW is easy enough
to do ourselves.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a vastly simpler VIR_INT64_STR_BUFLEN constant
which is large enough for all cases where we currently
use INT_BUFSIZE_BOUND. This eliminates most use of the
gnulib intprops.h header.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
RHEL7 has libcurl 7.29.0, which is the oldest of any
supported build platform. Thus we no longer need the
back compat for libcurl < 7.28.0.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Switch from old VIR_ allocation APIs to glib equivalents.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function potentially grabs both a monitor job and an agent job at
the same time. This is problematic because it means that a malicious (or
just buggy) guest agent can cause a denial of service on the host. The
presence of this function makes it easy to do the wrong thing and hold
both jobs at the same time. All existing uses have already been removed
by previous commits.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order to avoid holding an agent job and a normal job at the same
time, we want to avoid accessing the domain's definition while holding
the agent job. To achieve this, qemuAgentGetFSInfo() only returns the
raw information from the agent query to the caller. The caller can then
release the agent job and then proceed to look up the disk alias from
the vm definition. This necessitates moving a few helper functions to
qemu_driver.c and exposing the agent data structure (qemuAgentFSInfo) in
the header.
In addition, because the agent function no longer returns the looked-up
disk alias, we can't test the alias within qemuagenttest. Instead we
simply test that we parse and return the raw agent data correctly.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemuAgentDiskInfo structure is filled with information received from
the agent command response, except for the 'alias' field, which is
retrieved from the vm definition. Limit this structure only to data that
was received from the agent message.
This is another intermediate step in moving the responsibility for
searching the vmdef from qemu_agent.c to qemu_driver.c so that we can
avoid holding an agent job and a normal job at the same time.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In an effort to avoid holding both an agent and normal job at the same
time, we shouldn't access the vm definition from within qemu_agent.c
(i.e. while the agent job is being held). In preparation, we need to
store the full filesystem disk information in qemuAgentDiskInfo. In a
following commit, we can pass this information back to the caller and
the caller can search the vm definition to match the filsystem disk to
an alias.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function name doesn't give a good idea of what the function does.
Rename to qemuAgentGetFSInfoFillDisks() to make it more obvious than it
is filling in the disk information in the fsinfo struct.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Update the host CPU code to report the die_id in the NUMA topology
capabilities. On systems with multiple dies, this fixes the bug
where CPU cores can't be distinguished:
<cpus num='12'>
<cpu id='0' socket_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' core_id='1' siblings='3'/>
</cpus>
Notice how core_id is repeated within the scope of the same socket_id.
It now reports
<cpus num='12'>
<cpu id='0' socket_id='0' die_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' die_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' die_id='1' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' die_id='1' core_id='1' siblings='3'/>
</cpus>
So core_id is now unique within a (socket_id, die_id) pair.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU since 4.1.0 supports the "dies" parameter for -smp
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Recently CPU hardware vendors have started to support a new structure
inside the CPU package topology known as a "die". Thus the hierarchy
is now:
sockets > dies > cores > threads
This adds support for "dies" in the XML parser, with the value
defaulting to 1 if not specified for backwards compatibility.
For example a system with 64 logical CPUs might report
<topology sockets="4" dies="2" cores="4" threads="2"/>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When pause-before-switchover QEMU capability is enabled, we get STOP
event before MIGRATION event with postcopy-active state. To properly
handle post-copy migration and emit correct events commit
v4.10.0-rc1-4-geca9d21e6c added a hack to
qemuProcessHandleMigrationStatus which translates the paused state
reason to VIR_DOMAIN_PAUSED_POSTCOPY and emits
VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY event when migration state changes
to post-copy.
However, the code was effective on both sides of migration resulting in
a confusing VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY event on the destination
host, where entering post-copy mode is already properly advertised by
VIR_DOMAIN_EVENT_RESUMED_POSTCOPY event.
https://bugzilla.redhat.com/show_bug.cgi?id=1791458
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is only a theoretical leak, but in virChrdevAlloc() we
initialize a mutex and if creating a hash table fails,
then virChrdevFree() is called which because of incorrect check
doesn't deinit the mutex.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When opening a console to a domain, we put a tuple of {path,
virStreamPtr} into a hash table that's private to the domain.
This is to ensure only one client at most has the console stream
open. Later, when the console is closed, the tuple is removed
from the hash table and freed. Except, @path won't be freed.
==234102== 60 bytes in 5 blocks are definitely lost in loss record 436 of 651
==234102== at 0x4836753: malloc (vg_replace_malloc.c:307)
==234102== by 0x5549110: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x5562D1E: g_strdup (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x4A5A917: virChrdevOpen (virchrdev.c:412)
==234102== by 0x17B64645: qemuDomainOpenConsole (qemu_driver.c:17309)
==234102== by 0x4BC8031: virDomainOpenConsole (libvirt-domain.c:9662)
==234102== by 0x13F854: remoteDispatchDomainOpenConsole (remote_daemon_dispatch_stubs.h:9211)
==234102== by 0x13F72F: remoteDispatchDomainOpenConsoleHelper (remote_daemon_dispatch_stubs.h:9178)
==234102== by 0x4AB0685: virNetServerProgramDispatchCall (virnetserverprogram.c:430)
==234102== by 0x4AB01F0: virNetServerProgramDispatch (virnetserverprogram.c:302)
==234102== by 0x4AB700B: virNetServerProcessMsg (virnetserver.c:136)
==234102== by 0x4AB70CB: virNetServerHandleJob (virnetserver.c:153)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When resuming a domain from a save file, we read the domain XML
from the file, add it onto our internal list of domains, start
the qemu process, let it load the incoming migration stream and
resume its vCPUs afterwards. If anything goes wrong, the domain
object is removed from the list of domains and error is returned
to the caller. However, the qemu process might be left behind -
if resuming vCPUs fails (e.g. because qemu is unable to acquire
write lock on a disk) then due to a bug the qemu process is not
killed but the domain object is removed from the list.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1718707
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Since there is no guest agent in LXC world (yet), we can
implement _LEASE flag only.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We have to keep the default - querying the agent if no flag is
set.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
There is a lots of possibilities to retrieve hostname information
from domain. Libvirt could use lease information from dnsmasq to
get current hostname too. QEMU supports QEMU-agent but it can use
lease source.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
As of systemd commit:
commit d65652f1f21a4b0c59711320f34266c635393c89
Author: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
CommitDate: 2018-12-10 09:56:56 +0100
Partially unify hostname_is_valid() and dns_name_is_valid()
Dashes are no longer allowed at the end of machine names.
Trim the trailing dashes from the generated name before passing
it to machined.
https://bugzilla.redhat.com/show_bug.cgi?id=1790409
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
A new helper for trimming combinations of specified characters from
the tail of the buffer.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This leaks the FD of BPF map which means it will not be freed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
libvirt currently always reports that USB is available as a bus subsystem
type when running "virsh domcapabilities". However, this is not always
true, for example the qemu-system-s390x binary normally never has support
for USB. Thus we should only report that USB is available if there is
also a USB host controller available where we can attach USB devices.
Reported-by: Sebastian Mitterle <smitterl@redhat.com>
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1759849
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When trying to specify an input device on s390x without bus like this:
<input type='keyboard'/>
... then libvirt currently complains:
error: unsupported configuration: USB is disabled for this domain,
but USB devices are present in the domain XML
This is somewhat confusing since the user did not specify an USB
device here. Since USB is not available on s390x, we should default
to the "virtio" bus here instead.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1790189
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically there are two places where we format authentication and
encryption for a disk. The logich which formats it for backing files was
flawed though and didn't format it at all. This worked if the image
became a backing file through the means of a snapshot but not directly.
Force formatting of the source and encryption for any non-disk case to
fix the issue.
This caused problems in many places as we use the formatter to copy the
definition. Effectively any copy lost the secret definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1789310https://bugzilla.redhat.com/show_bug.cgi?id=1788898
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In v5.0.0-rc1~94 we switched from one huge switch() to an array
for translating error numbers into error messages. However, the
array is declared to have VIR_ERR_NUMBER_LAST items which makes
it impossible to spot this place by compile checking when adding
new error number.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Mention the knowledge base article which has tips how to fix the backing
chain to work with current libvirt.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Internal snapshots of a non-running domain do not carry any memory state
and restoring such a snapshot will not replace existing saved memory
state. This allows a scenario, where a user first suspends a domain into
managedsave, restores a non-running snapshot and then resumes the domain
from managedsave. After that, the guest system will run with its
previous memory state atop a different disk state. The most obvious
possible fallout from this is extensive file system corruption. Swap
content and RAID bitmaps might also be off.
This has been discussed[1] and fixed[2] from the end-user perspective for
virt-manager.
This patch marks the restore operation as risky at the libvirt level,
requiring the user to remove the saved memory state first or force the
operation.
[1] https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
[2] https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html
Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit v5.10.0-290-g3a4787a301 refactored qemuDomainGetHostdevPath to
return a single path rather than an array of paths. When the function is
called on a missing device, it will now return NULL in @path rather than
a NULL array with zero items and the callers need to be adapted
properly.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
bhyveargv2xmlmock calls virBhyveCapsBuild which in turn
calls virCPUProbeHost, probing the real host CPU. This
causes a test failure if the host CPU happens to contain
the 'arch-capabilities' feature as it triggers a call
to virHostCPUGetMSR() which fails on FreeBSD.
Fortunately we already have convenient code for mocking
the host CPU probing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In v5.10.0-508-gfbf3f3d86a, the 'error' label was removed from
bhyveParseBhyveCommandLine(), however the CONSUME_ARG() macro
still uses it. Fix the macro to return an error instead.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
src/qemu/qemu_domain_address.c:680:13: error: this statement may fall through [-Werror=implicit-fallthrough=]
switch ((virDomainFSModel) dev->data.fs->model) {
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f363af7e35
Commit v5.10.0-522-g9000b2f298 was too aggressive and removed the
'error' label from prlsdkRemoveBootDevices() even though it's
used. Luckily, it's used only from one place and we have an
alternative for it that doesn't require the label.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The point of this function is to translate virDomainOsDefFirmware
enum to qemuFirmwareOSInterface enum. However, with my commit
v5.10.0-507-g8e1804f9f6 we are passing a variable type of
virDomainLoader enum. Make the function accept both enums and
make the enum members correspond to each other.
This fixes clang build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split the formatting by fsdriver type to allow adding a new type.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Split the switch by fsdriver type to allow adding a new one.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Wire up the allocation and disposal of private data.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add an object to hold the private data and call the
allocation function if it's present in xmlopt.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This will be needed in the future for allocating private data.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the 'gluster' part and decouple the return from
the gluster_debug_level parsing to allow adding more options
to this section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Inactive VM doesn't have qemuCaps set thus we'd never properly report
that VM backups are supported only for running VMs.
Move the capability check after the active check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This is not strictly needed, but it makes sure we initialize the
@bootTime global variable. Thing is, in order to validate XATTRs
and prune those set in some previous runs of the host, a
timestamp is recorded in XATTRs. The host boot time was unique
enough so it was chosen as the timestamp value. And to avoid
querying and parsing /proc/uptime every time, the query function
does that only once and stores the boot time in a global
variable. However, the only time the query function is called is
in a child process that does lock files and changes seclabels. So
effectively, we are doing exactly what we wanted to prevent from
happening.
The fix is simple, call the virHostBootTimeInit() function which
sets the global variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The idea is to offer callers an init function that they can call
independently to ensure that the global variables get
initialized.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
After one of previous commits (v5.10.0-524-gce56408e5f) there is
a variable left unused in verify_xpath_context() which breaks the
build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
gather_scsi_host_cap() in node_device_hal.c can be greatly
simplified, given that the 'out' label is always getting
hit regardless of 'retval', which can also be eliminated.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Labels that are simply a jump to a 'return' call are
unneeded and can be replaced by the return value instead.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
While we discourage people to use the old style of specifying
UEFI for their domains (the old style is putting path to the FW
image under /domain/os/loader/ whilst the new one is using
/domain/os/@firmware), some applications might have not adapted
yet. They still rely on libvirt autofilling NVRAM path and
figuring out NVRAM template when using the old way (notably
virt-install does this). We must preserve backcompat for this
previously supported config approach. However, since we really
want distro maintainers to leave --with-loader-nvram configure
option and rely on JSON descriptors, we need to implement
autofilling of NVRAM template for the old way too.
Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778
RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
These functions are meant to replace verbose check for the old
style of specifying UEFI with a simple function call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This simplifies condition when matching FW interface by having a
single line condition instead of multiline one. Also, it prepares
the code for future expansion.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This function needs domain definition really, we don't need to
pass the whole domain object. This saves couple of dereferences
and characters esp. in more checks to come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
GLib header files annotate every API with a version number.
It is possible to define some constants before including
glib.h which will result in useful compile time warnings.
Setting GLIB_VERSION_MIN_REQUIRED will result in a warning
if libvirt uses an API that was deprecated in the declared
version, or before. Such API usage should be rewritten to
use the documented new replacement API.
Setting GLIB_VERSION_MAX_ALLOWED will result in a warning
if libvirt uses an API that was not introduced until a
version of GLib that's newer than our minimum declared
version. This avoids accidentally using functionality
that is not available on some supported platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The g_date_time_new_from_iso8601() function was introduced as
a replacement for strptime in
commit 810613a60e
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 23 15:37:26 2019 +0000
src: replace strptime()/timegm()/mktime() with GDateTime APIs set
Unfortunately g_date_time_new_from_iso8601 isn't available until
glib 2.56, and backporting it requires alot of code copying and
poking at private glib structs.
This reverts domain_conf.c back to its original parsing logic prior
to 810613a60e, but using g_date_time_new()
instead of gmtime(). The other files are then adapted to follow a
similar approach.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
g_canonicalize_filename was not introduced until glib 2.58
so we need a temporary backport of its impl.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
