While glibc provides qsort(), which usually is just a mergesort,
until sorting arrays so huge that temporary array used by
mergesort would not fit into physical memory (which in our case
is never), we are not guaranteed it'll use mergesort. The
advantage of mergesort is clear - it's stable. IOW, if we have an
array of values parsed from XML, qsort() it and produce some
output based on those values, we can then compare the output with
some expected output, line by line.
But with newer glibc this is all history. After [1], qsort() is
no longer mergesort but introsort instead, which is not stable.
This is suboptimal, because in some cases we want to preserve
order of equal items. For instance, in ebiptablesApplyNewRules(),
nwfilter rules are sorted by their priority. But if two rules
have the same priority, we want to keep them in the order they
appear in the XML. Since it's hard/needless work to identify
places where stable or unstable sorting is needed, let's just
play it safe and use stable sorting everywhere.
Fortunately, glib provides g_qsort_with_data() which indeed
implement mergesort and it's a drop in replacement for qsort(),
almost. It accepts fifth argument (pointer to opaque data), that
is passed to comparator function, which then accepts three
arguments.
We have to keep one occurance of qsort() though - in NSS module
which deliberately does not link with glib.
1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a435afcc3048e0b697b6cc04
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Error messages are exempt from the 80 columns rule. Move them
onto one line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
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>
Commit be1b7d5b18 introduced parsing /proc/cpuinfo for "address size"
which is not including on S390 and therefore reports an internal error.
Lets remove the parsing on S390.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
At this moment it is not possible to launch a 'riscv64' domain if a CPU
definition is presented in the domain. For example, adding this CPU
definition:
<cpu mode='custom' match='exact' check='none'>
<model fallback='forbid'>rv64</model>
</cpu>
Will trigger the following error:
$ sudo ./run tools/virsh start riscv-virt1
error: Failed to start domain 'riscv-virt1'
error: this function is not supported by the connection driver:
cannot update guest CPU for riscv64 architecture
The error comes from virCPUUpdate(), via qemuProcessUpdateGuestCPU(),
and it's caused by the absence of the 'update' API in the existing
RISC-V driver.
Add an 'update' API impl to the RISC-V driver to allow for CPU
definitions to be declared in RISC-V domains. This API was copied from
the ARM driver (virCPUarmUpdate()) since it's a good enough
implementation to get us going.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Make all callers always pass a valid pointer which in turn allows us to
remove return value check from the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make all callers always pass a valid pointer which in turn allows us to
remove return value check from the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When running "virsh domcapabilities" on a s390x host, all the CPU
models show up with vendor='unknown' - which sounds kind of weird
since the vendor of these mainframe CPUs is well known: IBM.
All CPUs starting with either "z" or "gen" match a real mainframe
CPU by IBM, so let's return the string "IBM" for those now.
The only remaining ones are now the artifical "qemu" and "max"
models from QEMU itself, so it should be OK to get an "unknown"
vendor for those two.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Boris Fiuczynski<fiuczy@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are tests in qemuxml2argvtest that will fail if we enable RISC-V
testing, with an error like the following:
"cpuGetSubDriver:64 : this function is not supported by the connection
driver: 'riscv64' architecture is not supp orted by CPU driver"
This happens because we don't have a RISC-V driver yet.
Add a barebone RISC-V driver to allow tests to be executed. The only 2
callbacks implemented here are 'compare' and 'validateFeatures', both
acting as a no-op. More callbacks and features will be added in the
future.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
These format are left unchanged when convert 'unsigned long' to
'unsigned long long', which caused compile warning.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Based on qemu commit e1f9a8e8c90ae54387922e33e5ac4fd759747d01 introduce
the hv-avic feature in leaf 0x40000004, EAX 0x00000200 (1 << 9).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'VIR_CPU_x86_HV_STIMER_DIRECT' is reported under leaf 0x40000003,
but the data is in the EDX register. Create a new group for such
features and move them after the 0x40000003 EAX group.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A later patch will add alias names to the feature map. They will be used
in virQEMUCapsCPUFeatureTranslate and for synchronizing the list with QEMU.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
We don't need to do the extra XPath lookups and we can use the proper
type right away.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch is effectively a no-op, but I wanted to initialize
.getVendorForModel explicitly as implementing this function does not
even make sense on ARM. The CPU models in our CPU map are only used for
describing host CPU in capabilities XML and cannot be used for guest CPU
definition in domain XML anyway. The CPU models listed as supported in
domain capabilities XML are just passed through from QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far QEMU driver does not get CPU model vendor from QEMU directly and
it has to ask the CPU driver for the info stored in CPU map.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The ppc64 CPU code still has to load and parse the CPU map everytime it
needs to look at it, which can make some operations pretty slow. Other
archs already switched to loading the CPU map once and keeping the
parsed structure in memory. Let's switch ppc64 as well.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The G_GNUC_NO_INLINE macro will eventually be marked as
deprecated [1] and we are recommended to use G_NO_INLINE instead.
Do the switch now, rather than waiting for compile time warning
to occur.
1: 15cd0f0461
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Internally we already collect x86 host family + model + stepping
numeric values. This exposed them in capabilities CPU output.
Example:
$ sudo virsh capabilities | grep -A1 -B1 signature
<microcode version='240'/>
<signature family='6' model='94' stepping='3'/>
<counter name='tsc' frequency='3408010000' scaling='no'/>
Users need to know these values to calculate an expected.
SEV-ES/SEV-SNP launch measurement.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
While we don't want to aim for the shortest list of disabled features in
the baseline result (it would select a very old model), we want to do so
while looking at any of the input models for which we're trying to
compute a baseline CPU model. Given a set of input models, we always
want to take the least capable one of them (i.e., the one with shortest
list of disabled features) or a better model which is not one of the
input models.
So when considering an input model, we just check whether its list of
disabled features is shorter than the currently best one. When looking
at other models we check both enabled and disabled features while
penalizing disabled features as implemented by the previous patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For finding the best matching CPU model for a given set of features
while we don't know the CPU signature (i.e., when computing a baseline
CPU model) we've been using a "shortest list of features" heuristics.
This works well if new CPU models are supersets of older models, but
that's not always the case. As a result it may actually select a new CPU
model as a baseline while removing some features from it to make it
compatible with older models. This is in general worse than using an old
CPU model with a bunch of added features as a guest OS or apps may crash
when using features that were disabled.
On the other hand we don't want to end up with a very old model which
would guarantee no disabled features as it could stop a guest OS or apps
from using some features provided by the CPU because they would not
expect them on such an old CPU.
This patch changes the heuristics to something in between. Enabled and
disabled features are counted separately so that a CPU model requiring
some features to be disabled looks worse than a model with fewer
disabled features even if its complete list of features is longer. The
penalty given for each additional disabled feature gets bigger to make
longer list of disabled features look even worse.
https://bugzilla.redhat.com/show_bug.cgi?id=1851227
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It will become more complicated and so it deserves to be separated into
a new function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Checking the signature in two different places makes no sense since the
code in between can only mark the candidate as the best option so far,
which is what the second signature match does as well.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These two pointers can never be NULL since they are initialised to a reference
of a struct. This became apparent when commit 210a195394 added a VIR_DEBUG
which used both pointers because due to the concise condition the compiler saw
that if the "and" part of the condition did short-circuit (and it assumed that
can happen) the second variable would not be initialised, but it is used in the
debugging message, so the build failed with:
In file included from ../src/cpu/cpu_x86.c:27:
../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’:
../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Fix this by just assigning the helper pointers and remove the condition
altogether.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The function returns 'virCPUData' but doesn't do two important steps
which other code takes:
1) leaves with all-zero data is stripped from the XML output
2) the data is expected to be sorted in the array
Now the 'virHostCPUGetCPUID' helper returns both all 0 leaves and
doesn't order them as we expect.
If this is then used in conjunction with 'virCPUx86DataIsIdentical'
together with data which made a roundtrip to XML and back the result
will be always false even if the data itself is identical.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Without this it's impossible to debug scenarios when this function
returns a mismatch but the formatted data looks identical.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are couple of places where virTristateBoolTypeFromString()
is called. Well, the same result can be achieved by
virXMLPropTristateBool() and on fewer lines.
Note there are couple of places left untouched because those
don't care about error reporting and thus are shorter they way
they are now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are couple of places (all of them in XML parsing) where
virTristateSwitchTypeFromString() is called. Well, the same
result can be achieved by virXMLPropTristateSwitch() and on fewer
lines.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This will be needed directly in the QEMU driver in a later patch.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
ppc64Compute() is used only once, by virCPUppc64Compare(), which
doesn't use the 'guest' parameter. It was last used by an API
called 'cpuGuestData' that was dropped by commit 03fa904c0c.
Removing the 'guest' parameter will not only remove unused code from
ppc64Compute() but also remove the ppc64MakeCPUData() entirely.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
x86Compute() is a static function called only by virCPUx86Compare()
which passes NULL to the 'guest' parameter of x86Compute().
The last caller of x86Compute() that used it with 'guest' != NULL
was an API called 'cpuGuestData'. This API was dropped by commit
03fa904c0c a few years ago. Since then all callers of x86Compute()
uses it with 'guest' = NULL.
Removing the 'guest' parameter allow us to remove a good chunk of
logic that isn't being used for awhile.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
