These leaks were introduced in commit 15d280fa97, use g_autofree for all
cert_path pointers.
Signed-off-by: Jin Yan <jinyan12@huawei.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use virJSONValueObjectCreate rather than creating the object
piece-by-piece and use new accessors for bitmap to simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Allow to map sound playback and recording devices to host devices
using "<audio type='oss'/>" OSS audio backend.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a new device element "<audio>" which allows
to map guest sound device specified using the "<sound>"
element to specific audio backend.
Example:
<sound model='ich7'>
<audio id='1'/>
</sound>
<audio id='1' type='oss'>
<input dev='/dev/dsp0'/>
<output dev='/dev/dsp0'/>
</audio>
This block maps to OSS audio backend on the host using
/dev/dsp0 device for both input (recording)
and output (playback).
OSS is the only backend supported so far.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
bhyve supports intel hda sound devices that could be specified
on the command like using "-1:0,hda,play=$play_dev,rec=$rec_dev",
where "1:0" is a PCI address, and "$play_dev" and "$rec_dev"
point to the playback and recording device on the host respectively.
Currently, schema of the 'sound' element doesn't allow specifying
neither playback nor recording devices, so for now hardcode
/dev/dsp0, which is the first audio device on the host.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add 'ich7' sound model. This is a preparation for sound support in
bhyve, as 'ich7' is the only model it supports.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Back when macvtap support was added in commit 315baab944 in Feb. 2010
(libvirt-0.7.7), it was setup to autogenerate a name for the device if
one wasn't supplied, in the pattern "macvtap%d" (or "macvlan%d"),
similar to the way an unspecified standard tap device name will lead
to an autogenerated "vnet%d".
As a matter of fact, in commit ca1b7cc8e4 added in May 2010, the code
was changed to *always* ignore a supplied device name for macvtap
interfaces by deleting *any* name immediately during the <interface>
parsing (this was intended to prevent one domain which had failed to
completely start from deleting the macvtap device of another domain
which had subsequently been provided the same device name (this will
seem mildly ironic later). This was later fixed to only clear the
device name when inactive XML was being parsed. HOWEVER - this was
only done if the xml was <interface type='direct'> - autogenerated
names were not cleared for <interface type='network'> (which could
also result in a macvtap device).
Although the names of "vnetX" tap devices had always been
automatically cleared when parsing <interface> (see commit d1304583d
from July 2008 (!)), at the time macvtap support was added, both vnetX
and macvtapX device names were always included when formatting the
XML.
Then in commit a8be259d0c (July 2011, libvirt-0.9.4), <interface>
formatting was changed to also clear out "vnetX" device names during
XML formatting as well. However the same treatment wasn't given to
"macvtapX".
Now in 2020, there has been a report that a failed migration leads to
the macvtap device of some other unrelated guest on the destination
host losing its network connectivity. It was determined that this was
due to the domain XML in the migration containing a macvtap device
name, e.g. "macvtap0", that was already in use by the other guest on
the destination. Normally this wouldn't be a problem, because libvirt
would see that the device was already in use, and then find a
different unused name. But in this case, other external problems were
causing the migration to fail prior to selecting a macvtap device and
successfully opening it, and during error recovery, qemuProcessStop()
was called, which went through all def->nets objects and (if they were
macvtap) deleted the device specified in net->ifname; since libvirt
hadn't gotten to the point of replacing the incoming "macvtap0" with
the name of a device it actually created for this guest, that meant
that "macvtap0" was deleted, *even though it was currently in use by a
different guest*!
Whew!
So, it turns out that when formatting "migratable" XML, "vnetX"
devices are omitted, just as when formatting "inactive" XML. By making
the code in both interface parsing and formatting consistent for
"vnetX", "macvtapX", and "macvlanX", we can thus make sure that the
autogenerated (and unneeded / completely *not* wanted) macvtap device
name will not be sent with the migration XML. This way when a
migration fails, net->ifname will be NULL, and libvirt won't have any
device to try and (erroneously) delete.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Back when the original version of this chunk of code was added (commit
41b087198 in libvirt-0.8.1 in April 2010), we used virExecDaemonize()
to start the qemu process, and would continue on in the function
(which at that time was called qemudStartVMDaemon()) even if a -1 was
returned. So it was possible to get to this code with rv == -1 (it was
called "ret" in that version of the code).
In modern libvirt code, qemu is started with virCommandRun(); then we
call virPidFileReadPath(); those are the only two ways of setting "rv"
prior to this code being removed, and in either case if the new value
of rv < 0, then we immediately skip over the rest of the code to the
cleanup: label.
This means that the code being removed by this patch is
unreachable.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Even if namespaces are disabled, then due to a missing check at the
beginning of qemuDomainBuildNamespace(), the domain startup code
still tries to populate (nonexistent) domain's namespace.
Fixes: 8da362fe62
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
After the recent changes, this function is now always returning
zero. Turn it to 'void' to relieve callers from checking it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't need the auto-alignment now that the user is handling
it by hand.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The existing auto-align behavior for pSeries has the idea to
alleviate user configuration of the NVDIMM size, given that the
alignment calculation is not trivial to do (256MiB alignment
of mem->size - mem->label_size value, a.k.a guest area). We
align mem->size down to avoid end of file problems.
The end result is not ideal though. We do not touch the domain
XML, meaning that the XML can report a NVDIMM size 255MiB smaller
than the actual size the guest is seeing. It also adds one more
thing to consider in case the guest is reporting less memory
than declared, since the auto-align is transparent to the
user.
Following Andrea's suggestion in [1], let's instead do an
size alignment validation. If the NVDIMM is unaligned, error out
and suggest a rounded up value. This can be bothersome to users,
but will bring consistency of NVDIMM size between the domain XML
and the guest.
This approach will force existing non-running pSeries guests to
readjust the NVDIMM value in their XMLs, if necessary. No changes
were made for x86 NVDIMM support.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Suggested-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Next patch will use it outside of qemu_domain.c.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function is not changing the domain definition, it's only
reading from it. The function is going to be used from another
function which already takes const virDomainDef. Make the @def
const to avoid typecasting it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 4362068979 moved the function to
util/virqemu.c which is compiled also on win32 and geteuid()/getegid()
doesn't exist there.
Move it to qemu_domain.c which is compiled only when the qemu driver is
enabled. Originally I didn't want to put it here as qemu_domain.c is a
code dump for helper functions but this is the least invasive fix.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We've dumped all the snapshot helpers and related code into
qemu_driver.c. It accounted for ~10% of overal size of qemu_driver.c.
Separate the code to qemu_snapshot.c/h.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There's a lot of helper code related to the save image handling. Extract
it to qemu_saveimage.c/h.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code to qemu_domain.c so that it can be reused in other parts
of the qemu driver. 'qemu_domain' was chosen as we check the domain
state after closing the wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code to qemu_domain.c so that it can be reused in other parts
of the qemu driver. 'qemu_domain' was chosen as the permissions are
based on the domain configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory allocation to simplify the code and remove the need
for a 'cleanup:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'cleanup:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'error:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'error:' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function doesn't have an overly verbose cleanup section as there
isn't any error code path. Unify it with the rest of the functions which
will simplify adding a possible error path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is similar to one of previous patches.
When receiving stream (on virStorageVolUpload() and subsequent
virStreamSparseSendAll()) we may receive a hole. If the volume we
are saving the incoming data into is a regular file we just
lseek() and ftruncate() to create the hole. But this won't work
if the file is a block device. If that is the case we must write
zeroes so that any subsequent reader reads nothing just zeroes
(just like they would from a hole in a regular file).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When handling sparse stream, a thread is executed. This thread
runs a read() or write() loop (depending what API is called; in
this case it's virStorageVolDownload() and this the thread run
read() loop). The read() is handled in virFDStreamThreadDoRead()
which is then data/hole section aware, meaning it uses
virFileInData() to detect data and hole sections and sends
TYPE_DATA or TYPE_HOLE virStream messages accordingly.
However, virFileInData() does not work with block devices. Simply
because block devices don't have data and hole sections. What we
can do though, is to mimic being always in a DATA section.
Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For libvirt, the volume is just a binary blob and it doesn't
interpret data on volume upload/download. But as it turns out,
this unspoken assumption is not clear to our users. Document it
explicitly.
Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Right now we're unconditionally adding RPATH information to the
installed binaries and libraries, but that's not always desired.
autotools seem to be smart enough to only include that information
when targeting a non-standard prefix, so most distro packages
don't actually contain it; moreover, both Debian and Fedora have
wiki pages encouraging packagers to avoid setting RPATH:
https://wiki.debian.org/RpathIssuehttps://fedoraproject.org/wiki/RPath_Packaging_Draft
Implement RPATH logic that Does The Right Thing™ in the most
common cases, while still offering users the ability to override
the default behavior if they have specific needs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In a very distant past, we came around machines that has not
continuous node IDs. This made us error out when constructing
capabilities XML. We resolved it by utilizing strange behaviour
of numa_node_to_cpus() in which it returned a mask with all bits
set for a non-existent node. However, this is not the only case
when it returns all ones mask - if the node exists and has enough
CPUs to fill the mask up (e.g. 128 CPUs).
The fix consists of using nodemask_isset(&numa_all_nodes, ..)
prior to calling numa_node_to_cpus() to determine if the node
exists.
Fixes: 628c935747
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1860231
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There was a report on libvirt-users [1] about the domxml to/from
native converter in the Xen driver not handling PCI addresses
without a domain specification. This patch improves parsing of PCI
addresses in the converter and allows PCI addresses with only
bb:ss.f. xl.cfg(5) also allows either the dddd:bb:ss.f or bb:ss.f
format. A test has been added to check the conversion from xl.cfg
to domXML.
[1] https://www.redhat.com/archives/libvirt-users/2020-August/msg00040.html
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
After previous cleanups, some labels in some functions have
nothing but 'return' statement in them. Drop the labels and
replace 'goto'-s with respective return statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Again, instead of closing FDs explicitly, we can automatically
close them when they go out of their respective scopes.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
A cleanup function can be declared for virFDStreamMsg type so
that the structure doesn't have to be freed explicitly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All callers of virFDStreamMsgQueuePush() have the same pattern:
they explicitly set @msg passed to NULL to avoid freeing it later
on. Well, the function can take address of the pointer and clear
it for them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The buffer that allocated in the virFDStreamThreadDoRead() can be
automatically freed, or if saved into the message structure it
can be stolen.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far, only ENOENT is ignored (to deal with kernels without
devmapper). However, as reported on the list, under certain
scenarios a different error can occur. For instance, when libvirt
is running inside a container which doesn't have permissions to
talk to the devmapper. If this is the case, then open() returns
-1 and sets errno=EPERM.
Assuming that multipath devices are fairly narrow use case and
using them in a restricted container is even more narrow the best
fix seems to be to ignore all open errors BUT produce a warning
on failure. To avoid flooding logs with warnings on kernels
without devmapper the level is reduced to a plain debug message.
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When adding support for HMAT, in f0611fe883 I've introduced a
check which aims to validate /domain/cpu/numa/interconnects. As a
part of that, there is a loop which checks whether all <latency/>
with @cache attribute refer to an existing cache level. For
instance:
<cpu mode='host-model' check='partial'>
<numa>
<cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
<cache level='1' associativity='direct' policy='writeback'>
<size value='8' unit='KiB'/>
<line value='5' unit='B'/>
</cache>
</cell>
<interconnects>
<latency initiator='0' target='0' cache='1' type='access' value='5'/>
<bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
</interconnects>
</numa>
</cpu>
This XML defines that accessing L1 cache of node #0 from node #0
has latency of 5ns.
However, the loop was not written properly. Well, the check in
it, as it was always checking for the first cache in the target
node and not the rest. Therefore, the following example errors
out:
<cpu mode='host-model' check='partial'>
<numa>
<cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
<cache level='3' associativity='direct' policy='writeback'>
<size value='10' unit='KiB'/>
<line value='8' unit='B'/>
</cache>
<cache level='1' associativity='direct' policy='writeback'>
<size value='8' unit='KiB'/>
<line value='5' unit='B'/>
</cache>
</cell>
<interconnects>
<latency initiator='0' target='0' cache='1' type='access' value='5'/>
<bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
</interconnects>
</numa>
</cpu>
This errors out even though it is a valid configuration. The L1
cache under node #0 is still present.
Fixes: f0611fe883
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Both parsing and formatting of NBD migration jobs is QEMU specific and
since we're trying to create a hypervisor-agnostic module out of
qemu_domainjob.c, move the NBD XML handling bits to the qemu_domain
module instead. Additionally, move the respective NBD XML calls to
the 'parseJob'/'formatJob' callbacks of the
qemuDomainObjPrivateJobCallbacks structure.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Functions `qemuDomainRemoveInactiveJob` and
`qemuDomainRemoveInactiveJobLocked` had their declaration misplaced in
`qemu_domainjob` and were moved to `qemu_domain` where their definitions
reside.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Most of our augeas files are generated during meson setup into build
directory and we were running augeas tests only for these files.
However, we have some other augeas and config files that are not
modified during meson setup and they are only in source directories.
In order to run tests for these files we need to provide different path
to both source and build directories.
Reported-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This will be used later to specify different include directories for
augparse binary to run augeas tests.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case qemuDumpToFd() returns zero followed by a VIR_CLOSE(fd) fail,
we'd jump to the "cleanup" label with "ret=0", potentially resulting in
an unexpected success return value.
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In one of my latest patch (v6.6.0~30) I was trying to remove
libdevmapper use in favor of our own implementation. However, the
code did not take into account that device mapper can be not
compiled into the kernel (e.g. be a separate module that's not
loaded) in which case /proc/devices won't have the device-mapper
major number and thus virDevMapperGetTargets() and/or
virIsDevMapperDevice() fails.
However, such failure is safe to ignore, because if device mapper
is missing then there can't be any multipath devices and thus we
don't need to allow the deps in CGroups, nor create them in the
domain private namespace, etc.
Fixes: 2249455654
Reported-by: Andrea Bolognani <abologna@redhat.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The device mapper major is needed in virIsDevMapperDevice() which
determines whether given device is managed by device-mapper. This
number is obtained by parsing /proc/devices and then stored in a
global variable so that the file doesn't have to be parsed again.
However, as it turns out this logic is flawed - the major number
is not static and can change as it can be specified as a
parameter when loading the dm-mod module.
Unfortunately, I was not able to come up with a good solution and
thus the /proc/devices file is being parsed every time we need
the device mapper major.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Change the 'make check' reference after the switch to meson/ninja.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
GCC 10 complains about variables may be used uninitialized.
Even though it might be false positives, we can easily avoid them.
Avoiding
../src/storage/storage_backend_iscsi_direct.c:634:11: error: ‘nb_block’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
634 | while (lba < nb_block) {
| ^
../src/storage/storage_backend_iscsi_direct.c:619:14: note: ‘nb_block’ was declared here
619 | uint64_t nb_block;
| ^~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:637:16: error: ‘block_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
637 | task = iscsi_write16_sync(iscsi, lun, lba, data,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
638 | block_size * to_write,
| ~~~~~~~~~~~~~~~~~~~~~~
639 | block_size, 0, 0, 0, 0, 0);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:618:14: note: ‘block_size’ was declared here
618 | uint32_t block_size;
| ^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c: In function ‘virStorageBackendISCSIDirectRefreshPool’:
../src/storage/storage_backend_iscsi_direct.c:320:39: error: ‘nb_block’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
320 | vol->target.capacity = block_size * nb_block;
| ~~~~~~~~~~~^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:306:14: note: ‘nb_block’ was declared here
306 | uint64_t nb_block;
| ^~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:320:39: error: ‘block_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
320 | vol->target.capacity = block_size * nb_block;
| ~~~~~~~~~~~^~~~~~~~~~
../src/storage/storage_backend_iscsi_direct.c:305:14: note: ‘block_size’ was declared here
305 | uint32_t block_size;
| ^~~~~~~~~~
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
GCC 10 complains about "well_formed_uri" may be used uninitialzed.
Even though it is a false positive, we can easily avoid it.
Avoiding
../src/qemu/qemu_migration.c: In function ‘qemuMigrationDstPrepareDirect’:
../src/qemu/qemu_migration.c:2920:16: error: ‘well_formed_uri’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
2920 | if (well_formed_uri) {
| ^
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
sc_spacing-check FAIL reporting a case of "Curly brackets around
single-line body:" in a recent commit.
Fixes: d9c21f4b "apparmor: allow adding permanent per guest rules"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
With qemu 5.0 and libvirt 6.6 there are new apparmor denials:
apparmor="DENIED" operation="umount" profile="libvirtd"
name="/run/libvirt/qemu/1-kvmguest-groovy-norm.dev/" comm="rpc-worker"
These are related to new issues around devmapper handling [1] and the
error path triggered by these issues now causes this new denial.
There are already related rules for mounting and it seems right to
allow also the related umount.
[1]: https://www.redhat.com/archives/libvir-list/2020-August/msg00236.html
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The design of apparmor in libvirt always had a way to define custom
per-guest rules as described in docs/drvqemu.html and [1].
A fix meant to clean the profiles after guest shutdown was a bit
overzealous and accidentially removed this important admin feature as
well.
Therefore reduce the --delete option of virt-aa-helper to only delete
the .files that would be re-generated in any case.
Users/Admins are always free to clean the profiles themselve if they
prefer a clean directory - they will be regenerated as needed. But
libvirt should never remove the base profile meant to allow per-guest
overrides and thereby break a documented feature.
[1]: https://gitlab.com/apparmor/apparmor/-/wikis/Libvirt#advanced-usage
Fixes: eba2225b "apparmor: delete profile on VM shutdown"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If we don't have cgroups available and user tries to update blkio
parameters for running VM it will crash.
It should have been protected by the virCgroupHasController() check but
it was never called if the API was executed without any flags.
We call virDomainObjGetDefs() which sets `def` and `persistentDef` based
on the flags and these two variables should be used to figure out if we
need to update LIVE, CONFIG or both states.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808293
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
src/bhyve/bhyve_parse_command.c:437:9: warning: Either the condition
'!config' is redundant or there is possible null pointer dereference:
config. [nullPointerRedundantCheck]
src/bhyve/bhyve_parse_command.c:280:23: warning: Either the condition
'!separator' is redundant or there is pointer arithmetic
with NULL pointer. [nullPointerArithmeticRedundantCheck]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
BPF syscall BPF_MAP_GET_NEXT_KEY returns -1 if something fails but it
will also return -1 if trying to get next key using the last key in the
map with errno set to ENOENT.
If there are VMs running and libvirtd is restarted and user tries to
call some cgroup devices operation on a VM we need to get the count of
entries in BPF map and it fails which will result in error when trying
to attach/detech devices.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1833321
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since [1] qemu can after upgrade fall back to pre-upgrade modules
to still be able to dynamically load qemu-module based features.
The paths for these modules are pre-defined by the code and should
be allowed to be mapped and loaded from which will allow packagers
avoiding the inability of late feature load [2] after package upgrades.
[1]: https://github.com/qemu/qemu/commit/bd83c861
[2]: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange redhat com>
On some architectures (ppc, s390x, sparc, arm) qemu will read auxv
to detect hardware capabilities via qemu_getauxval.
Allow that access read-only for the entry owned by the current
qemu process.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Allow qemu to read @{PROC}/sys/vm/overcommit_memory.
This is read on guest start-up and (as read-only) not a
critical secret that has to stay hidden.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Jamie Strandboge <jamie@ubuntu.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When using xen through libxl in Debian/Ubuntu it needs to be able to
call pygrub.
This is placed in a versioned path like /usr/lib/xen-4.11/bin.
In theory the rule could be more strict by rendering the libexec_dir
setting pkg-config can derive from libbxen-dev. But that would make
particular libvirt/xen packages version-depend on each other. It seems
more reasonable to avoid these versioned dependencies and use a wildcard
rule instead as it is already in place for libxl-save-helper.
Note: This change was in Debian [1] and Ubuntu [2] for quite some time
already.
[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=931768
[2]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1326003
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
/etc/pki/qemu is a pki path recommended by qemu tls docs [1]
and one that can cause issues with spice connections when missing.
Add the path to the allowed list of pki paths to fix the issue.
Note: this is active in Debian/Ubuntu [1] for quite a while already.
[1]: https://www.qemu.org/docs/master/system/tls.html
[2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930100
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Following the rationale from commit
<2020c6af8a8e4bb04acb629d089142be984484c8> we should do the same thing
for iothread info as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit <6328da04285d9f65cb323d399f731c20caf63f5a> introduced
testDomainGetEmulatorPinInfo() into test driver but used
virHostCPUGetCount() function to get the number of host CPUs.
This would be correct for other drivers but in test driver we must not
depend on the host, we have to use hard-coded host representation that
we have in test driver.
Follows the logic of testDomainGetVcpuPinInfo().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit <2020c6af8a8e4bb04acb629d089142be984484c8> fixed an issue with
QEMU driver by reporting offline CPUs as well. However, doing so it
introduced a regression into libxl and test drivers by completely
ignoring the passed `hostcpus` variable.
Move the virHostCPUGetAvailableCPUsBitmap() out of the helper into QEMU
driver so it will not affect other drivers which gets the number of host
CPUs differently.
This was uncovered by running libvirt-dbus test suite which counts on
the fact that test driver has hard-coded host definition and must not
depend on the host at all.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is not expose in most historical versions of glibc, nor
non-glibc impls. We must use our wrapper API instead.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If a Ctrl-C arrives while we are in the middle of executing the
virDomainCreateXML call, we will have no "virDomainPtr" object
available, but QEMU may none the less be running.
This means we'll never try to stop the QEMU process before we
honour the Ctrl-C and exit.
To deal with this race we need to postpone quit of the event
loop if it is requested while in the middle of domain startup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a race between vir_event_thread_finalize and
virEventThreadWorker in releasing the last reference on
the GMainContext. If virEventThreadDataFree() runs after
vir_event_thread_finalize releases its reference, then
it will release the last reference on the GMainContext.
As a result g_autoptr cleanup on the GSource will access
free'd memory.
The race can be seen in non-deterministic crashes of the
virt-run-qemu program during its shutdown, but could
also likely affect the main libvirtd QEMU driver:
Thread 2 (Thread 0x7f508ffff700 (LWP 222813)):
#0 0x00007f509c8e26b0 in malloc_consolidate (av=av@entry=0x7f5088000020) at malloc.c:4488
#1 0x00007f509c8e4b08 in _int_malloc (av=av@entry=0x7f5088000020, bytes=bytes@entry=2048) at malloc.c:3711
#2 0x00007f509c8e6412 in __GI___libc_malloc (bytes=2048) at malloc.c:3073
#3 0x00007f509d6e925e in g_realloc (mem=0x0, n_bytes=2048) at gmem.c:164
#4 0x00007f509d705a57 in g_string_maybe_expand (string=string@entry=0x7f5088001f20, len=len@entry=1024) at gstring.c:102
#5 0x00007f509d705ab6 in g_string_sized_new (dfl_size=dfl_size@entry=1024) at gstring.c:127
#6 0x00007f509d708c5e in g_test_log_dump (len=<synthetic pointer>, msg=<synthetic pointer>) at gtestutils.c:3330
#7 0x00007f509d708c5e in g_test_log
(lbit=G_TEST_LOG_ERROR, string1=0x7f508800fcb0 "GLib:ERROR:ghash.c:377:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > 0)", string2=<optimized out>, n_args=0, largs=0x0) at gtestutils.c:975
#8 0x00007f509d70af2a in g_assertion_message
(domain=<optimized out>, file=0x7f509d7324a2 "ghash.c", line=<optimized out>, func=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", message=<optimized out>)
at gtestutils.c:2504
#9 0x00007f509d70af8e in g_assertion_message_expr
(domain=domain@entry=0x7f509d72d76e "GLib", file=file@entry=0x7f509d7324a2 "ghash.c", line=line@entry=377, func=func@entry=0x7f509d732750 <__func__.11348> "g_hash_table_lookup_node", expr=expr@entry=0x7f509d732488 "hash_table->ref_count > 0") at gtestutils.c:2555
#10 0x00007f509d6d197e in g_hash_table_lookup_node (hash_table=0x55b70ace1760, key=<optimized out>, hash_return=<synthetic pointer>) at ghash.c:377
#11 0x00007f509d6d197e in g_hash_table_lookup_node (hash_return=<synthetic pointer>, key=<optimized out>, hash_table=0x55b70ace1760) at ghash.c:361
#12 0x00007f509d6d197e in g_hash_table_remove_internal (hash_table=0x55b70ace1760, key=<optimized out>, notify=1) at ghash.c:1371
#13 0x00007f509d6e0664 in g_source_unref_internal (source=0x7f5088000b60, context=0x55b70ad87e00, have_lock=0) at gmain.c:2103
#14 0x00007f509d6e1f64 in g_source_unref (source=<optimized out>) at gmain.c:2176
#15 0x00007f50a08ff84c in glib_autoptr_cleanup_GSource (_ptr=<synthetic pointer>) at /usr/include/glib-2.0/glib/glib-autocleanups.h:58
#16 0x00007f50a08ff84c in virEventThreadWorker (opaque=0x55b70ad87f80) at ../../src/util/vireventthread.c:114
#17 0x00007f509d70bd4a in g_thread_proxy (data=0x55b70acf3850) at gthread.c:784
#18 0x00007f509d04714a in start_thread (arg=<optimized out>) at pthread_create.c:479
#19 0x00007f509c95cf23 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Thread 1 (Thread 0x7f50a1380c00 (LWP 222802)):
#0 0x00007f509c8977ff in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f509c881c35 in __GI_abort () at abort.c:79
#2 0x00007f509d72a823 in g_mutex_clear (mutex=0x55b70ad87e00) at gthread-posix.c:1307
#3 0x00007f509d72a823 in g_mutex_clear (mutex=mutex@entry=0x55b70ad87e00) at gthread-posix.c:1302
#4 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:582
#5 0x00007f509d6e1a84 in g_main_context_unref (context=0x55b70ad87e00) at gmain.c:541
#6 0x00007f50a08ffabb in vir_event_thread_finalize (object=0x55b70ad83180 [virEventThread]) at ../../src/util/vireventthread.c:50
#7 0x00007f509d9c48a9 in g_object_unref (_object=<optimized out>) at gobject.c:3340
#8 0x00007f509d9c48a9 in g_object_unref (_object=0x55b70ad83180) at gobject.c:3232
#9 0x00007f509583d311 in qemuProcessQMPFree (proc=proc@entry=0x55b70ad87b90) at ../../src/qemu/qemu_process.c:8355
#10 0x00007f5095790f58 in virQEMUCapsInitQMPSingle
(qemuCaps=qemuCaps@entry=0x55b70ad88010, libDir=libDir@entry=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=runUid@entry=107, runGid=runGid@entry=107, onlyTCG=onlyTCG@entry=false) at ../../src/qemu/qemu_capabilities.c:5409
#11 0x00007f509579108f in virQEMUCapsInitQMP (runGid=107, runUid=107, libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", qemuCaps=0x55b70ad88010)
at ../../src/qemu/qemu_capabilities.c:5420
#12 0x00007f509579108f in virQEMUCapsNewForBinaryInternal
(hostArch=VIR_ARCH_X86_64, binary=binary@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", libDir=0x55b70ad049e0 "/tmp/virt-qemu-run-VZC9N0/lib/qemu", runUid=107, runGid=107, hostCPUSignature=0x55b70ad01320 "GenuineIntel, Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz, family: 6, model: 85, stepping: 7", microcodeVersion=83898113, kernelVersion=0x55b70ad00d60 "4.18.0-211.el8.x86_64 #1 SMP Thu Jun 4 08:08:16 UTC 2020") at ../../src/qemu/qemu_capabilities.c:5472
#13 0x00007f5095791373 in virQEMUCapsNewData (binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", privData=0x55b70ad5b8f0) at ../../src/qemu/qemu_capabilities.c:5505
#14 0x00007f50a09a32b1 in virFileCacheNewData (name=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", cache=<optimized out>) at ../../src/util/virfilecache.c:208
#15 0x00007f50a09a32b1 in virFileCacheValidate (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm", data=data@entry=0x7ffca39ffd90)
at ../../src/util/virfilecache.c:277
#16 0x00007f50a09a37ea in virFileCacheLookup (cache=cache@entry=0x55b70ad5c030, name=name@entry=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/util/virfilecache.c:310
#17 0x00007f5095791627 in virQEMUCapsCacheLookup (cache=0x55b70ad5c030, binary=0x55b70ad7dc40 "/usr/libexec/qemu-kvm") at ../../src/qemu/qemu_capabilities.c:5647
#18 0x00007f50957c34c3 in qemuDomainPostParseDataAlloc (def=<optimized out>, parseFlags=<optimized out>, opaque=<optimized out>, parseOpaque=0x7ffca39ffe18)
at ../../src/qemu/qemu_domain.c:5470
#19 0x00007f50a0a34051 in virDomainDefPostParse
(def=def@entry=0x55b70ad7d200, parseFlags=parseFlags@entry=258, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0)
at ../../src/conf/domain_conf.c:5970
#20 0x00007f50a0a464bb in virDomainDefParseNode
(xml=xml@entry=0x55b70aced140, root=root@entry=0x55b70ad5f020, xmlopt=xmlopt@entry=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22520
#21 0x00007f50a0a4669b in virDomainDefParse
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., filename=filename@entry=0x0, xmlopt=0x55b70ad5d010, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258) at ../../src/conf/domain_conf.c:22474
#22 0x00007f50a0a467ae in virDomainDefParseString
(xmlStr=xmlStr@entry=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., xmlopt=<optimized out>, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=258)
at ../../src/conf/domain_conf.c:22488
#23 0x00007f50958ce112 in qemuDomainCreateXML
(conn=0x55b70acf9090, xml=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/qemu/qemu_driver.c:1744
#24 0x00007f50a0c268ac in virDomainCreateXML
(conn=0x55b70acf9090, xmlDesc=0x55b70ad5f9e0 "<domain type='kvm'>\n <name>83</name>\n <uuid>9350639d-1c8a-4f51-a4a6-4eaf8eabe83e</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n <"..., flags=0) at ../../src/libvirt-domain.c:176
#25 0x000055b709547e7b in main (argc=<optimized out>, argv=<optimized out>) at ../../src/qemu/qemu_shim.c:289
The solution is to explicitly unref the GSource at a safe time instead
of letting g_autoptr unref it when leaving scope.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There is a fairly long standing race condition bug in glib which can hit
if you call g_source_destroy or g_source_unref from a non-main thread:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358
Unfortunately it is really common for libvirt to call g_source_destroy
from a non-main thread. This glib bug is the cause of non-determinstic
crashes in eventtest, and probably in libvirtd too.
To work around the problem we need to ensure that we never release
the last reference on a GSource from a non-main thread. The previous
patch replaced our use of g_source_destroy with a pair of
g_source_remove and g_source_unref. We can now delay the g_source_unref
call by using a idle callback to invoke it from the main thread which
avoids the race condition.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When COW is not explicitly requested to be disabled or enabled, the
function is supposed to do nothing on non-BTRFS file systems.
Fixes commit 7230bc95aa.
https://bugzilla.redhat.com/show_bug.cgi?id=1866157
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
and stop erroneously equating NULL with "". The latter means that the
element has empty content, while the former means there was an error
during parsing (either internal with the parser, or the content of the
XML was bad).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Many of our calls to xmlNodeGetContent() (which are now all via
virXMLNodeContentString() are failing to check for a NULL return. We
need to remedy that, but in order to make the remedy simpler, let's
log an error in virXMLNodeContentString(), so that the callers don't
all individually need to (since it would be the same error message for
all of them anyway).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainBlkioDeviceParseXML() calls xmlNodeGetContent() multiple
times in a loop, but can easily be refactored to call it once for all
element nodes, and then use the result of that one call in each of the
(mutually exclusive) blocks that previously each had their own call to
xmlNodeGetContent.
This is being done in order to reduce the number of changes needed in
an upcoming patch that will eliminate the lack of checking for NULL on
return from xmlNodeGetContent().
As part of the simplification, the while() loop has been changed into
a for() so that we can use "continue" without bypassing the
"node = node->next".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We should prevent inlining of symbols from the driver .so files that are
mocked, as well as those in the main libvirt.so
This isn't fixing any currently known problem, just trying to prevent
future issues.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We already allow controlling the initiator IQN for iSCSI based disks.
Add the same for host devices.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Whenever libvirt is upgraded on a Debian system, the user will be
prompted along the lines of
Configuration file '/etc/libvirt/qemu/networks/default.xml'
==> Modified (by you or by a script) since installation.
==> Package distributor has shipped an updated version.
What would you like to do about it ? Your options are:
Y or I : install the package maintainer's version
N or O : keep your currently-installed version
D : show the differences between the versions
Z : start a shell to examine the situation
The default action is to keep your current version.
*** default.xml (Y/I/N/O/D/Z) [default=N] ? d
--- /etc/libvirt/qemu/networks/default.xml 2020-08-04 12:57:25.450911143 +0200
+++ /etc/libvirt/qemu/networks/default.xml.dpkg-new 2020-08-03 22:47:15.000000000 +0200
@@ -1,19 +1,11 @@
-<!--
-WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
-OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
- virsh net-edit default
-or other application using the libvirt API.
--->
-
<network>
<name>default</name>
- <uuid>612a2cab-72fb-416d-92bc-4d9e597bfb63</uuid>
- <forward mode='nat'/>
- <bridge name='virbr0' stp='on' delay='0'/>
- <mac address='52:54:00:1f:03:79'/>
- <ip address='192.168.122.1' netmask='255.255.255.0'>
+ <uuid>d020b839-4379-492c-aa74-eab7365076e6</uuid>
+ <bridge name="virbr0"/>
+ <forward/>
+ <ip address="192.168.122.1" netmask="255.255.255.0">
<dhcp>
- <range start='192.168.122.2' end='192.168.122.254'/>
+ <range start="192.168.122.2" end="192.168.122.254"/>
</dhcp>
</ip>
</network>
The UUID situation should probably be handled the same way it is
in the spec file by stripping it, and in general we could behave
much better towards users, but one part of the diff that
immediately stands out is that some lines are highlighted not
because they are semantically different, but simply because they
use different types of quotes around attributes.
Since the canonical version of all libvirt XML documents (as
returned by the various vir*GetXMLDesc() APIs) as well as the
on-disk representations use single quotes, let's use the same
for configuration files we install as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If there's a list of mdevs to be assigned to a domain, but one of them
(NOT the first) is already assigned to a different domain we're going
to crash in the qemuProcessStop phase in
virMediatedDeviceListFindIndex, because some of the pointers in
mgr->activeMediatedHostdevs are dangling. This is due to
virMediatedDeviceListMarkDevices using cleanup instead of rollback when
we find out that a device is already taken.
Reproducer steps:
1. start vm1 with mdev1
2. start vm2 with mdev2, mdev1 (the order is important!)
Backtrace:
#0 0x0000ffffb8c36250 in strcmp
#1 0x0000ffffb9b80754 in virMediatedDeviceListFindIndex
#2 0x0000ffffb9b80870 in virMediatedDeviceListFind
#3 0x0000ffffb9c9e168 in virHostdevReAttachMediatedDevices
#4 0x0000ffff9949f724 in qemuHostdevReAttachMediatedDevices
#5 0x0000ffff9949f7f8 in qemuHostdevReAttachDomainDevices
#6 0x0000ffff994bcd70 in qemuProcessStop
#7 0x0000ffff994bf4e0 in qemuProcessStart
Signed-off-by: Binfeng Wu <wubinfeng@huawei.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The header log_manager.h doesn't use anything from log_protocol.h and
the only other place than logging using log_protocol.h is qemu_command.c
where we can include log_protocol.h directly to have enum value
VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE available.
Fixes race-condition compilation error with meson:
In file included from ../tests/qemuhotplugmock.c:21:
In file included from ../src/qemu/qemu_hotplug.h:25:
In file included from ../src/qemu/qemu_domain.h:42:
../src/logging/log_manager.h:25:10: fatal error: 'logging/log_protocol.h' file not found
^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Reported-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
During the switch to meson, one of the patches mistakenly changed the
runtime socket prefix for {libvirtd, virtproxyd} to "libvirtd-" from
the original "libvirt-". Not to be mistaken with the systemd unit name
which actually follows the daemon name, IOW the systemd unit name
remains as e.g. "libvirtd.socket", but the actual unix socket created
on the filesystem that the daemon binds to must be named "libvirt-sock"
and not "libvirtd-sock".
Fixes: dd4f2c73ad
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can use qemuDomainSetupInput() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupRNG() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupChardev() to obtain the path that we
need to unlink() from within domain's namespace. Note, while
previously we unlinked only VIR_DOMAIN_CHR_TYPE_DEV chardevs,
with this change we unlink some other types too - exactly those
types we created when plugging the device in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use qemuDomainSetupMemory() to obtain the path that we
need to unlink() from within domain's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In my attempt to deduplicate the code, we can use
qemuDomainSetupHostdev() to obtain the list of paths to unlink
and then pass it to qemuDomainNamespaceUnlinkPaths() to unlink
them in a single fork.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, the only caller qemuDomainNamespaceUnlinkPath() will
always pass a single path to unlink, but similarly to
qemuDomainNamespaceMknodPaths() - there are a few callers that
would like to pass two or more files to unlink at once (held in a
string list). Make the @paths argument a string list then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Simirarly to qemuDomainAttachDeviceMknodHelper() which was
modified just a couple of commits ago, modify the unlink helper
which is called on device detach so that it can unlink multiple
files in one go instead of forking off for every single one of
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanup, creating /dev nodes from pre-exec hook is
no longer needed and thus can be removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain SEV into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain loader into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain RNGs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain inputs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain graphics (render node)
into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain TPM into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain chardevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain memory (nvdimms) into
daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain hostdevs into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in one of previous commits, populating domain's
namespace from pre-exec() hook is dangerous. This commit moves
population of the namespace with domain disks into daemon's
namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in previous commit, populating domain's namespace
from pre-exec() hook is dangerous. This commit moves population
of the namespace with basic /dev nodes (e.g. /dev/null, /dev/kvm,
etc.) into daemon's namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Okay, here is the deal. Currently, the way we build namespace is
very fragile. It is done from pre-exec hook when starting a
domain, after we mass closed all FDs and before we drop
privileges and exec() QEMU. This fact poses some limitations onto
the namespace build code, e.g. it has to make sure not to keep
any FD opened (not even through a library call), because it would
be leaked to QEMU. Also, it has to call only async signal safe
functions. These requirements are hard to meet - in fact as of my
commit v6.2.0-rc1~235 we are leaking a FD into QEMU by calling
libdevmapper functions.
To solve this issue and avoid similar problems in the future, we
should change our paradigm. We already have functions which can
populate domain's namespace with nodes from the daemon context.
If we use them to populate the namespace and keep only the bare
minimum in the pre-exec hook, we've mitigated the risk.
Therefore, the old qemuDomainBuildNamespace() is renamed to
qemuDomainUnshareNamespace() and new qemuDomainBuildNamespace()
function is introduced. So far, the new function is basically a
NOP and domain's namespace is still populated from the pre-exec
hook - next patches will fix it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim to make it look as close to
qemuDomainNamespaceSetupDisk() as possible. The latter will call
the former and this change makes that diff easier to read.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Every caller does the same - counts the number of items in a
string list they have, only to pass the number to
qemuDomainNamespaceMknodPaths(). This is needless - the function
can accept the string list and count the items itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the previous commit prepared the helper function run in a
forked off helper (with corresponding struct), this commit
modifies the caller, which now create all files requested in a
single process and does not fork off for every single path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, when attaching a device needs two or more /dev nodes
created into a domain, we fork off and run the helper for every
node separately. For majority of devices this is okay, because
they need no or one node created anyway. But the idea is to use
this attach code to build the namespace when starting a domain,
in which case there will be way more nodes than one.
To achieve this, the recursive approach for handling symlinks has
to be turned into an iterative one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When attaching a device into a domain, the corresponding /dev
node might need to be created in the domain's namespace. For some
types of files we call mknod(), for symlinks we call symlink(),
but for others - which exist in the host namespace - we need to
so called 'bind mount' them (which is a way of passing a
file/directory between mount namespaces). There is this condition
in qemuDomainAttachDeviceMknodRecursive() which decides whether a
bind mount will be used, move it into a separate function so that
it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This structure is going to be used from not only device attach
code, but also when building the namespace. Moreover, the code
lives in a separate file so the chances of clashing with another
name are minimal.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's not really a problem since this is a helper process that
dies as soon as the helper function returns, but the cleanup code
will be replaced with a function soon and this change prepares
the code for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While qemuDomainNamespaceMknodPaths() doesn't actually create
files in the namespace in one go (it forks for each path), it a
few commits time it will.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Functions that create a device node after domain startup (used
from hotplug) will get a list of paths they want to create and
eventually call qemuDomainNamespaceMknodPaths() which then checks
whether domain mount namespace is enabled in the first place.
Alternatively, on device hotunplug, we might want to delete a
path inside domain namespace in which case
qemuDomainNamespaceUnlinkPaths() checks whether the namespace is
enabled. While this is not dangerous, it certainly burns a couple
of CPU cycles needlessly.
Check whether mount namespace is enabled upfront.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is a lot of functions called from
qemuDomainBuildNamespace() that accept @cfg
(virQEMUDriverConfigPtr) as an argument and don't use it.
Historically, it was done so that all qemuDomainSetupAll*()
functions look the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The name of this function is not very helpful, because it doesn't
create anything, it just flips a bit in a bitmask when domain is
starting up. Move the function internals into qemu_process.c and
forget the function ever existed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemu_domain.c file is big as is and we should split it into
separate semantic blocks. Start with code that handles domain
namespaces.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
These variables are only used for assignment and have
no other effect.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In virCgroupV2BindMount there is an unused variable containing
what seem to be tmpfs mount options.
Delete it. Unlike with cgroups v1, we do not create a tmpfs
here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that everything uses g_strfreev, this function is no longer
needed.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Both accept a NULL value gracefully and virStringFreeList
does not zero the pointer afterwards, so a straight replace
is safe.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The g_strdupv function from GLib provides
the same functionality.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Also remove the temporary variable - even virStringListCopy
aborts on OOM now.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Last usage out of virlog.c was removed by
commit 91268c715c
node_device_udev: remove deprecated logging function
Also drop the virbuffer.h include - it seems it was never used
for anything else than the transitive stdarg.h include.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This function calls virLogVMessage. Move it below the definition
of virLogVMessage so it can call it even without a prototype.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The XML function is needed in the C file,
not in the header.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This file is using XML functions without including
the header.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It was needed for virAsprintf, which is now dropped.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 33ed622106
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We use an array of size VIR_NODE_MEMORY_STATS_FIELD_LENGTH
to store the string read from sysfs, but pass unbound "%s"
to sscanf.
Make the array larger by one and simply stringify that
constant as the field width specifier.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The 'dom' pointer is already dereferenced earlier.
src/vz/vz_sdk.c:249:24: warning: Either the condition 'if(dom)'
is redundant or there is possible null pointer dereference:
dom. [nullPointerRedundantCheck]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There is no distinction between Read/Write locks for resctrl from libvirt's
point of view any more.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It was created to get rid of conditional compilation in the resctrl code and
make it usable anywhere else. However this is not something that is going to be
used in other places because it is not portable and resctrl is just very
specific in this regard. And there is no reason why there could not be a
preprocessor conditional in the resctrl code. Also the interface of
virFileFlock() was very ambiguous which lead to some issues.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
That's the way it should've been all the time. It was originally the case, but
then the rework to virFileFlock() made the function ambiguous when it was
created in commit 5a0a5f7fb5, and due to that it was misused in commit
657ddeff23 and since then the lock being taken was shared rather than
exclusive.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The shmem 'name' specifies the shared memory path in '/dev/shm/',
however, we may need to change it to avoid filename conflict
when VM migrate to other host. This patch remove shmem name
consistency check.
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Role(master or peer) controls how the domain behaves on migration.
For more details about migration with ivshmem, see
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/system/ivshmem.rst;hb=HEAD
It's a optional attribute in libvirt, and qemu will choose default
role for ivshmem device if the user is not specified.
With device property 'role', the value can be 'master' or 'peer'.
- 'master' (means 'master=on' in qemu), the guest will copy
the shared memory on migration to the destination host.
- 'peer' (means 'master=off' in qemu), the migration is disabled.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Yang Hang <yanghang44@huawei.com>
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
-export-dynamic is provided by src_dep
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
This introduces intermediate static library that is required for
following remote protocol check.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
We have to compile the libvirt-admin.so outside of src/admin directory
because it depends on libvirt.so.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
There are two extra dependencies compared to automake, apparmor and
selinux. It looks like libtool is doing some magic and inheriting
dependencies from libvirt.la which inherits these dependencies from
other libraries.
Without these two dependencies in meson the compilation fails.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Drop automake like print from scripts/hyperv_wmi_generator.py as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Drop automake like print from scripts/esx_vi_generator.py as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
WARN_FLAGS are not relevant for meson as all warning flags are set to
the whole project using add_project_arguments().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Meson doesn't use .libs directory, everything is placed directly into
directories where meson.build file is used.
In order to have working tests and running libvirt directly from GIT we
need to fix all the paths pointing '.libs' directory.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
With meson we no longer have .libs directory with the actual binary so
we have to take a different approach to detect if running from build
directory.
This is not as robust as for autotools because if you select --prefix
in the build directory it will incorrectly enable the override as well
but nobody should do that.
We have to modify some of the tests to not add current build path into
PATH variable and use the full path for virsh instead. Otherwise it
would be impossible to figure out that we are running virsh from build
directory.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
There is no point of having this option in libvirt because the debug
logs can be configured using log filters.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
EXTRA_DIST is not relevant because meson makes a git copy when creating
dist archive so everything tracked by git is part of dist tarball.
The remaining ones are not converted to meson files as they are
automatically tracked by meson.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
After the switch to libnl these are no longer used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 77e7c13b2e
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
After dropping support for sanlock < 2.4,
this constant is no longer used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: c495169478
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
When trying to parse an XML with overlapping iothread scheduler
settings, the error message was rather confusing:
error: iothreadssched attributes 'vcpus' must not overlap
Pass the correct element name.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pass the scheduler element name instead of trying to reconstructing
it from the attribute name.
This has the benefit of not mixing '%s' with regular text in
translatable strings as well as preventing the confusion when
the 's' marking the plural in the element name ('vcpus') is taken
as a first letter of the 'sched' suffix.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7ea55a481d
Fixes: 99c5fe0e7c
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virDomainThreadSchedParseHelper is used for parsing both iothread
and vcpu scheduling settings. Rename its 'name' attribute to
make it obvious this refers to the attribute name, not the name of
the element (which is currently constructed from the attribute name).
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When doing an external snapshot we migrate memory to a file as a form of
taking the memory state. This creates a problem as qemu deactivates all
active bitmaps after a successful migration. This means that calling
'query-named-block-nodes' will return an empty list of bitmaps for
devices. We use the bitmap list to propagate the active bitmaps into the
overlay files being created which is required for backups to work after
a snapshot. Since we wouldn't propagate anything a subsequent backup
will fail with:
invalid argument: missing or broken bitmap 'testchck' for disk 'vda'
To fix this, we can simply collect the bitmap list prior to the
migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1862472
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
virPCIDeviceAddressPtr 'addr' is forgotten to be freed in the branch
'VIR_APPEND_ELEMENT() < 0'. Use g_autoptr instead.
Signed-off-by: Hao Wang <wanghao232@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr to free the temporary virDomainDef object created by
qemuDomainSaveInternal() when xmlin is non-NULL. Leak was added in
commit 0ea479f8f6, first appearing in libvirt 0.9.4 in August 2011.
Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The term number is used for other stats and even for hugetlb
stats in virsh man page. The term number is also more clear.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
CVE-2020-14339
When building domain's private /dev in a namespace, libdevmapper
is consulted for getting full dependency tree of domain's disks.
The reason is that for a multipath devices all dependent devices
must be created in the namespace and allowed in CGroups.
However, this approach is very fragile as building of namespace
happens in the forked off child process, after mass close of FDs
and just before dropping privileges and execing QEMU. And it so
happens that when calling libdevmapper APIs, one of them opens
/dev/mapper/control and saves the FD into a global variable. The
FD is kept open until the lib is unlinked or dm_lib_release() is
called explicitly. We are doing neither.
However, the virDevMapperGetTargets() function is called also
from libvirtd (when setting up CGroups) and thus has to be thread
safe. Unfortunately, libdevmapper APIs are not thread safe (nor
async signal safe) and thus we can't use them. Reimplement what
libdevmapper would do using plain C (ioctl()-s, /proc/devices
parsing, /dev/mapper dirwalking, and so on).
Fixes: a30078cb83
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1858260
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since we have VIR_AUTOSTRINGLIST we can use it to free string
lists used in the function automatically.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are two distinct WITH_DEVMAPPER sections in the file, for
different functions each. Rearrange the code to make some of
future commits smaller.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The storage pool code now attempts to disable COW by default on btrfs,
but management applications may wish to override this behaviour. Thus we
introduce a concept of storage pool features:
<features>
<cow state='yes|no'/>
</features>
If the <cow> feature policy is set, it will be enforced. It will always
return an hard error if COW cannot be explicitly set or unset.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This calls virFileSetCOW when building a pool with a request to attempt,
but not require, COW to be disabled. The effect is that nothing changes
on non-btrfs filesystems, but btrfs will get COW disabled on the
directory. This setting is then inherited by all newly created files in
the pool, avoiding the need for mgmt app to set "nocow" on a per-volume
basis.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When disabling COW on individual files, we now use the virFileSetCOW
method. Note that this change has a slight semantic difference to the
old implementation.
The original code reported errors but returned success when disabling
COW failed.
With this new code, we will always report an error if the user requested
disabling of COW and we could not honour it, either because btrfs
returned an error, or because the filesystem is not btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
btrfs defaults to performing copy-on-write for files. This is often
undesirable for VM images, so we need to be able to control whether this
behaviour is used.
The virFileSetCOW() will allow for this. We use a tristate, since out of
the box, we want the default behaviour attempt to disable cow, but only
on btrfs, silently do nothing on non-btrfs. If someone explicitly asks
to disable/enable cow, then we want to raise a hard error on non-btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is only used in the ESX driver where, when set to "no", it will
ignore all the checks libvirt does about the origin of the MAC address
(whether or not it's in a VMWare OUI) and forward the original one to
the ESX server telling it not to check it either.
This allows keeping a deterministic MAC address which can be useful for
licensed software which might dislike changes.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
VMX conversion parts rewritten to apply on top of previously merged
support for type='generated|static'
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When support for MAC addresses having a type='static|generated'
attribute was added in:
commit 454e5961ab
Author: Bastien Orivel <bastien.orivel@diateam.net>
Date: Mon Jul 13 16:28:53 2020 +0200
Add a type attribute on the mac address element
the VMX -> XML parser was not updated. As a result while we
accept the 'type' attribute on input, we never show it again
on 'output', so we loose information during the roundtrip.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the current formatter, the XML snippets:
<interface type='bridge'>
<mac address='00:0c:29:dd:ee:fe' type='static'/>
<source bridge='br1'/>
</interface>
<interface type='bridge'>
<mac address='aa:bb:cc:dd:ee:fd' type='generated'/>
<source bridge='br2'/>
</interface>
result in
ethernet1.present = "true"
ethernet1.networkName = "br1"
ethernet1.connectionType = "bridged"
ethernet1.addressType = "static"
ethernet1.address = "00:0c:29:dd:ee:fe"
ethernet1.checkMACAddress = "false"
ethernet2.present = "true"
ethernet2.networkName = "br2"
ethernet2.connectionType = "bridged"
ethernet2.addressType = "static"
ethernet2.address = "aa:bb:cc:dd:ee:fd"
ethernet2.checkMACAddress = "false"
which is flawed, as both type='static' and type='generated' in the XML
turn into 'static' in the VMX config.
The existence of the 'static' attribute is further overriding whether
the checkMACAddress config option is set as a side effect.
Both these pieces of flawed logic were introduced in
commit 454e5961ab
Author: Bastien Orivel <bastien.orivel@diateam.net>
Date: Mon Jul 13 16:28:53 2020 +0200
Add a type attribute on the mac address element
which intentionally added the 'checkMACAddress' side effect based on
the 'type' attribute.
With this change, we're reverting the handling of checkMACAddress
to match what existed historically. The 'type' attribute now directly
maps to the addressType attribute, so the above config becomes:
ethernet1.present = "true"
ethernet1.networkName = "br1"
ethernet1.connectionType = "bridged"
ethernet1.addressType = "static"
ethernet1.address = "00:0c:29:dd:ee:fe"
ethernet2.present = "true"
ethernet2.networkName = "br2"
ethernet2.connectionType = "bridged"
ethernet2.addressType = "generated"
ethernet2.generatedAddress = "aa:bb:cc:dd:ee:fd"
ethernet2.generatedAddressOffset = "0"
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gcc 10.1.0 on Debian sid has a bug where the bounds checking gets
confused beteen two branches:
In file included from /usr/include/string.h:495,
from ../../src/internal.h:28,
from ../../src/util/virsocket.h:21,
from ../../src/util/virsocketaddr.h:21,
from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
In function 'memcpy',
inlined from 'virNetDevGetifaddrsAddress' at ../../src/util/virnetdevip.c:914:13,
inlined from 'virNetDevIPAddrGet' at ../../src/util/virnetdevip.c:962:16:
/usr/include/arm-linux-gnueabihf/bits/string_fortified.h:34:10: error: '__builtin_memcpy' offset [16, 27] from the object at 'addr' is out of the bounds of referenced subobject 'inet4' with type 'struct sockaddr_in' at offset 0 [-Werror=array-bounds]
34 | return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
../../src/util/virnetdevip.c: In function 'virNetDevIPAddrGet':
../../src/util/virsocketaddr.h:29:28: note: subobject 'inet4' declared here
29 | struct sockaddr_in inet4;
| ^~~~~
cc1: all warnings being treated as errors
Note the source location is pointing to the "inet6" / AF_INET6 branch of
the "if", but is complaining about bounds of the "inet4" field. Changing
the code into a switch() is sufficient to avoid triggering the bug and
is arguably better code too.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are races condiction to make '/run/libvirt/qemu/dbus' directory in
virDirCreateNoFork() while concurrent start VMs, and get "failed to create
directory '/run/libvirt/qemu/dbus': File exists" error message. pre-create the
dbus directory in qemuStateInitialize.
Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove the superfuous break, as there is a 'return' before it.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Since active domains which do not have the attribute already set were
not started by libvirt that probed for CPU migratable property, we need
to check this property on reconnect and update the domain definition
accordingly.
https://bugzilla.redhat.com/show_bug.cgi?id=1857967
Reported-by: Mark Mielke <mark.mielke@gmail.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit v6.4.0-61-g201bd5db63 started to fill the default value for
//cpu/@migratable attribute according to QEMU support. However, active
domains either have the migratable attribute already set or the
capabilities we use for checking the QEMU support were created by older
libvirt which didn't probe for this specific capability. Thus we should
leave active domains alone when parsing their XMLs.
https://bugzilla.redhat.com/show_bug.cgi?id=1857967
Reported-by: Mark Mielke <mark.mielke@gmail.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use g_auto* on pointers to avoid using the 'cleanup' label.
In theory the 'ret' variable can also be discarded if the flow
of the logic is reworked. Perhaps another time.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-5-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use g_autoptr() on pointers and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-4-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-3-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Next patches will use g_autoptr() in qemuProcessQMPPtr pointers
for some cleanups in QMP code.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200717211556.1024748-2-danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Refactor networkSetIPv6Sysctls to remove repetition and reuse
of the 'field' variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ignore errors from creating "libvirt-tmp-activewrite" bitmap. This
prevents failures of finishing blockjobs if the bitmap already exists.
Note that if the bitmap exists, the worst case that can happen is that
more bits are marked as dirty in the resulting merge.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
There are two possible 'transaction' command arguments in the function.
Rename 'actions' as they deal with creating bitmaps only.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The 'libvirt-tmp-activewrite' bitmap is added during the 'pivot'
operation of block copy and active layer block commit operations
regardless of whether there are any bitmaps to merge, but was not
removed unless a bitmap was merged. This meant that subsequent attempts
to merge into the same image would fail.
Fix it by checking whether the 'libvirt-tmp-activewrite' would be used
by the code and don't skip the code which would delete it.
This is a regression introduced when we switched to the new code for
block commit in <20a7abc2d2d> and for block copy in <7bfff40fdfe5>. The
actual bug originates from <4fa8654ece>.
https://bugzilla.redhat.com/show_bug.cgi?id=1857735
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Commit 20a7abc2d2 tried to delete the possibly leftover bitmap but
neglected to call the actual monitor to do so.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The handler finalizing the active layer block commit doesn't actually
reopen the file for active layer block commit, so the comment and check
are invalid.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The infrastructure supports setting the threshold also for the <mirror>.
Mention it in the docs.
https://bugzilla.redhat.com/show_bug.cgi?id=1807741
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When doing a block copy, there is another chain of images attached to a
disk. Consider them as well when looking up a disk using nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Top level image may get two events, one with the disk target (vda) and
one with disk target with index (vda[3]) if the top level image has an
index.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The index returned by qemuDomainDiskLookupByNodename is the position in
the backing chain rather than the index we report in the XML.
Since with -blockdev they differ now and additionally the disk source
also has an index we need to fix the 'threshold' events we report:
1) If it's the top level image we must always trigger the event without
any suffix as we did until now
2) We must report the correct index
3) We must report the correct index also for the top level image, when
blockdev is used.
This means that we need to potentially emit 2 events, one for the device
without the index and then when blockdev is used and the top level image
has an index we must do it also with the index.
This will fix it for blockdev cases, while also not removing previous
semantics.
https://bugzilla.redhat.com/show_bug.cgi?id=1857204
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Rather than having labels named exit, done, exit_snooprequnlock,
skip_rename, etc, use the standard "cleanup" label. And instead of
err_exit, malformed, tear_down_tmpebchains, use "error".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This rewrite of a nested conditional produces the same results, but
eliminate a goto and corresponding label.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's possible/probable the callers to virNWFilterInstReset() make it
unnecessary to set the object's nrules to 0 after freeing all its
rules, but that same function is setting nfilters to 0, so let's do
the same for the sake of consistency.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On failure, this function would clear out and free the list of
subchains it had been called with. This is unnecessary, because the
*only* caller of this function will also clear out and free the list
of subchains if it gets a failure from ebtablesGetSubChainInsts().
(It also makes more logical sense for the function that is creating
the entire list to be the one freeing the entire list, rather than
having a function whose purpose is only to create *one item* on the
list freeing the entire list).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko redhat com>
All these cleanup/error labels were reduced to having just "return
ret" by a previous patch, so get rid of them and return directly.
This patch coincidentally fixes a bug in
networkFindUnusedBridgeName(), where we would log an error yet still
return success if we failed to find a single unused "virbrNNN" name
after checking all values of "N" from 0 - 256. Said bug was introduced
when that function was originally written, in commit a28d3e485f
(libvirt 1.2.15, 2015)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This includes standard g_autofree() as well as other objects that have
a cleanup function defined to use via g_autoptr (virCommand,
virJSONValue)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the
public API file libvirt-network.h, and we can't pollute that with glib
macro invocations, so put this in src/datatypes.h next to the other
virNetwork items.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
g_new() is used in only 3 places. Switching them to g_new0() will do
no harm, reduces confusion, and helps me sleep better at night knowing
that all allocated memory is initialized to 0 :-) (Yes, I *know* that
in all three cases the associated memory is immediately assigned some
other value. Today.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To remove dependecy of `qemuDomainJob` on job specific
paramters, a `privateData` pointer is introduced.
To handle it, structure of callback functions is
also introduced.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In 9536379da4 and 8b0cb0e666 I've tried to call
virNetSocketCheckProtocolByLookup() only if we are suspecting the
host is IPv4 or IPv6 capable (because we've found an interface
with such address). However, the code was missing dereference of
the boolean variables and thus was comparing pointers against
NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This partially reverts commit 5331c4804f.
The original commit mistakenly thought virFileCacheLookup did not set
an error. In fact the only case it doesn't set an error for is when
the cache key is NULL. This in fact the fault of the caller for passing
an invalid cache key, so doesn't need to be handled.
This caller bug was fixed by checking for a NULL binary in the
virQEMUCapsCacheLookupDefault method.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Conver the code to the new approach which uses XPath to fetch known
elements rather than looping through all XML children.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a switch statement which will not be omitted when adding potential
new types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the mdev subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the vHBA subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split up formatting of the '<address>' element rather that trying to
optimize it with formatting string hacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the SCSI subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit split out formatting of the PCI subsystem
related stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate out bits related to USB so that the logic isn't entangled in
multiple conditional statements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the formatter to the new multiple buffer based approach so that
we can easily separate it into formatters per subsys type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commits, modify the hostdev detach code to use
blockdev infrastructure to detach (i)SCSI hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to command line creation, use the blockdev helpers when
hotplugging an (i)SCSI hostdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In preparation for instantiating (i)SCSI hostdevs via -blockdev,
refactor qemuBuildHostdevSCSICommandLine to use the new infrastructure
which will do it automatically.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add convertor for creating qemuBlockStorageSourceAttachData which will
allow reusing the infrastructure which we have for attaching disks also
for hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We want to instantiate hostdevs via -blockdev too. Add a separate
capability for them for a clean transition. The new capability will be
enabled when QEMU_CAPS_BLOCKDEV is present once all code is prepared.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't (re)generate the backend alias (alias of the -drive backend for
now) internally but rather pass it in. Later on it will be replaced by
the nodename when blockdev is used depending on the capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move all (i)SCSI related code into a new function named
'qemuBuildHostdevSCSICommandLine'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now store the alias of the secrets in the status XML so there's no
need to generate it again.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When upgrading from a libvirt which didn't format private data of a
virStorageSource representing an iSCSI hostdev source, we might need to
generate some internal data so that the code still works as if it was
present in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need the alias to deal with hot-unplug of the hostdev. Use
qemuDomainSecretInfoDestroy which clears only the secrets and not the
alias. The same function is used also for handling disk secrets.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We store the config of an iSCSI hostdev in a virStorageSource structure.
Parse the private data portion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
iSCSI subsystem hostdevs store the data as a virStorageSource. Format
the private data part of the virStorageSource in the appropriate place.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
SCSI hostdevs don't have a virStorageSource associated with the backend
in certain cases. Adding a separate field to hold memory for a copy of
the nodename of the storage backend will allow reusing the blockdev
machinery also for SCSI hostdevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It doesn't make sense to format "discard" when doing a -blockdev backend
of scsi-generic used with SCSI hostdevs. Add a way to skip it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming commit will need to add another flag for the function so
convert it to a bitwise-or'd array of flags to prevent having 4
booleans.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, if we found an interface with an IPv6 address we
did not blindly trust that host is IPv6 capable (as in we can
successfully translate IPv4 addresses) but used getaddrinfo() to
confirm it. Turns out, we have use the same argument for IPv4.
For instance, in an namespace created by the following steps,
getaddrinfo("127.0.0.1", ...) fails (demonstrating by "Socket
TCP/IPv4 Accept" test case failing in virnetsockettest):
unshare -n
ip link set lo up
ip link add dummy0 type dummy
ip link set dummy0 up
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is not much sense trying to disprove host is IPv6 capable
if we know after first round (getifaddrs()) that is is not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virNetSocketCheckProtocols() function is supposed to tell
caller whether IPv4 and/or IPv6 is supported on the system. In
the initial round, it uses getifaddrs() to see if an interface
has IPv4/IPv6 address assigned and then to double check IPv6 it
uses getaddrinfo() to lookup IPv6 loopback address. Separate out
this latter code because it is going to be reused.
Since the original code lived under an #ifdef and the new
function doesn't it is marked as unused - because on some systems
it may be so.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, we've used security_context_t for variables passed
to libselinux APIs. But almost 7 years ago, libselinux developers
admitted in their API that in fact, it's just a 'char *' type
[1]. Ever since then the APIs accept 'char *' instead, but they
kept the old alias just for API stability. Well, not anymore [2].
1: 9eb9c93275
2: 7a124ca275
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Choose a TPM 2 device for the backend as default for the CRB interface
since TPM 1.2 would not work.
This patch addresses BZ 1781913: https://bugzilla.redhat.com/show_bug.cgi?id=1781913
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so
prevent the choice of TPM 1.2 when the SPAPR device model is chosen
and use a default of '2.0' (TPM 2) for the backend.
This patch addresses BZ 1781913: https://bugzilla.redhat.com/show_bug.cgi?id=1781913
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Move setting the TPM default version out of the validation function into
the post parse function.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Functions `qemuDomainObjPrivateXMLParseJob` and
`qemuDomainObjPrivateXMLFormatJob` are moved from
`qemu_domain` to `qemu_domainjob`.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`qemuDomainObjPrivatePtr` parameter was avoided being passed
as a paramter in functions `qemuDomainObjPrivateXMLParseJob`
and `qemuDomainObjPrivateXMLFormatJob`, as we already pass
`virDomainObjPtr`, which can be used to get `privateData`
pointer.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During disk hot plugging, qemuDomainAttachDeviceLive() adds the new
disk to the device list of the VM object. However, hot plugging
cdroms and floppies only updates the src variable of the original
disk device, so the newly generated disk object needs to be freed.
Signed-off-by: Jin Yan <jinyan12@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is only used in the ESX driver where, when set to "static", it will
ignore all the checks libvirt does about the origin of the MAC address
(whether or not it's in a VMWare OUI) and forward the original one to
the ESX server telling it not to check it either.
This allows keeping a deterministic MAC address which can be useful for
licensed software which might dislike changes.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove both 'error' and 'cleanup' labels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the obsolete 'error' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Next patch will use g_autoptr() in a qemuMigrationCookiePtr pointer to
modernize qemuMigrationSrcBeginPhase().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the now obsolete 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove both 'cleanup' and 'error' labels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use variable autocleanup and remove the now obsolete 'cleanup'
label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() on pointers and remove the unneeded 'cleanup'
label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree on strings and remove the 'done' label since it's
now unneeded.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add headers with declarations of geteuid/getegid
and virGetUserName/virGetGroupName.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
While moving the code, qemuDomainNamespace also was moved
to `qemu_domainjob`. Hence it is moved back to `qemu_domain`
where it will be more appropriate.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With meson introduction which is using the same CFLAGS for the whole
project some compilation errors were discovered. The wireshark plugin
library is the only one in tools directory that is not using AM_CFLAGS.
With the AM_CFLAGS we get these errors:
../../tools/wireshark/src/packet-libvirt.c: In function 'dissect_libvirt_fds':
../../tools/wireshark/src/packet-libvirt.c:348:31: error: unused parameter 'tvb' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~~~~~~^~~
../../tools/wireshark/src/packet-libvirt.c:348:41: error: unused parameter 'start' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~^~~~~
../../tools/wireshark/src/packet-libvirt.c:348:55: error: unused parameter 'nfds' [-Werror=unused-parameter]
348 | dissect_libvirt_fds(tvbuff_t *tvb, gint start, gint32 nfds)
| ~~~~~~~^~~~
At top level:
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_bool' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:88:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
88 | XDR_PRIMITIVE_DISSECTOR(bool, bool_t, boolean)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_float' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:86:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
86 | XDR_PRIMITIVE_DISSECTOR(float, gfloat, float)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:64:5: error: 'dissect_xdr_short' defined but not used [-Werror=unused-function]
64 | dissect_xdr_##xtype(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf) \
| ^~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c:80:1: note: in expansion of macro 'XDR_PRIMITIVE_DISSECTOR'
80 | XDR_PRIMITIVE_DISSECTOR(short, gint16, int)
| ^~~~~~~~~~~~~~~~~~~~~~~
../../tools/wireshark/src/packet-libvirt.c: In function 'dissect_libvirt_message':
../../tools/wireshark/src/packet-libvirt.c:423:34: error: null pointer dereference [-Werror=null-dereference]
423 | vir_xdr_dissector_t xd = find_payload_dissector(proc, type, get_program_data(prog, VIR_PROGRAM_DISSECTORS),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
424 | *(gsize *)get_program_data(prog, VIR_PROGRAM_DISSECTORS_LEN));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The current code to check XDR support was obsolete and way to
complicated.
On linux we can use pkg-config to check for libtirpc and have
the CFLAGS and LIBS configured by it as well.
On MinGW there is portablexdr library which installs header files
directly into system include directory.
On FreeBSD and macOS XDR functions are part of libc so there is
no library needed, we just need to call AM_CONDITIONAL to silence
configure which otherwise complains about missing WITH_XDR.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All of the listed functions are available in libselinux version 2.2.
Our supported OSes start with version 2.5 so there is no need to check
it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Just like in the previous commit, the stdin_path argument of
virSecurityManagerSetAllLabel() is renamed to incomingPath.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The argument (if not NULL) points to the file the domain is
restoring from. On QEMU command line this used to be '-incoming
$path', but we've switched to passing FD ages ago and thus this
argument is used only in AppArmor (which loads the profile on
domain start). Anyway, the argument does not refer to stdin,
rename it to 'incomingPath' then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The only consumer was removed in the previous commit.
This reverts commit f03a38bd1d.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Currently, when restoring from a domain the path that the domain
restores from is labelled under qemuSecuritySetAllLabel() (and after
v6.3.0-rc1~108 even outside transactions). While this grants QEMU
the access, it has a flaw, because once the domain is restored, up
and running then qemuSecurityDomainRestorePathLabel() is called,
which is not real counterpart. In case of DAC driver the
SetAllLabel() does nothing with the restore path but
RestorePathLabel() does - it chown()-s the file back and since there
is no original label remembered, the file is chown()-ed to
root:root. While the apparent solution is to have DAC driver set the
label (and thus remember the original one) in SetAllLabel(), we can
do better.
Turns out, we are opening the file ourselves (because it may live on
a root squashed NFS) and then are just passing the FD to QEMU. But
this means, that we don't have to chown() the file at all, we need
to set SELinux labels and/or add the path to AppArmor profile.
And since we want to restore labels right after QEMU is done loading
the migration stream (we don't want to wait until
qemuSecurityRestoreAllLabel()), the best way to approach this is to
have separate APIs for labelling and restoring label on the restore
file.
I will investigate whether AppArmor can use the SavedStateLabel()
API instead of passing the restore path to SetAllLabel().
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1851016
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These APIs are are basically
virSecuritySELinuxDomainSetPathLabelRO() and
virSecuritySELinuxDomainRestorePathLabel().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These APIs don't use namespaces because the
virSecurityManagerSetSavedStateLabel() runs
when the namespace doesn't exist yet and thus
the virSecurityManagerRestoreSavedStateLabel()
has to run without namespace too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These APIs were removed/renamed in v6.5.0-rc1~142 and v6.5.0-rc1~141
because they deemed unused. And if it wasn't for the RFE [1] things
would stay that way.
The RFE asks for us to not change DAC ownership on the file a domain is
restoring from. We have been doing that for ages (if not forever),
nevertheless it's annoying because if the restore file is on an NFS
remembering owner won't help - NFS doesn't support XATTRs yet. But more
importantly, there is no need for us to chown() the file because when
restoring the domain the file is opened and the FD is then passed to
QEMU. Therefore, we really need only to set SELinux and AppArmor.
This reverts bd22eec903.
This partially reverts 4ccbd207f2.
The difference to the original code is that secdrivers are now
not required to provide dummy implementation to avoid
virReportUnsupportedError(). The callback is run if it exists, if
it doesn't zero is returned without any error.
1: https://bugzilla.redhat.com/show_bug.cgi?id=1851016
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When locking files for metadata change, we open() them for R/W
access. The write access is needed because we want to acquire
exclusive (write) lock (to mutually exclude with other daemons
trying to modify XATTRs on the same file). Anyway, the open()
might fail if the file lives on a RO filesystem. Well, if that's
the case, ignore the error and continue with the next file on the
list. We won't change any seclabel on the file anyway - there is
nothing to remember then.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the light of recent commit of 9d83281382 fix the comment that
says directories can't be locked. Well, in general they can, but
not in our case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit <5b816e16968ba02def56f067774ecd9a8c8d44d7> removed hard-coded
sysconfdir path from *.conf files but missed virtproxyd.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <f650e86703847af544762d02f79c70131ff7fbab> added check for
openpty function from util library using AC_CHECK_LIB(). However, that
macro doesn't define OPENPTY_LIBS, it only defines WITH_LIBUTIL and
prepends -lutil into LIBS for the whole project.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It was introduced by commit <c606671aaad10a9bc87f226bc473a091e00a9629>
as a gnulib ldexp module and later removed by commit
<09fe607b4de8eb883c966e90aaf5563299a22738>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Fixes inconsistency with macro names for external programs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <0985a9597bb0348d46c0d18dc548a676bf0ad8e2> added unused variable
so remove it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <894556ca813ad3c4ebb01083b7971d73b4f53c8b> moved function
virSecretGetSecretString out of secret directory but forgot to update
CFLAGS in places where the include is no longer needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit <fd3b8fe7ad491c77c0b3f57110adaf64f743855e> removed objectlocking
test but forgot to remove all of the usages of LOCK_CHECKING_CFLAGS.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function sanlock_write_lockspace() was introduced in 2.7 version which
is available in all supported OSes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function sanlock_killpath() was introduced in 2.4 version and had
modified one of the arguments from `char *` into `const char *` in
version 2.7. All of this is available in all supported OSes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
SANLK_INQ_WAIT was introduced in sanlock 2.4 which is available in all
supported OSes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This check was introduced by commit
<96a02703daad4dc6663165adbc0feade9900cebd> to guard calling
sanlock_inq_lockspace() function but it used SANLK_INQ_WAIT as a
parameter which was introduced later. This was eventually fixed by
commit <238dba0f9c925359cb3b8beddd8c8ae739cb4e06>.
We can safely replace check for sanlock_inq_lockspace as that function
was introduced in sanlock-1.9. The oldest used version, sanlock-2.2,
is by Ubuntu 16.04.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was introduced together with clock-time gnulib module by commit
<d74e5a4dfc434d3a1d01856d013a7f50d910fa95> and removed from libvirt
by commit <86d223a762990c9d529065a2d3b30b6a00ea63dd>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set FLAT_NAMESPACE_FLAGS to -Wl,-flat_namespace in configure only for
macOS and use it unconditionally in Makefiles.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is no need to have DRIVER_MODULES_LIBS as it's used only for
libvirt.so. The other places are using DLOPEN_LIBS directly and dlopen
is required if building with libvirtd.
It's mandatory since <5aec02dc37623bf739d1edd8f2be3e4ad9f94ff5>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virFileIsAccessible does not return true on accessible
directories. Check whether it set EISDIR and only
then assume the directory is inaccessible.
Return 0 (not found) instead of 1 (found),
since the bridge driver taints the network based on
this return value, not whether the hook actually ran.
Remove the bogus check from virHookCall, since it already
checks the virHooksFound bitmap that was filled before
by virHookCheck.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7fa7f7eeb6
Closes: https://gitlab.com/libvirt/libvirt/-/issues/47
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
- Add a check for asm/hwcap.h header presence,
- Add a check for getauxval() function that is used
on Linux, and for elf_aux_info() which is a FreeBSD
equivalent.
This is based on a patch submitted by Mikael Urankar in
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247722.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When preparing for the removal of GNULIB commit 18dca21a32 removed the
unneeded O_DIRECTORY, but unfortunately started opening the directory for
writing which fails every time for a directory. There is also no need for that
as flock() works on O_RDONLY file descriptor as well, even for LOCK_EX.
https://bugzilla.redhat.com/1852741
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
All modified functions are similar, in all cases "cleanup" label is removed,
along with all the "goto" calls.
Signed-off-by: Nicolas Brignone <nmbrignone@gmail.com>
Reviewed-by: Laine Stump <laine@redhat.com>
