Michal Privoznik [Mon, 11 Nov 2019 12:41:34 +0000 (13:41 +0100)]
make check-driverimpls work again
Previously we generated all source files into $srcdir which is no
longer true. This means that we can't just blindly prepend each
source file with $srcdir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 11 Nov 2019 05:37:33 +0000 (06:37 +0100)]
src: Make check-aclrules work again
Previously we generated all source files into $srcdir which is no
longer true. This means that we can't just blindly prepend each
source file with $srcdir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Fri, 25 Oct 2019 12:21:08 +0000 (14:21 +0200)]
conf: domaincaps: Fix broken attempt at being const-correct
'virBlahPtr const blah' results into modification to the value of 'blah'
triggering compilation error rather than the modification of the virBlah
struct the pointer points to.
All of the domain capability formatting code was broken in this regard.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Mao Zhongyi [Wed, 30 Oct 2019 03:42:11 +0000 (11:42 +0800)]
qemu: remove duplicate header files
"#include vircgroup.h" appears in both qemu_cgroup.h and
qemu_cgroup.c, and qemu_cgroup.c contains qemu_cgroup.h,
so remove the duplicate declarations.
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 8 Nov 2019 14:37:26 +0000 (15:37 +0100)]
qemu: command: Use XML based disk bus convertor in error message
The qemu driver has an internal implementation for converting disk bus
to string for use with qemu. This should not be used in error messages
though as we want to report the string based on the XML value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ACKed-by: Eric Blake <eblake@redhat.com>
Michal Privoznik [Mon, 11 Nov 2019 16:22:50 +0000 (17:22 +0100)]
qemu: Warn verbosely if using old loader:nvram pairs
There are two ways for specifying loader:nvram pairs:
1) --with-loader-nvram configure option
2) nvram variable in qemu.conf
Since we have FW descriptors, using this old style is
discouraged, but not as strong as one would expect. Produce more
warnings:
1) produce a warning if somebody tries the configure option
2) produce a warning if somebody sets nvram variable and at
least on FW descriptor was found
The reason for producing warning in case 1) is that package
maintainers, who set the configure option in the first place
should start moving towards FW descriptors and abandon the
configure option. After all, the warning is printed into config
output only in this case.
Peter Krempa [Mon, 11 Nov 2019 12:03:14 +0000 (13:03 +0100)]
util: pci: Remove always-false condition
Commit d19c21429fd modified the condition so that it checks whether the
value is more than 0xFFFFFFFF. Since addr->domain is an unsigned int, it
will never be more than that.
Remove the whole check
src/util/virpci.c:1291:22: error: result of comparison 'unsigned int' > 4294967295 is always false [-Werror,-Wtautological-type-limit-compare]
if (addr->domain > 0xFFFFFFFF) {
~~~~~~~~~~~~ ^ ~~~~~~~~~~
Peter Krempa [Fri, 25 Oct 2019 11:59:46 +0000 (13:59 +0200)]
tests: make domaincapstest less anoying to debug
Since 6a077cf2b3 domaincapstest does not run through all cases on
failure but terminates right away. This makes it super annoying to debug
or use in combination with VIR_TEST_REGENERATE_OUTPUT.
Fix it by remembering failure and still running through all cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The makefile rules for man pages were accidentally not updated for the
new variablle name.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Exception catching statements should always match on a class name, the
most specific one possible. Rather than analyse the code to look at what
the most specific one is, this just uses the base Exception class.
docs/apibuild.py:255:9: E722 do not use bare 'except'
except:
^
docs/apibuild.py:279:9: E722 do not use bare 'except'
except:
^
...more...
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
build: change flake8 to use blacklist instead of whitelist
The current flake8 check only looks at one item (semicolons at end of
line). This means that our code quality will continue to get worse,
violating an increasing number of checks.
Switching to a whitelist means that we freeze the badness at its
current level & can incrementally fix things up.
We are excluding the following...
Indentation:
E114 indentation is not a multiple of four (comment)
E115 expected an indented block (comment)
E116 unexpected indentation (comment)
E121 continuation line under-indented for hanging indent
E125 continuation line with same indent as next logical line
E126 continuation line over-indented for hanging indent
E127 continuation line over-indented for visual indent
E128 continuation line under-indented for visual indent
E129 visually indented line with same indent as next logical line
E131 continuation line unaligned for hanging indent
Whitespace:
E211 whitespace before ‘(‘
E221 multiple spaces before operator
E222 multiple spaces after operator
E225 missing whitespace around operator
E226 missing whitespace around arithmetic operator
E231 missing whitespace after ‘,’, ‘;’, or ‘:’
E261 at least two spaces before inline comment
Blank lines
E301 expected 1 blank line, found 0
E302 expected 2 blank lines, found 0
E303 too many blank lines (3)
E305 expected 2 blank lines after end of function or class
Line length
E501 line too long (82 > 79 characters)
Statements
E722 do not use bare except, specify exception instead
E741 do not use variables named ‘l’, ‘O’, or ‘I’
Errors:
F821 undefined name 'name'
Warnings:
W504 line break after binary operator
W605 invalid escape sequence ‘x’
Later commits will enable most of these exclusions.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Sun, 10 Nov 2019 10:06:19 +0000 (11:06 +0100)]
src: lxc: Fix typo in a Makefile variable
In commit 0985a9597bb0348d46c0d18dc548a676bf0ad8e2 we stopped
distributing generated source file. This is done by prepending
binary_SOURCES variable with "nodist_". However, there is a typo
- the prefix is "nodst_" instead of "nodist_".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Fri, 8 Nov 2019 14:25:33 +0000 (15:25 +0100)]
po: rewrite the way how we generate files
There was no need to handle files for translation from build directory
but that will change with following patches where we will stop
generating source files into source directory.
In order to have them included for translation we have to prefix each
file with SRCDIR or BUILDDIR.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Wed, 23 Oct 2019 19:56:50 +0000 (21:56 +0200)]
po: generate files into build directory
Historically we did not support VPATH builds and everything was
generated into source directory. The introduction of VPATH builds
did not changed the way how our translation files are handled.
This patch changes the rules to generate everything into build
directory and stops distributing generated files in order to have
properly separated VPATH builds.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Wed, 23 Oct 2019 13:35:05 +0000 (15:35 +0200)]
syntax-check.mk: cleanup sc_po_check dependencies
Introduce new rule 'generated-sources' as a helper for PO files check
to make sure that all generated files are prepared and to not duplicate
the list on different places. This will be used as a dependency for
sc_po_check rule instead of duplicated list of generated files.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Wed, 23 Oct 2019 19:26:31 +0000 (21:26 +0200)]
syntax-check.mk: fix sc_po_check rule
Commit <22d8e27ccd5faf48ee2bf288a1b9059aa7ffd28b> introduced our
syntax-check.mk file based on gnulib rules. However, the rule was
completely ignored as we don't have POTFILES.in file.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Thu, 24 Oct 2019 08:26:12 +0000 (10:26 +0200)]
.gitignore: cleanup old and obsolete ignores
Now that we forbid builds in source directory we can remove a lot of
ignores that are created during build time. To make the cleanup easier
in the future create a sections in our .gitignore file.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pavel Hrdina [Wed, 6 Nov 2019 14:55:12 +0000 (15:55 +0100)]
build: mandate use of a build dir != src dir
Historically we've allowed builds in the main src dir, but meson does
not support this. Explicitly force separate build dir in autotools to
align with meson. We must re-enable dependency tracking which the RPM
%configure macro turns off. Without this, the build dir doesn't get
the source directory tree mirrored.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: Check for job being set when getting iothread stats
The qemuDomainGetStatsIOThread() accesses the monitor by calling
qemuDomainGetIOThreadsMon(). And it's also marked as "need
monitor" in qemuDomainGetStatsWorkers[]. However, it's not
checking if acquiring job was successful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Wang Yechao [Fri, 8 Nov 2019 00:25:15 +0000 (08:25 +0800)]
util: Set SIGPIPE to a no-op handler in virFork
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal
handlers to the defaults. But child process may write logs to
stderr/stdout, that may generate SIGPIPE if journald has stopped.
So set SIGPIPE to a dummy no-op handler before unmask signals in
virFork(), and the handler will get reset to SIG_DFL when execve()
runs. Now we can delete sigaction() call entirely in virExec().
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Laine Stump [Fri, 18 Oct 2019 01:12:30 +0000 (21:12 -0400)]
util: set bridge device MAC address explicitly during virNetDevBridgeCreate
When libvirt first implemented a stable and configurable MAC address
for the bridges created for libvirt virtual networks (commit 5754dbd56d, in libvirt v0.8.8) most distro stable releases didn't
support explicitly setting the MAC address of a bridge; the bridge
just always assumed the lowest numbered MAC of all attached
interfaces. Because of this, we stabilized the bridge MAC address by
creating a "dummy" tap interface with a MAC address guaranteed to be
lower than any of the guest tap devices' MACs (which all started with
0xFE, so it's not difficult to do) and attached it to the bridge -
this was the inception of the "virbr0-nic" device that has confused so
many people over the years.
Even though the linux kernel had recently gained support for
explicitly setting a bridge MAC, we deemed it unnecessary to set the
MAC that way, because the other (indirect) method worked everywhere.
But recently there have been reports that the bridge MAC address was
not following the setting in the network config, and mismatched the
MAC of the dummy tap device (which was still correct). It turns out
that this is due to a change in systemd-242 that persists whatever MAC
address is set for a bridge when it's initially started. According to
the systemd NEWS file entry for version 242
(https://github.com/systemd/systemd/blob/master/NEWS):
"if a bridge interface is created without any slaves, and gains
a slave later, then now the bridge does not inherit slave's MAC."
This change was the result of:
https://github.com/systemd/systemd/issues/3374
(apparently if there is no MAC saved for a bridge by the name of a
bridge being created, the random MAC generated during creation is
saved, and then that same MAC is used to explicitly set the MAC each
time it is created). Once a bridge has an explicitly set MAC, the "use
the lowest numbered MAC of attached devices" rule is ignored, so our
dummy tap device is like the goggles - it does nothing! (well, almost).
We could whine about changes in default behavior, etc. etc., but
because the change was in response to actual user problems, that seems
likely a fruitless task. Fortunately, time has marched on, and even
distro releases that are old enough that they are no longer supported
by upstream libvirt (e.g. RHEL6) have support for explicitly setting a
bridge device MAC address, either during creation or with a separate
ioctl after creation, so we can now do that.
To enable explicitly setting the mac during bridge creation, we add a
mac arg to virNetDevBridgeCreate(). In the case of platforms where
the bridge is created with a netlink RTM_NEWLINK message, we just add
that mac to the message. For platforms that still use an ioctl (either
SIOCBRADDBR or SIOCIFCREATE2), we make a separate call to
virNetDevSetMAC() after creating the bridge.
(NB: I was unable to test the calling of virNetDevSetMAC() from the
SIOCIFCREATE2 (BSD) version of virNetDevBridgeCreate(); even though I
managed to get a FreeBSD system setup and libvirt built there, when I
tried to start the default network the SIOCIFCREATE2 ioctl itself
failed, so it never even got to the virNetDevSetMAC(). That leaves the
FreeBSD implementation untested.)
This makes the dummy tap pointless for purposes of setting the MAC
address, but it is still useful for IPv6 DAD initialization (which
apparently requires at least one interface to be attached to the
bridge and online), as well as for setting an initial MTU for the
bridge, so it hasn't been removed.
(NB: we can safely *always* call virNetDevBridgeCreate() with
&def->mac from the network driver because, in spite of the existence
of a "mac_specified" bool in the config suggesting that it may not
always be present, in reality a mac address will always be added to
any network that doesn't have one - this is guaranteed in all cases by
commit a47ae7c004)
https://bugzilla.redhat.com/show_bug.cgi?id=1760851 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Laine Stump [Wed, 16 Oct 2019 18:06:54 +0000 (14:06 -0400)]
util: allow sending mac addr to virNetNewLink without ifindex
Although until now, any use of the extra_args argument (a pointer to a
struct containing extra attributes to add the the RTM_NEWLINK message)
would always have the ifindex and mac set, so the code could assume it
was safe to add both to the message if extra_args != NULL. There is
now a use for setting a MAC address in the RTM_NEWLINK without setting
the ifindex, so we should check each of these separately.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Laine Stump [Sat, 19 Oct 2019 05:58:07 +0000 (01:58 -0400)]
qemu: avoid double reservation of PCI address for interface type='hostdev'
Commit 01ca4010d86 (libvirt v5.1.0) moved address reservation for
hotplugged interface devices up to an earlier point in
qemuDomainAttachNetDevice(), because that function calls
qemuDomainSupportsNicdev() (in the case of
VIR_DOMAIN_NET_TYPE_VHOSTUSER), and qemuDomainSupportsNicdev() needs
to know the address type (for ARM machinetypes) and returns incorrect
results when the address type is "none".
This bugfix unfortunately caused a regression, because it also made PCI
address reservation happen before we noticed that the device was a
*hostdev* interface. Those interfaces are hotplugged by just calling
out to qemuDomainAttachHostdevDevice() - that function would then also
attempt to reserve the *same PCI address* that had just been reserved
in qemuDomainAttachNetDevice().
The solution is to move the bit of code that short-circuits out to
virDomainHostdevAttach() up *even earlier* so that no PCI address has
been allocated by the time it's called.
https://bugzilla.redhat.com/show_bug.cgi?id=1744523 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Fri, 11 Oct 2019 08:05:59 +0000 (10:05 +0200)]
qemu: Validate ARM CPU features
This introduces semantic validation for SVE-related features,
preventing the user from combining them in invalid ways; it also
automatically enables overall SVE support if any SVE vector
length has been enabled by the user to make sure QEMU behaves
correctly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Tested-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Mon, 22 Jul 2019 14:41:45 +0000 (16:41 +0200)]
cpu_map: Introduce ARM CPU features
The only feature we care about for the moment is SVE, which can
be controlled both with a coarse granularity by turning it on/off
completely and with a finer granularity by enabling/disabling
individual vector lengths.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Tested-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Mon, 24 Jun 2019 12:16:17 +0000 (14:16 +0200)]
qemu: Perform full expansion on ARM
The ARM implementation of query-cpu-model-expansion only
supports full expansion, so we have to make sure we're using
that expansion mode if we want to obtain any useful data.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Tested-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
CPU features are available on ARM only wherever the
query-cpu-model-expansion QMP command is available, same as
on s390. Update qemuBuildCpuModelArgStr() to reflect this
fact.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Tested-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 10 Oct 2019 16:28:47 +0000 (12:28 -0400)]
tests: Update capabilities for QEMU 4.2.0 on aarch64
Unfortunately this results in a lot of churn because of the eigth
hundred and change QEMU commits since the file was last touched,
but the only part we actually care about is the fact that the
query-cpu-model-expansion QMP command is now available on aarch64.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Tested-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:55:21 +0000 (07:55 -0500)]
tests: Fix logic to not have possible NULL deref
It's possible that virBitmapNewString returns NULL with an error
string (and not an allocation failure that would abort); however, if
virBitmapToString is called with a NULL @bitmap, then it will fail
in an ugly manner. So rather than have if (!map && !str) logic, split
the checks for each variable.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:39:22 +0000 (07:39 -0500)]
tests: Add return value check in checkUserInfo
Commit 1c8113f9c added the call to virTypedParamsGetString without
a return value check which caused Coverity to complain especially
since other checks for the same function are made.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:35:25 +0000 (07:35 -0500)]
qemu: Fix possible NULL deref in qemuDomainSaveImageStartVM
Commit 075523438 added a direct reference to @cookie even though
it may be NULL as shown by a comment a few lines previous - so add
the check here as well.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:34:04 +0000 (07:34 -0500)]
tests: Remove _NULLABLE in virNetDevExists mock
The @ifname is listed as an ATTRIBUTE_NONNULL(1) parameter, so
checking for _NULLABLE causes a coverity build failure - remove
that and if it's NULL for the test let's fail miserably.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:19:48 +0000 (07:19 -0500)]
tests: Fix memory leak in mymain
Commit 944a35d7f0 added @fakerootdir; however, there are multiple
paths out of mymain that didn't free the memory - so just use the
g_autofree to resolve the potential leak.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
John Ferlan [Sun, 3 Nov 2019 12:12:58 +0000 (07:12 -0500)]
conf: Remove ATTRIBUTE_NONNULL for virDomainQemuMonitorEventNew
Commit 17561eb36 modified the logic to check "if (!event)" for an
attribute that was not supposed to be passed as NULL. This causes
the static checker/Coverity build to fail. Since the check is made,
alter the header.
Also add an error message since returning -1 without some sort of
error message as previously would have happened with the failed
VIR_STRDUP so that the eventual error doesn't get the default
for some reason message.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
projects / libvirt.git / log