Since the error message originates from a log file it contains a
trailing newline. Strip it as all error handling adds it's own newline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The caller passes in a 1k buffer, which when debug logging is in use is
easily filled with debug messages only. Thus after the first pass which
is common if the controller process already terminated the buffer will
not contain the real error, but rather a truncated debug message,
which will result in an error such as:
error: internal error: guest failed to start: 2023-08-01 12:58:31.948+0000: 798195: i
instead of the proper error:
error: internal error: guest failed to start: Failure in libvirt_lxc startup: Failed to create /home/rootfs/.oldroot: Permission denied
To fix the above retry the reading loop if the filtering function made
space in the buffer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Avoid logging multiline debug logs so that the function which attempts
to extract a non-debug log error message can work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
If one of previous commits taught us something, it's that:
sizeof(variable) and sizeof(type) are not the same. Especially
because for live enough code the type might change (e.g. as we
use autoptr more). And since we don't get any warnings when an
incorrect length is passed to memset() it is easy to mess up. But
with sizeof(variable) instead, it's not as easy. Therefore,
switch to using memset(variable, 0, sizeof(*variable)), or its
alternatives, depending on level of pointers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
There are some cases left after previous commit which were not
picked up by coccinelle. Mostly, becuase the spatch was not
generic enough. We are left with cases like: two variables
declared on one line, a variable declared in #ifdef-s (there are
notoriously difficult for coccinelle), arrays, macro definitions,
etc.
Finish what coccinelle started, by hand.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Generated using the following semantic patch:
@@
type T;
identifier X;
@@
- T X;
+ T X = { 0 };
... when exists
(
- memset(&X, 0, sizeof(X));
|
- memset(&X, 0, sizeof(T));
)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
Ideally, these would be fixed by coccinelle (see next commit),
but because of various reasons they aren't. Fix them manually.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
Instead of suggesting to zero structs out using memset() we
should suggest initializing structs with zero initializer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
The fds variable inside of virNetlinkCommand() is not used
really. It's passed to memset() (hence compilers do not
complain), but that's about it. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
This is a residue of v6.8.0-rc1~100. The error variable inside of
virFirewallDApplyRule() is already initialized to NULL. There's
no need to memset() it to zero again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
Inside of remoteAuthSASL() the sargs variable is already
initialized to zero during declaration. There's no need to
memset() it again as it's unused in between it's declaration and
said memset().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
When a VSERPORT_CHANGE event is processed, we firstly do a little
detour and try to detect whether the event is coming from guest
agent. If so, we notify threads that are currently talking to the
agent about this fact. Then we proceed with usual event
processing (BeginJob(), update domain def, emit event, and so
on).
In both cases we use the same @dev variable to refer to domain
device. While this works, it will make writing semantic patch
unnecessary harder (see next commit(s)). Therefore, introduce a
separate variable for the detour code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
There are couple of variables that are declared at function
beginning but then used solely within a block (either for() loop
or if() statement). And just before their use they are zeroed
explicitly using memset(). Decrease their scope, use struct zero
initializer and drop explicit memset().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
This is similar to the previous commit, except this is for a
different type (vahControl) and also fixes the case where _ctl is
passed not initialized to vah_error() (via ctl pointer so that's
probably why compilers don't complain).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
Both virsh and virt-admin have vshControl typed variables and
also pointers to these variables. In both cases these are
declared on a single line. Do the following:
1) break declaration into two lines,
2) use struct zero initializer for vshControl and
virshControl/vshAdmControl structs,
3) drop explicit memset(.., 0, ...) ;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
When I implemented passt support in libvirt, I saw the --mac-addr
option on the passt commandline, immediately assumed that this was
used for setting the guest interface's mac address somewhere within
passt, and read no further. As a result, "--mac-addr" is always added
to the passt commandline, specifying the setting from <mac
addr='blah'/> in the guest's interface config.
But as pointed out in this bugzilla comment:
https://bugzilla.redhat.com/2184967#c8
That is *not at all* what passt's --mac-addr option does. Instead, it
is used to force the *remote* mac address for incoming traffic to a
specific value. So setting --mac-addr results in all traffic on the
interface having the same (the guest's) mac address for both source
and destination in all traffic. Surprisingly, this still works, so
nobody noticed it during testing.
The proper thing is to not specify any mac address to passt - the
remote MAC addresses can and should remain untouched, and the local
MAC address will end up being known to passt just by the guest sending
out packets with that MAC address.
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
This reverts commit 8511b96a31.
Turns out, we need to do a bit more than just plain
qemuSecurityDomainSetPathLabel() which sets svirt_image_t. Passt
has its own SELinux policy and as a part of that they invent
passt_log_t for log files. Right now, I don't know how libvirt
could query that and even if I did, passt SELinux policy would
need to permit relabelling from svirt_t to passt_log_t, which it
doesn't [1].
Until these problems are addressed we shouldn't be pre-creating
the file as it puts users into way worse position - even
scenarios that used to work don't work. But then again - using
log file for passt is usually valuable for developers only and
not regular users.
1: https://bugzilla.redhat.com/show_bug.cgi?id=2209191#c10
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This reverts commit 83686f1eea.
This is needed only so that the next revert is clean.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
We dropped our private virXXXPtr typedefs in v7.3.0-rc1~229 but
somehow v7.9.0-rc1~292 introduced one back:
virDomainEventMemoryDeviceSizeChangePtr. There's no need for it
and it's internal only. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently translated at 100.0% (10393 of 10393 strings)
Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/cs/
Co-authored-by: Pavel Borecki <pavel.borecki@gmail.com>
Signed-off-by: Pavel Borecki <pavel.borecki@gmail.com>
In most cases the replacement is straightforward, with the
biggest difference being that we now schedule restarts during
%pre instead of %post. This also means that we can get rid of
%post for most packages, reducing the number of scriptlets that
need to run during install/upgrade.
Notable exceptions are libvirt-guests.service, where we stop
using the standard systemd macros to adopt our custom ones, as
well as the virtlogd and virtlockd services, where the reload
operation is moved from %postun to %posttrans.
https://bugzilla.redhat.com/show_bug.cgi?id=2210058
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
systemd provides a number of standard RPM macros but they don't
quite satisfy our requirements, as evidenced by the fact that we
have already built some custom tooling around them.
Scenarios that the standard macros don't cover and that we're
already addressing with our custom ones:
* for some services (libvirtd, virtnetworkd, virtnwfilterd)
there are multiple conditions that might lead to a restart,
and we want to make sure that they're not needlessly
restarted several times per transaction;
* some services (virtlogd, virtlockd) must not be restarted
during upgrade, so we have to reload them instead.
Issues that neither the standard macros nor our custom ones
address:
* presets for units should be applied when the unit is first
installed, not when the package that contains it is.
The package split that happened in 9.1.0 highlighted why this
last point is so important: when virtproxyd and its sockets
were moved from libvirt-daemon to the new libvirt-daemon-proxy
package, upgrades from 9.0.0 caused presets for them to be
applied.
On a platform such as Fedora, where modular daemons are the
default, this has resulted in breaking existing deployments in
at least two scenarios.
The first one is machines that were configured to use the
monolithic daemon, either because the local admin had manually
changed the configuration or because the installation dated
back to before modular daemons had become the default. In this
case, virtproxyd.socket being enabled resulted in a silent
conflict with libvirtd.socket, which by design shares the same
path, and thus a completely broken setup.
The second one is machines where virtproxy-tls.socket, which is
disabled by default, had manually been enabled: in this case,
applying the presets resulted in it being disabled and thus a
loss of remote availability.
Note that these are just two concrete scenarios, but the problem
is more generic. For example, if we were to add more units to an
existing package, per the current approach they wouldn't have
their presets applied.
The new macros are designed to avoid all of the pitfalls
mentioned above. As a bonus, they're also simpler to use: where
the current approach requires restarts and other operations to
be handled separately, the new one integrates the two so that,
for each scriptlet, a single macro call is needed.
https://bugzilla.redhat.com/show_bug.cgi?id=2210058
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This logic was necessary when socket activation was introduced
in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades.
These days, even the oldest platform that we target ships a
version of libvirtd that implements socket activation, so the
additional code is no longer useful and we can treat libvirtd
the same as all other services.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The current implementation pretty much assumes that targets
where modular daemons are the default will stick with that
configuration, as will targets where they're not, or that
changes to these defaults will be performed by the admin after
the packages have been installed.
This is unnecessarily limiting: for example, on a target that
defaults to using the monolithic daemon, it's entirely possible
to create a local preset such as
# /etc/systemd/system-preset/00-virt.preset
disable libvirtd.service
disable libvirtd*.socket
enable virtqemud.service
to opt into a modular daemon deployment. The opposite is of
course also true. We shouldn't get in the way of these
reasonable use cases.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
To make things more readable, use the same order (%pre, %post,
%posttrans, %preun, %postun) everywhere.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
According to our platform support policy, now that Fedora 38 is
out we no longer target Fedora 36 and older. This allows us to
simplify a few conditionals.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.
Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/
Co-authored-by: Weblate <noreply@weblate.org>
Signed-off-by: Fedora Weblate Translation <i18n@lists.fedoraproject.org>
A new bug was introduced as a part of use-after-free fix below:
commit 411cbe7199
Author: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Date: Tue Jul 4 13:10:22 2023 +0600
remote: fix stream use-after-free
When the message was processed partially, it is actually supposed to
stay in the queue to be processed again. In such case, reinsert it back.
Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The virRandomGenerateWWN() is used solely by nodedev driver to
autogenerate WWNN and WWNP when parsing a nodedev XML. Now, the
idea was (at least during monolithic daemon) that depending on
which hypervisor driver called the nodedev XML parsing (and
virRandomGenerateWWN() under the hood) the corresponding OUI is
used (e.g. "001a4a" for the QEMU driver).
But in era of split daemons things are not that easy. We do not
know which hypervisor driver called us. And there might be no
hypervisor driver at all - users are allowed to connect to
individual drivers directly (e.g. "nodedev:///system").
In this case, we can't use proper OUI. Well, do the next best
thing: pick one (QUMRANET_OUI).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When automatically adding a NUMA node (qemuDomainDefNumaAutoAdd()) the
memory size of the node is computed as:
total_memory - sum(memory devices)
And we have a nice helper for that: virDomainDefGetMemoryInitial() so
it looks logical to just call it. Except, this code runs in post parse
callback, i.e. memory sizes were not validated and it may happen that
the sum is greater than the total memory. This would be caught by
virDomainDefPostParseMemory() but that runs only after driver specific
callbacks (i.e. after qemuDomainDefNumaAutoAdd()) and because the
domain config was changed and memory was increased to this huge
number no error is caught.
So let's do what virDomainDefGetMemoryInitial() would do, but
with error checking.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2216236
Fixes: f5d4f5c8ee
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Currently translated at 100.0% (10395 of 10395 strings)
Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/cs/
Co-authored-by: Pavel Borecki <pavel.borecki@gmail.com>
Signed-off-by: Pavel Borecki <pavel.borecki@gmail.com>
OpenSUSE Leap was released recently (2023-06-07). Refresh our CI
with latest lcitool which brings this minor update.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This brings the tool's list of features in sync with qemu
commit 6f05a92ddc73ac8aa16cfd6188f907b30b0501e3.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
