Laine Stump [Sat, 22 Aug 2020 21:43:24 +0000 (17:43 -0400)]
qemu: remove unreachable code in qemuProcessStart()
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>
Michal Privoznik [Fri, 21 Aug 2020 13:49:29 +0000 (15:49 +0200)]
qemu_namespace: Don't build namespace if domain doesn't have it enabled
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: 8da362fe62766b4eee209cd3ce591ceb62299d13 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@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.
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>
qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public
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>
Michal Privoznik [Mon, 24 Aug 2020 16:29:44 +0000 (18:29 +0200)]
qemuDomainGetMemorySizeAlignment: Mark domain @def const
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>
- Update the descriptions of --current & --config flags.
For --config, the reason to rephrase "next boot" to "next start"
is: "Next boot may still imply somebody selecting "reboot" in the
guest OS and fully expecting the changes to be applied." (per Peter
Krempa)
For --current, existing documentation says:
"If *--current* is specified, affect the current guest state."
It's not entirely clear what states can "current" mean or imply. So
rephrase it in context of the other two related flags --live and
--config.
- While at it, I also took the liberty to replace the few occurrences
of "peristent domain[s]" with "persistent guest[s]"
Fix all occurrences (i.e. as many as I could spot) of this.
(Thanks: Dan Berrangé on IRC.)
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 24 Aug 2020 15:38:05 +0000 (17:38 +0200)]
qemu: Move virQEMUFileOpenAs to qemu_domain.c
Commit 43620689794507308fbd3def6992a68ee2f8fa97 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>
Peter Krempa [Thu, 16 Jul 2020 09:40:34 +0000 (11:40 +0200)]
qemuFileWrapperFDClose: move to qemu_domain.c
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>
Peter Krempa [Thu, 16 Jul 2020 09:17:47 +0000 (11:17 +0200)]
qemuOpenFile: Move to qemu_domain.c
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>
Peter Krempa [Wed, 19 Aug 2020 11:17:06 +0000 (13:17 +0200)]
qemuMigrationParamsFromJSON: Unify return value handling with other functions
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>
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).
The virshStreamInData() callback is used by virStreamSparseSendAll()
to detect whether the file the data is read from is in data or hole
section. The SendAll() will then send corresponding type of virStream
message to make server create a hole or write actual data. But the
callback uses virFileInData() even for block devices, which results in
an error. Just like in previous commit, emulate a DATA section
for block devices.
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.
This callback is called when the server sends us STREAM_HOLE
meaning there is no real data, only zeroes. For regular files
we would just seek() beyond EOF and ftruncate() to create the
hole. But for block devices this won't work. Not only we can't
seek() beyond EOF, and ftruncate() will fail, this approach won't
fill the device with zeroes. We have to do it manually.
virsh: Track if vol-upload or vol-download work over a block device
We can't use virFileInData() with block devices, but we can
emulate being in data section all the time (vol-upload case).
Alternatively, we can't just lseek() beyond EOF with block
devices to create a hole, we will have to write zeroes
(vol-download case). But to decide we need to know if the FD we
are reading data from / writing data to is a block device. Store
this information in _virshStreamCallbackData.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
virsh: Pass virshStreamCallbackDataPtr to virshStreamSink() and virshStreamSkip()
These callback will need to know more that the FD they are
working on. Pass the structure that is passed to other stream
callbacks (e.g. virshStreamSource() or virshStreamSourceSkip())
instead of inventing a new one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libvirt-storage: Document volume upload/download stream format
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.
Peter Krempa [Thu, 6 Aug 2020 17:43:51 +0000 (19:43 +0200)]
testutilsqemuschema: Add template checker for schema entries
We'll need to match that a certain part of the qemu schema hasn't grown
new properties unexpectedly. Add a helper which matches an 'object' QMP
schema entry against a template and reports errors if expected types
don't match or new entries are added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Andrea Bolognani [Wed, 19 Aug 2020 09:15:35 +0000 (11:15 +0200)]
meson: Improve RPATH handling
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:
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>
Andrea Bolognani [Mon, 24 Aug 2020 07:52:43 +0000 (09:52 +0200)]
ABOUT-NLS: Drop symlink
The ABOUT-NLS symlink pointing to po/README.rst is a leftover
from when we were using autotools as the build system, and now
that we're using Meson we can drop it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
virsh: guest-agent-timeout: set default value for optional argument
The timeout argument for guest-agent-timeout is optional but it did not
have proper default value specified. Also update the virsh man page
accordingly.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Sun, 23 Aug 2020 22:07:14 +0000 (00:07 +0200)]
README: Drop symlink
Having a README file called "README" is necessary when using
autotools, and for quite some time now we've been complying with
that requirement by having a symlink with that name pointing to
README.rst, where the actual contents live. Now that we've moved
to Meson, we can drop the symlink.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Some commands were improperly converted from original POD file. Their
names were stripped after the first dash.
Fixes: ab06dd9db37f12063c8206c110dcaf9fa626d1bc Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Tue, 18 Aug 2020 23:39:19 +0000 (01:39 +0200)]
meson: Don't hardcode /etc in APPARMOR_DIR
src/security/apparmor/meson.build builds this path dynamically
based on the value of sysconfdir, so we should do the same here
or the code and the filesystem might end up disagreeing.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Andrea Bolognani [Tue, 18 Aug 2020 23:39:18 +0000 (01:39 +0200)]
meson: Set WITH_APPARMOR_PROFILES
This variable is used in src/security/meson.build to decide
whether to install the AppArmor profiles, and at the moment
even when the user specifies -Dapparmor_profiles=true they
don't get installed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Michal Privoznik [Fri, 21 Aug 2020 12:43:21 +0000 (14:43 +0200)]
virnuma: Don't work around numa_node_to_cpus() for non-existent nodes
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: 628c93574758abb59e71160042524d321a33543f
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>
Jim Fehlig [Mon, 10 Aug 2020 22:58:59 +0000 (16:58 -0600)]
Xen: Improve parsing of PCI addresses in config converter
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.
In libvirt 6.6 stopping guests with libvirt-guests.sh is broken.
As soon as there is more than one guest one can see
`systemctl stop libvirt-guests` failing and in the log we see:
libvirt-guests.sh[2455]: Running guests on default URI:
libvirt-guests.sh[2457]: /usr/lib/libvirt/libvirt-guests.sh: 120:
local: 2a49cb0f-1ff8-44b5-a61d-806b9e52dae2: bad variable name
libvirt-guests.sh[2462]: no running guests.
That is due do mutliple guests becoming a list of UUIDs. Without
recognizing this as one single string the assignment breaks when using 'local'
(which was recently added in 6.3.0). This is because local is defined as
local [option] [name[=value] ... | - ]
which makes the shell trying handle the further part of the string as
variable names. In the error above that string isn't a valid variable
name triggering the issue that is seen.
This depends on the shell being used. POSIX shells don't have 'local'
specified yet and for the common shells it depends. It worked in bash and
bash-in-POSIX-mode, but for example dash in POSIX mode triggers the issue.
To resolve that 'textify' all assignments that are strings or potentially
can become such lists (even if they are not using the local qualifier).
Fixes: 08071ec0 "tools: variables clean-up in libvirt-guests script" Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.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>
virFDStreamMsgQueuePush: Clear pointer to passed message
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>
Michal Privoznik [Wed, 19 Aug 2020 11:35:55 +0000 (13:35 +0200)]
virdevmapper: Ignore all errors when opening /dev/mapper/control
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>
Michal Privoznik [Tue, 18 Aug 2020 10:46:38 +0000 (12:46 +0200)]
numa_conf: Properly check for caches in virDomainNumaDefValidate()
When adding support for HMAT, in f0611fe8830 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:
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:
qemu_domainjob: remove dependency on `qemuDomainDiskPrivatePtr`
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>
qemu_domain: Move a couple of function declarations to the correct file
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>
Pavel Hrdina [Tue, 18 Aug 2020 13:38:34 +0000 (15:38 +0200)]
src/meson: add missing augeas tests
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>
Peter Krempa [Mon, 17 Aug 2020 10:06:13 +0000 (12:06 +0200)]
docs: build: Fix links to 'edit this page' for kbase/manpages/internals
Commit 862cf2ace4f04dadc175caacc74448e96c625ccb modified the generator
to base edit links in the root of the repository but forgot to add the
'docs/' prefix to the code generating kbase articles, manpages and the
internals documentation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Hao Wang [Sat, 8 Aug 2020 08:19:58 +0000 (16:19 +0800)]
qemu: doCoreDump: Fix return value not expect as result
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>
Michal Privoznik [Tue, 18 Aug 2020 09:04:24 +0000 (11:04 +0200)]
virdevmapper: Handle kernel without device-mapper support
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: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e 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>
Michal Privoznik [Tue, 18 Aug 2020 09:08:15 +0000 (11:08 +0200)]
virdevmapper: Don't cache device-mapper major
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>
Pino Toscano [Mon, 17 Aug 2020 07:55:26 +0000 (09:55 +0200)]
docs: improve auth service listing
Slightly improve the list of known authentication service types:
- reword 'ssh' to mention it is used for the ssh driver (for remote
QEMU), and stop mentioning the removed Phyp driver
- add 'hyperv', used by the HyperV driver
- alphabetically sort the list
- use a bulletted list instead of a numbered one
Signed-off-by: Pino Toscano <ptoscano@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Boris Fiuczynski [Thu, 13 Aug 2020 14:03:46 +0000 (16:03 +0200)]
storage: avoid maybe-uninitialized warning by GCC 10
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>
Boris Fiuczynski [Thu, 13 Aug 2020 14:03:45 +0000 (16:03 +0200)]
tools: avoid potential null pointer dereference by GCC 10
GCC 10 complains about "arg" possibly being a NULL dereference.
Even though it might be a false positive, we can easily avoid it.
Avoiding
../tools/vsh.c: In function ‘vshCommandOptStringReq’:
../tools/vsh.c:1034:19: error: potential null pointer dereference [-Werror=null-dereference]
1034 | else if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))
| ~~~^~~~~~
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Boris Fiuczynski [Thu, 13 Aug 2020 14:03:44 +0000 (16:03 +0200)]
qemu: avoid maybe-uninitialized warning by GCC 10
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>
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.
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.
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>
Pavel Hrdina [Tue, 11 Aug 2020 13:56:54 +0000 (15:56 +0200)]
qemu: fix crash in qemuDomainSetBlkioParameters without cgroups
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.
Erik Skultety [Fri, 7 Aug 2020 11:13:39 +0000 (13:13 +0200)]
kbase: sev: Provide more details on virtio-net configuration
With virtio-net we also need to disable the iPXE option ROM otherwise
a SEV-enabled guest would not boot. While at it, fix the full machine
XML examples accordingly.
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ján Tomko [Sun, 2 Aug 2020 20:50:37 +0000 (22:50 +0200)]
bhyve: fix NULL pointer check position
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>
Pavel Hrdina [Tue, 11 Aug 2020 09:07:06 +0000 (11:07 +0200)]
vircgroupv2devices: fix counting entries in BPF map
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.
apparmor: let qemu load old shared objects after upgrades
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.
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>
Stefan Bader [Mon, 3 Aug 2020 10:21:23 +0000 (12:21 +0200)]
apparmor: allow libvirtd to call pygrub
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.
Pavel Hrdina [Fri, 7 Aug 2020 13:57:28 +0000 (15:57 +0200)]
test: fix emulator pin info in test driver
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>
Pavel Hrdina [Fri, 7 Aug 2020 13:48:27 +0000 (15:48 +0200)]
conf: fix detection of available host CPUs for vcpupin
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>
projects / thirdparty / libvirt.git / log
