qemu_driver.c: Coverity fix in qemuNodeDeviceDetachFlags()
Commit 76f47889326c4 made qemuNodeDeviceDetachFlags() unusable due to an
'if then else if' chain that will always results in a 'return -1',
regardless of 'driverName' input.
Found by Coverity.
Fixes: 76f47889326c45d2732711bc6dd5751aaf6e5194 Reported-by: John Ferlan <jferlan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
scripts/check-aclrules.py: check ACL for domain_driver.c ACL callers
This script works under two specific conditions. For each opened file,
search for all functions that has ACL calls and store them, and see
if there is a vir*DriverPtr struct declared in it. For each implementation
found, check if there is an ACL verification inside it, and error out if
none was found. The script also supports the concept of stub, where another
function takes the responsibility for the ACL call instead of the
original API.
Unfortunately this is not enough to cover the new scenario we have now,
with domain_driver.c containing helper functions that execute the ACL
calls. The script does not store state between files because, until now,
it wasn't needed to - APIs and stubs and vir*DriverPtr declarations were
always in the same file. Also, the script will not check for ACL in functions
that does not belong to a vir*DriverPtr interface. What we have now in
domain_driver.c breaks both assumptions: the functions are in a different
file, and there is no vir*DriverPtr being implemented in the file that
uses these functions.
This patch changes check-aclrules.py to accomodate this scenario. The helpers
that have ACL checks are stored beforehand in aclFuncHelpers, allowing other
files to use them to recognize a stub situation. In case the current file
being analyzed is domain_driver.c itself, we'll do a manual check using
aclFuncHelpers to verify that these functions indeed have ACL checks.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu, libxl, hypervisor: use virDomainDriverNodeDeviceDetachFlags() helper
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.
Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.
The helper is named with the 'Flags' suffix, even when the helper itself
isn't receiving the flags from the callers, to be compliant with the
ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called
inside it and was called from the original functions. Renaming the helper
would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the
related structs inside remote_protocol.x, to be compliant with the ACL
rules.
This is not being checked at this moment, but we'll fix check-aclrules.py to
verify all the helpers that calls ACL functions in domain_driver.c shortly.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu_driver.c: validate 'driverName' earlier in qemuNodeDeviceDetachFlags()
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
libxl_driver.c: validate 'driverName' earlier in libxlNodeDeviceDetachFlags()
The validation of 'driverName' does not depend on any other state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
BiaoXiang Ye [Wed, 10 Feb 2021 05:58:05 +0000 (05:58 +0000)]
rpc: avoid crash when system time jump back
Setting the system time backward would lead to a
multiplication overflow in function virKeepAliveStart.
The function virKeepAliveTimerInternal got the same bug too.
Backtrace below:
#0 0x0000ffffae898470 in raise () from /usr/lib64/libc.so.6
#1 0x0000ffffae89981c in abort () from /usr/lib64/libc.so.6
#2 0x0000ffffaf9a36a8 in __mulvsi3 () from /usr/lib64/libvirt.so.0
#3 0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
count=count entry=0) at ../../src/rpc/virkeepalive.c:283
#4 0x0000ffffaf908560 in virNetServerClientStartKeepAlive (client=0xaaaaf954cbe0)
at ../../src/rpc/virnetserverclient.c:1628
#5 0x0000aaaac57eb6dc in remoteDispatchConnectSupportsFeature (server=0xaaaaf95309d0,
msg=0xaaaaf9549d90, ret=0xffff8c007fc0, args=0xffff8c002e70, rerr=0xffff9ea054a0,
client=0xaaaaf954cbe0) at ../../src/remote/remote_daemon_dispatch.c:5063
#6 remoteDispatchConnectSupportsFeatureHelper (server=0xaaaaf95309d0, client=0xaaaaf954cbe0,
msg=0xaaaaf9549d90, rerr=0xffff9ea054a0, args=0xffff8c002e70, ret=0xffff8c007fc0)
at ./remote/remote_daemon_dispatch_stubs.h:3503
#7 0x0000ffffaf9053a4 in virNetServerProgramDispatchCall(msg=0xaaaaf9549d90, client=0xaaaaf954cbe0,
server=0x0, prog=0xaaaaf953a170) at ../../src/rpc/virnetserverprogram.c:451
#8 virNetServerProgramDispatch (prog=0xaaaaf953a170, server=0x0, server entry=0xaaaaf95309d0,
client=0xaaaaf954cbe0, msg=0xaaaaf9549d90) at ../../src/rpc/virnetserverprogram.c:306
#9 0x0000ffffaf90a6bc in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>,
client=<optimized out>, srv=0xaaaaf95309d0) at ../../src/rpc/virnetserver.c:137
#10 virNetServerHandleJob (jobOpaque=0xaaaaf950df80, opaque=0xaaaaf95309d0)
at ../../src/rpc/virnetserver.c:154
#11 0x0000ffffaf812e14 in virThreadPoolWorker (opaque=<optimized out>)
at ../../src/util/virthreadpool.c:163
#12 0x0000ffffaf81237c in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:246
#13 0x0000ffffaea327ac in ?? () from /usr/lib64/libpthread.so.0
#14 0x0000ffffae93747c in ?? () from /usr/lib64/libc.so.6
(gdb) frame 3
#3 0x0000ffffaf8fd9e8 in virKeepAliveStart (ka=0xaaaaf954ce10, interval=interval entry=0,
count=count entry=0) at ../../src/rpc/virkeepalive.c:283
283 timeout = ka->interval - delay;
(gdb) list
278 now = time(NULL);
279 delay = now - ka->lastPacketReceived; <='delay' got a negative value
280 if (delay > ka->interval)
281 timeout = 0;
282 else
283 timeout = ka->interval - delay;
284 ka->intervalStart = now - (ka->interval - timeout);
285 ka->timer = virEventAddTimeout(timeout * 1000, virKeepAliveTimer, <= multiplication overflow
286 ka, virObjectFreeCallback);
287 if (ka->timer < 0)
(gdb) p now
$2 = 18288001
(gdb) p ka->lastPacketReceived
$3 = 1609430405
Signed-off-by: BiaoXiang Ye <yebiaoxiang@huawei.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Erik Skultety [Mon, 15 Feb 2021 12:07:08 +0000 (13:07 +0100)]
ci: Makefile: Expose CI_IMAGE_PREFIX and CI_IMAGE_TAG in 'ci-help'
Using locally built images is a useful feature; our commentaries even
mention overriding them may be useful in some scenarios. Expose the
variables in the help to let users know they can use the feature.
Formatting would definitely break, so this patch adds more spacing for
proper alignment.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Wed, 17 Feb 2021 05:58:59 +0000 (06:58 +0100)]
esx: Bump minimal version of curl
According to meson.build the minimal version of curl needed is
7.18.0 which was released in January 2008. If the minimal version
is bumped to 7.19.1 (released in November 2008) we can drop some
workarounds because this newer version provides APIs we need.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Fri, 12 Feb 2021 21:39:38 +0000 (16:39 -0500)]
esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
These are all cases when 1) the pointer is passed by reference from
the caller (ie.e. **) and expects it to be NULL on return if there is
an error, or 2) the variable holding the pointer is being checked or
re-used in the same function, but not right away.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 12 Feb 2021 19:22:21 +0000 (14:22 -0500)]
esx: reorder code to avoid need to VIR_FREE mimeType
mimeType is initialized to NULL, and then only set in one place, just
before a check (not involving mimeType) that then VIR_FREEs mimeType
if it fails. If we just reorder the code to do the check prior to
setting mimeType, then there won't be any need to VIR_FREE(mimeType)
on failure (because it will already be empty/NULL).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 12 Feb 2021 19:21:36 +0000 (14:21 -0500)]
esx: use g_steal_pointer+g_autofree on return value
If we put the potential return string into the g_autofreed tmpResult,
and the move it to the returned "result" only as a final step ater, we
can avoid the need to explicitly VIR_FREE (or g_free) on failure.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 12 Feb 2021 19:13:03 +0000 (14:13 -0500)]
esx: switch VIR_FREE->g_free in esx*Free*()
Although the three functions esxFreePrivate(), esxFreeStreamPrivate(),
and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in
theory the caller of the function might rely on "object" (the free
function's arg) being set to NULL, in practice these functions are
only called from a couple places each, and in all cases the pointer
that is passed is a local variable, and goes out of scope almost
immediately after calling the Free function, so it is safe to change
VIR_FREE() into g_free().
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 12 Feb 2021 17:53:33 +0000 (12:53 -0500)]
esx: fix memory leak by switching to g_autofree
volumeName was defined at the top of the function, then a new string
was assigned to it each time through a loop, but after the first
iteration of the loop, the previous string wasn't freed before
allocating a new string the next time. By reducing the scope of
volumeName to be just the loop, and making it g_autofree, we eliminate
the leak.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 12 Feb 2021 17:50:47 +0000 (12:50 -0500)]
esx: use g_autofree when made possible by reducing scope
These strings were being VIR_FREEd multiple times because they were
defined at the top of a function, but then set each time through a
loop. But they are only used inside that loop, so they can be
converted to use g_autofree if their definition is also placed inside
that loop.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Mon, 15 Feb 2021 14:39:19 +0000 (15:39 +0100)]
qemu: Don't lie about @ndevAlias when translating FSInfo
When virDomainGetFSInfo() is called over a QEMU/KVM domain it
results into calling of 'guest-get-fsinfo' guest agent command to
which it replies with info on guest (mounted) filesystems. When
filling return structure we also try to do basic lookup and
translate guest agent provided disk address into disk target (as
seen in domain XML). This can of course fail - guest can have
variety of disks not recorded in domain XML (iSCSI, scsi_debug,
NFS to name a few). If that's the case, a debug message is logged
and no disk target is added into the return structure.
However, due to the way our code is written the caller is led to
believe that the target was added into the structure. This may
lead to a situation where the array of disk targets (strings)
contains NULL. But our RPC structure says the array contains only
non-NULL strings. This results in somewhat 'cryptic' (at least to
users) error message:
error: Unable to get filesystem information
error: Unable to encode message payload
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919783 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 16 Feb 2021 10:27:56 +0000 (11:27 +0100)]
qemu: Move qemuAgentFSInfo array free into qemuDomainGetFSInfo()
When qemuDomainGetFSInfo() is called it calls
qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
guest agent command, parses returned JSON and returns an array of
qemuAgentFSInfo structures (well, pointers to those structs).
Then it grabs a domain job and tries to do some matching of guest
returned info against domain definition. This matching is done in
virDomainFSInfoFormat() which also frees the array of
qemuAgentFSInfo structures allocated earlier.
But this is not just. If acquiring the domain job fails (or
domain activeness check executed right after that fails) then
virDomainFSInfoFormat() is not called, leaking the array of
structs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 15 Feb 2021 16:13:22 +0000 (17:13 +0100)]
qemu: Drop needless check in virDomainFSInfoFormat()
As the very first thing, this function checks whether the number
of items inside @agentinfo array is not negative. This is
redundant as the only caller - qemuDomainGetFSInfo() already
checked for that and would not even call this function if that
was the case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 15 Feb 2021 16:52:24 +0000 (17:52 +0100)]
qemuSnapshotCreateActiveExternal: Don't thaw filesystems when freeze fails
If we didn't freeze any filesystems we should not even attempt thawing
them. Additionally 'guest-fsfreeze-freeze' fails if the filesystems are
already frozen, where thawing them may break users data integrity if
they used VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE accidentally after an
explicit virDomainFSFreeze and the next snapshot without that flag would
be taken with already thawed filesystems.
This effectively reverts 7c736bab06479ccec59df69fb79a5c06d112d8fb .
Libvirt nowadays checks whether the guest agent is connected and pings
it before issuing an command so it's very unlikely that we'd end up in a
situation where qemuSnapshotCreateActiveExternal froze filesystems and
didn't thaw them.
Additionally we now discourage the use of
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE since users have better control if
they freeze the FS themselves.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 15 Feb 2021 16:50:56 +0000 (17:50 +0100)]
api: Discourage use of VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
The flag creates additional points of failure which are hard to recover
from, such as when thawing of the filesystems fails after an otherwise
successful snapshot.
Encourage use of explicit virDomainFSFreeze/virDomainFSThaw.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 8 Feb 2021 09:23:45 +0000 (10:23 +0100)]
storagevolxml2argvdata: Rewrap all output files
Use scripts/test-wrap-argv.py to rewrap the output files so that any
further changes don't introduce churn since we are rewrapping the output
automatically now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Wed, 10 Feb 2021 17:39:11 +0000 (18:39 +0100)]
qemuMigrationSrcPerformPeer2Peer3: Don't leak 'dom_xml' on cleanup
Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code
paths jumping to cleanup prior to the deallocation which is done right
after it's not needed any more since it's a big string.
Noticed when running under valgrind:
==2204780== 8,192 bytes in 1 blocks are definitely lost in loss record 2,539 of 2,551
==2204780== at 0x483BCE8: realloc (vg_replace_malloc.c:834)
==2204780== by 0x4D890DF: g_realloc (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4DA3AF0: g_string_append_vprintf (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4917293: virBufferAsprintf (virbuffer.c:307)
==2204780== by 0x49B0B75: virDomainChrDefFormat (domain_conf.c:26109)
==2204780== by 0x49E25EF: virDomainDefFormatInternalSetRootName (domain_conf.c:28956)
==2204780== by 0x15F81D24: qemuDomainDefFormatBufInternal (qemu_domain.c:6204)
==2204780== by 0x15F8270D: qemuDomainDefFormatXMLInternal (qemu_domain.c:6229)
==2204780== by 0x15F8270D: qemuDomainDefFormatLive (qemu_domain.c:6279)
==2204780== by 0x15FD8100: qemuMigrationSrcBeginPhase (qemu_migration.c:2395)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer3 (qemu_migration.c:4640)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer (qemu_migration.c:5093)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformJob (qemu_migration.c:5168)
==2204780== by 0x15FE280E: qemuMigrationSrcPerform (qemu_migration.c:5372)
==2204780== by 0x15F9BA3D: qemuDomainMigratePerform3Params (qemu_driver.c:11841)
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Mon, 15 Feb 2021 14:13:39 +0000 (15:13 +0100)]
ci: Build on macOS 11 instead of macOS 10.15
macOS builder capacity on Cirrus CI is quite limited, and so we
can't afford to keep the old build job around after adding the
new one like we do for FreeBSD.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Mon, 15 Feb 2021 14:29:52 +0000 (15:29 +0100)]
ci: Update package list on Cirrus CI
While pkgng on FreeBSD updates the package list automatically
when it's run, homebrew on macOS doesn't do the same thing, which
can result in stale packages being installed. Explicitly call
'brew update' before 'brew install' to avoid that scenario.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 15 Feb 2021 17:30:41 +0000 (18:30 +0100)]
qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()
In one of my previous commits I've made an attempt to restore the
noqueue qdisc on a TAP corresponding to domain's <interface/> if
QoS is cleared out. The commit consisted of two almost identical
hunks. In both the pointer is dereferenced. But in one of them,
the pointer to new bandwidth can't be NULL while in the other it
can leading to a crash.
Fixes: d53b09235398c1320ed2f1b45b640823171467ed
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Andrea Bolognani [Thu, 11 Feb 2021 11:15:45 +0000 (12:15 +0100)]
ci: Build on FreeBSD 12.2
The FreeBSD 12.1 image on Cirrus CI is currently broken, but
that's okay because a FreeBSD 12.2 image is also available and
we'd rather build on the more up-to-date target anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Laine Stump [Tue, 9 Feb 2021 17:20:57 +0000 (12:20 -0500)]
conf: convert VIR_FREE to g_free in other functions that free their arg
Previous patches have converted VIR_FREE to g_free in functions with
names ending in Free() and Dispose(), but there are a few similar
functions with names that don't fit that pattern, but server the same
purpose (and thus can survive the same conversion). in particular
*Free*(), and *Unref().
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 5 Feb 2021 05:46:24 +0000 (00:46 -0500)]
esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE
Invocations of the macro ESX_VI__TEMPLATE__FREE() will free the main
object (referenced as "item") that's pointing to all the things being
VIR_FREEd in the body, so it is safe for all the pointers in item to
just be g_freed rather that VIR_FREEd.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 12 Feb 2021 16:27:26 +0000 (17:27 +0100)]
qemu_shim: URI escape root directory
The root directory can be provided by user (or a temporary one is
generated) and is always formatted into connection URI for both
secret driver and QEMU driver, like this:
qemu:///embed?root=$root
But if it so happens that there is an URI unfriendly character in
root directory or path to it (say a space) then invalid URI is
formatted which results in unexpected results. We can trust
g_dir_make_tmp() to generate valid URI but we can't trust user.
Escape user provided root directory. Always.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Erik Skultety [Wed, 10 Feb 2021 07:27:29 +0000 (08:27 +0100)]
ci: Makefile: Expose the CI_USER_LOGIN variable for users to use
More often than not I find myself debugging in the containers which
means that I need to have root inside, but without manually tweaking
the Makefile each time the execution would simply fail thanks to the
uid/gid mapping we do. What if we expose the CI_USER_LOGIN variable, so
that when needed, the root can be simply passed with this variable and
voila - you have a root shell inside the container with CWD=~root.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Erik Skultety [Wed, 10 Feb 2021 06:06:19 +0000 (07:06 +0100)]
ci: Drop the prepare.sh script
The purpose of this script was to prepare a customized environment in
the container, but was actually never used and it required the usage of
sudo to switch the environment from root's context to a regular user's
one.
The thing is that once someone needs a custom script they would very
likely to debug something and would also benefit from root privileges
in general, so the usage of 'sudo' in such case was a bit cumbersome.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Erik Skultety [Wed, 10 Feb 2021 06:22:18 +0000 (07:22 +0100)]
ci: Run podman command directly without wrapping it with prepare.sh
The prepare.sh script isn't currently used and forces us to make use
of sudo to switch the user inside the container from root to $USER
which created a problem on our Debian Slim-based containers which don't
have the 'sudo' package installed.
This patch removes the sudo invocation and instead runs the CMD
directly with podman.
Summary of the changes:
- move the corresponding env variables which we need to be set in the
environment from the sudo invocation to the podman invocation
- pass --workdir to podman to retain the original behaviour we had with
sudo spawning a login shell.
- MESON_OPTS env variable doesn't need to propagated to the execution
environment anymore (like we had to do with sudo), because it's
defined in the Dockerfile
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Thu, 11 Feb 2021 11:45:53 +0000 (12:45 +0100)]
ci: Move ppc64le build from Debian sid to Debian 10
Debian sid is currently broken on ppc64le, so move the build to
Debian 10; do the opposite for the aarch64 and mips64el builds to
try and restore the 10/sid balance.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Thu, 11 Feb 2021 11:55:37 +0000 (12:55 +0100)]
ci: Mark container build jobs as required/optional correctly
Whether a container build job is considered required depends on
whether the corresponding cross-build job exists, and in a few
cases the two got out of sync over time.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Thu, 11 Feb 2021 11:57:51 +0000 (12:57 +0100)]
ci: Shuffle cross-building jobs around
Keep them ordered by architecture, the same way the corresponding
container jobs are, to make it easier to jump between the two
sections and compare them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This API allows fetching a list of informational messages recorded
against the domain. This provides a way to give information about
tainting of the guest due to undesirable actions/configs, as well
as provide details of deprecated features.
The output of this API is explicitly targetted at humans, not
machines, so it is inappropriate to attempt to pattern match on
the strings and take action off them, not least because the messages
are marked for translation.
Should there be a demand for machine targetted information, this
would have to be addressed via a new API, and is not planned at
this point in time.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Thu, 11 Feb 2021 05:58:29 +0000 (00:58 -0500)]
conf: parse/format <teaming> element in plain <hostdev>
The <teaming> element in <interface> allows pairing two interfaces
together as a simple "failover bond" network device in a guest. One of
the devices is the "transient" interface - it will be preferred for
all network traffic when it is present, but may be removed when
necessary, in particular during migration, when traffic will instead
go through the other interface of the pair - the "persistent"
interface. As it happens, in the QEMU implementation of this teaming
pair (called "virtio failover" in QEMU) the transient interface is
always a host network device assigned to the guest using VFIO (aka
"hostdev"); the persistent interface is always an emulated virtio NIC.
When support was initially added for <teaming>, it was written to
require that the transient/hostdev device be defined using <interface
type='hostdev'>; this was done because the virtio failover
implementation in QEMU and the virtio guest driver demands that the
two interfaces in the pair have matching MAC addresses, and the only
way libvirt can guarantee the MAC address of a hostdev network device
is to use <interface type='hostdev'>, whose main purpose is to
configure the device's MAC address before handing the device to
QEMU. (note that <interface type='hostdev'> in turn requires that the
network device be an SRIOV VF (Virtual Function), as that is the only
type of network device whose MAC address we can set in a way that will
survive the device's driver init in the guest).
It has recently come up that some users are unable to use <teaming>
because they are running in a container environment where libvirt
doesn't have the necessary privileges or resources to set the VF's MAC
address (because setting the VF MAC is done via the same device's PF
(Physical Function), and the PF is not exposed to libvirt's container).
At the same time, these users *are* able to set the VF's MAC address
themselves in advance of staring up libvirt in the container. So they
could theoretically use the <teaming> feature if libvirt just skipped
the "setting the MAC address" part.
Fortunately, that is *exactly* the difference between <interface
type='hostdev'> (which must be a "hostdev VF") and <hostdev> (a "plain
hostdev" - it could be *any* PCI device; libvirt doesn't know what type
of PCI device it is, and doesn't care).
But what is still needed is for libvirt to provide a small bit of
information on the QEMU commandline argument for the hostdev, telling
QEMU that this device will be part of a team ("failover pair"), and
the id of the other device in the pair.
To make both of those goals simultaneously possible, this patch adds
support for the <teaming> element to plain <hostdev> - libvirt doesn't
try to set any MAC addresses, and QEMU gets the extra commandline
argument it needs)
(actually, this patch adds only the parsing/formatting of the
<teaming> element in <hostdev>. The next patch will actually wire that
into the qemu driver.)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Thu, 11 Feb 2021 03:44:08 +0000 (22:44 -0500)]
conf: separate Parse/Format functions for virDomainNetTeamingInfo
In preparation for using the same element in two places, split the
parsing/formating for that subelement out of the virDomainNetDef
functions into their own functions.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Thu, 11 Feb 2021 02:09:58 +0000 (21:09 -0500)]
conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo
To make it easier to split out the parsing/formatting of the <teaming>
element into separate functions (so we can more easily add the
<teaming> element to <hostdev>, change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Thu, 11 Feb 2021 01:08:29 +0000 (20:08 -0500)]
conf: make teaming info an official type
This struct was previously defined only within virDomainNetDef where
it was used, but I need to also use it in virDomainHostdevDef, so move
the internal struct out to its own "official" struct and give it the
standard typedef duo and *Free() function.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Wed, 10 Feb 2021 19:52:25 +0000 (14:52 -0500)]
qemu: match alias when looking for proper <interface> to detach.
Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).
Resolves: https://bugzilla.redhat.com/1926190 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jim Fehlig [Tue, 9 Feb 2021 22:47:30 +0000 (15:47 -0700)]
qemu: Validate TPM TIS device
TPM devices with model='tpm-tis' are only valid with x86 and aarch64
virt machines. Add a check to qemuValidateDomainDeviceDefTPM() to
ensure VIR_DOMAIN_TPM_MODEL_TIS is only used with these architectures.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
error: internal error: process exited while connecting to monitor:
2021-02-07T05:15:35.378927Z qemu-system-aarch64: -device
tpm-tis,tpmdev=tpm-tpm0,id=tpm0: 'tpm-tis' is not a valid device model name
Indeed the TPM device name is 'tpm-tis-device' [1][2] for aarch64,
versus the shorter 'tpm-tis' for x86. The devices are the same from
a functional POV, i.e. they both emulate a TPM device conforming to
the TIS specification. Account for the unfortunate name difference
when building the TPM device option in qemuBuildTPMDevStr(). Also
include a test case for 'tpm-tis-device'.
projects / libvirt.git / log