'qemuDomainSetBlockIoTuneFields' already populates the contents of the
VIR_DOMAIN_EVENT_ID_TUNABLE params with the group name so there's no
need to do it explicitly. We'd report the group name twice:
event 'tunable' for domain 'cd':
blkdeviotune.group_name: asdf
blkdeviotune.total_bytes_sec: 1234
blkdeviotune.group_name: asdf
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Due to the code share with 'qemuDomainSetBlockIoTune' the throttle group
setting code accepts the throttle group name also via typed parameters.
In 'qemuDomainSetThrottleGroup', this means that there are 2 ways to
pass it the throttle group name and both are handled slightly
differently. Specifically the name of the group used in the list of
groups is the name taken from the typed parameters rather than the one
passed via API. We also don't validate that they match.
Now if the name in the typed parameters is missing we'd add empty string
to the group list which would later crash when looking up the group
name.
To avoid this problem always use the name passed via argument. This is
achieved by passing it into 'qemuDomainSetBlockIoTuneFields' so that it
overrides whatever is in the typed parameters.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The refactor will simplify further change which will introduce another
source for the group name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Having a name of 0 characters makes no sense. Reject it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The dump is based on QEMU commit 'v10.2.0-476-gcf3e71d8fc'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The dump is based on QEMU commit 'v10.2.0-476-gcf3e71d8fc'
Notable changes:
- machine type changes: new types added, '5.0' machines dropped
- new and updated cpus:
- ClearwaterForest-v2-x86_64-cpu
- DiamondRapids-v1-x86_64-cpu
- GraniteRapids-v4-x86_64-cpu
- SapphireRapids-v5-x86_64-cpu
- SierraForest-v4-x86_64-cpu
- various CPU flag changes for 'max-x86_64-cpu'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Per commit message of 3f390db2e2 which added the
'fd-memory-numa-topology4' case, it's supposed to test memory
preallocation setting in the NUMA setup.
Now the 5.0 machine type is the last one supporting the old-style NUMA
config and thus also the only way to test the formatting of
'--mem-prealloc' commandline option for qemu, which was since replaced
by a property in the memory backend object.
Thus to preserve the idea of this test, add
'fd-memory-numa-topology4-old-machine' test case pinned to qemu-10.2,
which will use pc-i440fx-5.0 and modernize the normal case using the
latest qemu version by dropping the specific machine version.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
ACPI index support doesn't depend on the machine type and
'pc-i440fx-5.0' will be removed from qemu-11.0.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The test case specifically excercises our generator with the
'pc-i440fx-5.0' machine type which will be removed in qemu-11.0.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since v10.8.0-rc1~133 a different error is reported from
virNetDevTapCreate() when the tap device already exists (and
<interface/> XML specifies managed='no'). But the change covers
only one scenario: if multiqueue was requested in <inteface/> XML
BUT pre-created tap device doesn't have multi_queue flag set. The
opposite scenario: if the device has the multi_queue flag set BUT
no multiqueue was specified in the XML. Until now.
Resolves: https://issues.redhat.com/browse/RHEL-118303
Fixes: f6fb097e11
Fixes: 465a38154f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When connecting to a VMWare server, the hostname from URI is
resolved using esxUtil_ResolveHostname() which in turn calls
getaddrinfo(). But in the hints argument, we restrict the return
address to be IPv4 (AF_INET) which obviously fails if the address
to resolve is an IPv6 address. Set the hint to AF_UNSPEC which
allows both IPv4 and IPv6. While at it, also allow IPv4 addresses
mapped in IPv6 by setting AI_V4MAPPED flag.
Resolves: https://issues.redhat.com/browse/RHEL-138300
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, bhyve does not support UTC clock offset on ARM64.
However, when <clock offset= > is not specified in the domain XML,
UTC offset is used by default. That results in an incorrect
configuration for the bhyve ARM64 guests by default.
Workaround is to extend bhyveDomainDefPostParse() to fall back
to the LOCALTIME clock offset when UTC clock offset is not
supported by bhyve.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In shell, the following function doesn't echo '1' but '0':
func() {
local var=$(false)
echo $?
}
This is because '$?' does not refer to 'false' but 'local'. The
bash_builtins(1) manpage explains it well. And it also mentions
other commands behaving the same: export, declare and readonly.
Since it is really easy to miss this pattern, introduce a
syntax-check rule. Mind you, the following pattern (which passes
the rule) does check for the subshell exit code:
func() {
local var
var=$(false)
echo $?
}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The guest_is_on() function is documented to check whether given
domain is running and set guest_running variable accordingly. It
does so by running virsh (transitively), then setting the
variable and only after that comparing $? variable. This is
obviously wrong, because after the guest_running variable
assignment the $? variable no longer holds the exit code of
virsh. Even worse, as explained in the previous commit, it never
held that value in the first place. Fix this by firstly setting
the global variable and only after that running virsh.
Fixes: 08071ec0f1
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/839
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In Bash, the following code does not do what you think it does:
func() {
local var=$(false)
echo $?
}
Here, '0' is echoed even though false is designed to exit with a
non-zero code. This is because in fact the last executed command
is 'local' not 'false' and thus '$?' contains zero as in "yeah,
the variable is successfully declared" [1]. In our libvirt-guest
shell script, there are a few places like this. Fix them.
1: bash_builtins(1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This pulls in changes which cut down the build log sizes
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Passing warning flags to the C compiler results in incredibly long
command lines, which in turns results in incredibly large CI log
files. Our logs are so large that they often exceed the GitLab
file limits.
We've cut out the irrelevant cruft from the logs and they're still
too large. The only option left is to stop passing so many args
to the compiler.
Fortunately it is easy to achieve this with GCC/CLang as when seeing
an argument "@somepath" they will treat each line in "somepath" as
being an additional compiler argument.
Putting the warning flags in a 'c-warnings.txt' file is fairly
easy and a massive win. We don't lose anything from the CI logs
as we print the full set of warning flags at the end of running
'meson'. Meanwhile for interactive builds the flags are visible
in the c-warnings.txt file in the build directory root.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The compiler args are so verbose that CI jobs hit the limit on the
log file size GitLab is willing to capture.
Mitigate this a little bit by removing
-DIN_LIBVIRT
-Dabs_top_builddir="..dir.."
-Dabs_top_srcdir="..dir.."
The last two are already present in the meson-config.h file so
were redundant, and the first is easily added to internal.h.
NB, remote_protocol.x needed special treatment since it is an
exception which (intentionally) does NOT include internal.h
before libvirt/libvirt.h
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This patch changes how the maximum socket count is calculated.
On some systems (e.g. GB200), physical_package_id values are not
contiguous or zero-based. Instead of 0..N, they may contain large
arbitrary identifiers (e.g. 256123234). The previous implementation
assumed a 0..N range and used the maximum ID value directly.
This caused:
excessive memory allocation
extremely large loop bounds
OOM / DoS scenarios
unnecessary CPU time consumption
The new implementation computes the socket count as the number of unique
package IDs present on the node, rather than relying on the maximum numeric
value.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Alexandr Semenikhin <alexandr2e78@gmail.com>
Since the only two types of bridges we support are OVS bridges and
Linux host bridges, and since both of those now support vlan tagging,
we don't need to check the virtualport type etc - if there is a bridge
specified then we know the interface will support vlan tagging.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Somehow this was never done in virDomainNetDefValidate() (which is run
immediately post-parse) - it was only in
virDomainActualNetDefValidate() (which isn't done until the interface
is actually attached to the domain). While it is true that we *might*
not know if vlan tagging is supported for the interface if the
interface type == 'network', we otherwise will always know right away,
so we may as well check sooner than later.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Using &newdev->vlan when updating the vlan tag causes problems if the
interface is using a libvirt virtual network to get its vlan tag
info - in this case the info won't be properly pulled from the virtual network
config, but instead it will just use the (empty)
vlan object from the netdef.
The proper thing to do is to use virDomainNetGetActualVlan(), which
will pull the vlan tag from the network definition, if applicable.
Signed-off-by: Laine Stunp <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Prior to this patch, a network device of a running domain could be
updated to change the portForwards list, and libvirt wouldn't
complain, but the change would be silently ignored. This list is only
used by the passt backend, and passt can only change the list of
portForwards by killing and re-running the passt process, which we
don't want to do because that would destroy any open tcp session flows
in passt (ie. it would disrupt guest network traffic); we don't want
to do *that*, but we should at least let the user know that their
requested change isn't possible.
This patch checks if the portForwards list of the updated network
device exactly matches the portForwards list of the current network
device, and fails the update if they don't match.
Resolves: https://issues.redhat.com/browse/RHEL-7338
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function checks two arrays of virDomainNetPortForwards for
equality. It seemed silly to send a separate #elements for each array
just to check for their equality, but I could easily be convinced to
do it that way (rather than having the caller check for equal #element
and then just send it once).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a virSocketAddr has family AF_UNSPEC that means it is
unspecified/empty. If two virSocketAddrs are AF_UNSPEC then they are
equal, but virSocketAddrEqual() was countint that as *not* equal. This
doesn't make a difference for current uses of the function since they
all check that at least one of the virSocketAddrs is *not* AF_UNSPEC
before calling virSocketAddrEqual(), but an upcoming patch using
virSocketAddrEqual() will not be making that check, so let's fix the
utility function.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Prepare for introduction of nwfilter nftables driver test files.
Create new nwfilterxml2ebipfirewalldata directory for nwfilter
ebiptables driver specific test files.
This way we split off the xml test data from the argument test
files, which are driver specific.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dion Bosschieter <dionbosschieter@gmail.com>
Some functions inside of domain_logcontext.c call virLogManager
APIs. But that one is available only when remote driver is
enabled. Therefore, make domain_logcontext.c conditional, just
like log_manager.c is: if config.has('WITH_REMOTE').
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/842
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The test dump is obtained on a Raspberry Pi 5, thus the differences in
some of the config, but using this platform will give us the possibility
to update them more frequently.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Update the latest x86_64 dump to populate the 'scsi-block' and
'scsi-generic' device properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add code to probe properties of 'scsi-block' and 'scsi-generic' qemu
devices. For now the '.replies' are modified using the
'qemu-replies-tool' script and the data is annotated as fake and is in
fact missing. Further updates will populate them for any real dump.
The properties for the two devices will be later used e.g. to detect
support for persistent reservations migration support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The concept of moving a seclabel is used e.g. when a new image is
introduced to the backing chain (or one of the existing ones becomes
active during block commit). What it does is that it moves the metedata
remembering the original seclabel to the new image.
That idea works reasonably well if both the original and new image are
of same type e.g. a file, where they have comparable seclabel.
It breaks down though when you e.g. create a snapshot stored in a 'file'
on top of a disk originally backed by a 'block' storage source, since
the seclabels differ quite siginificantly.
This patch restricts the seclabel move in qemuSecurityMoveImageMetadata
to happen only if the storage sources are of same type to avoid the
issue. This means that the seclabels will not be remebered and will be
restored to the default but it's better than to transfer wrong labels.
Resolves: https://issues.redhat.com/browse/RHEL-114412
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When virt-aa-helper is invoked with a -u parameter followed by an actual UUID
as suggested by the help text, it throws an 'invalid UUID' error, even when the
UUID is the UUID of an existing VM. With this adjustment of the help text,
there's a better chance of users providing the correct input.
Signed-off-by: Erik Huelsmann <ehuels@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The aim of this rule is to capture the following pattern:
g_autofree const char *var = ...;
The pattern is problematic, because it frees a const pointer. If
written the old way, we'd get a compiler warning:
warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The 'forwardIf' variable inside of pfAddNatFirewallRules() is
declared as both g_autofree and const. This makes no sense.
Since the variable is g_strdup()-ed into right in the
declaration, it's not const. Drop that part of variable
declaration.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The 'resourceType' variable inside of hypervDomainAttachSerial()
is declared as both g_autofree and const. This makes no sense.
Since the variable is g_strdup_printf()-ed into a couple of lines
later, it's not const. Drop that part of variable declaration.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The ELN build root performs rebuilds of Fedora rawhide packages with
%{rhel} set instead of %{fedora}. The vast majority of the time this
will "do the right thing", but when we control -Dwerror we chose to
enable -Werror on RHEL on the basis that the GCC toolchain is stable
and thus we don't expect new warnings to appear. That does not hold
true for ELN which tracks rawhide GCC. Thus we have the rare situation
where we should explicitly check %{eln} not just %{rhel}.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are some features/improvements/bug fixes I've either
contributed or reviewed/merged. Document them for upcoming
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>