In the case that a new version of libvirt is started that uses
different rules to build the network firewall, we need to re-save the
status so that when the network is destroyed (or the *next* time
libvirt is restarted and wants to remove/re-add the firewall), it will
have the proper information to perform the firewall removal.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When destroying a network, the network driver has always assumed that
it knew what firewall rules had been added as the network was
started. This was usually correct - I only recall one time in the past
that the firewall rules added by libvirt were changed. But if the
exact rules used for a network *were* ever changed from one
build/version of libvirt to another, then we would end up attempting
to remove rules that hadn't been added, and could possibly *not*
remove rules that had been added.
The solution to this to not make such brash assumptions about the
past, but instead to save (in the network status object at network
start time) a list of all the rules needed to remove the rules that
were added for the network, and then use that saved list during
network destroy to remove exactly what was previous added.
Beyond making net-destroy more precise, there are other benefits:
1) We can change the details of the rules we add for networks from one
build/release of libvirt to another and painlessly upgrade.
2) The user can switch from one firewall backend to another by simply
changing the setting in network.conf and restarting
libvirtd/virtnetworkd.
In both cases, the restarted libvirtd/virtnetworkd will remove all the
rules that had been previously added (based on the network status),
and then add new rules (saving the new removal commands back into the
network status)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This virFirewall object will store the list of actions required to
remove the firewall that was added for the currently active instance
of the network, so it has been named "fwRemoval" (and when parsed into
XML, the <firewall> element will have the name "fwRemoval").
There are no uses of the fwRemoval object in the virNetworkObj yet,
but everything is in place to add it to the XML when formatted, parse
it from the XML when reading network status, and free the virFirewall
object when the virNetworkObj is freed.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
These functions convert a virFirewall object to/from XML so that it
can be serialized to disk (in a virNetworkObj's status file) and
restored later (e.g. after libvirtd/virtnetworkd is restarted).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virFirewallNewFromRollback() creates a new virFirewall object that
contains a copy of the "rollback" commands from an existing
virFirewall object, but in reverse order. The intent is that this
virFirewall be saved and used later to remove the firewall rules that
were added for a network.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This will be used to label (via "name='blah'") a firewall when it is
formatted to XML and written to the network status.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
So far this will only affect what happens if there is some failure
while applying the firewall rules; the rollback rules aren't yet
persistent beyond that time. More work is needed to remember the
rollback rules while the network is active, and use those rules to
remove the firewall for the network when it is destroyed.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If the VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK flag is set, each time
an iptables command is executed that is adding a rule or chain, a
corresponding command that will *delete* the same rule/chain is
constructed and added to the list of rollback commands. If we later
want to undo the entire firewall, we can just run those commands.
This isn't yet used anywhere, since
VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK isn't being set.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In the past virFirewall required all rollback commands for a group
(those commands necessary to "undo" any rules that had been added in
that group in case of a later failure) to be manually added by
switching into the virFirewall object into "rollback mode" and then
re-calling the inverse of the exact virFirewallAddCmd*() APIs that had
been called to add the original rules (ie. for each
"iptables --insert" command, for rollback we would need to add a
command with all arguments identical except that "--insert" would be
replaced by "--delete").
Because nftables can't search for rules to remove by comparing all the
arguments (it instead expects *only* a handle that is provided via
stdout when the rule was originally added), we won't be able to follow
the iptables method and manually construct the command to undo any
given nft command by just duplicating all the args of the command
(except the action). Instead we will need to be able to automatically
create a rollback command at the time the rule-adding command is
executed (e.g. an "nft delete rule" command that would include the
rule handle returned in stdout by an "nft add rule" command).
In order to make this happen, we need to be able to 1) learn whether
the user of the virFirewall API desires this behavior (handled by a new
transaction flag called VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK that
can be retrieved with the new virFirewallTransactionGetFlags() API),
and 2) add a new command to the current group's rollback command list (with
the new virFirewallAddRollbackCmd()).
We will actually use this capability in an upcoming patch.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Modify networkSetupPrivateChains() in the network driver to accept a
firewallBackend argument so it will know which backend to call. (right
now it always calls the iptables version of the lower level function,
but in the future it could instead call the nftables version based on
configuration).
But networkSetupPrivateChains() was being called with virOnce(), and
virOnce() doesn't support calling functions that require an argument
(it's based on pthread_once(), which accepts no arguments, so it's not
something we can easily fix in our implementation of virOnce()). To
solve this dilemma, this patch eliminates use of virOnce() by adding a
static lock, and putting all of networkSetupPrivateChains() (including
the setting of "chainInitDone") inside a lock guard - now the places
that used to call it via virOnce() can safely call it directly
instead (adding in the necessary argument to specify backend).
(If it turns out to be significant, we could optimize this by checking
for chainInitDone outside the lock guard, returning immediately if
it's already set, and then moving the setting of chainInitDone up to
the top of the guarded section.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It still can have only one useful value ("iptables"), but once a 2nd
value is supported, it will be selectable by setting
"firewall_backend=nftables" in /etc/libvirt/network.conf.
If firewall_backend isn't set in network.conf, then libvirt will check
to see if FIREWALL_BACKEND_DEFAULT_1 is available and, if so, set
that. (Since FIREWALL_BACKEND_DEFAULT_1 is currently "iptables", this
means checking to see it the iptables binary is present on the
system). If the default backend isn't available, that is considered a
fatal error (since no networks can be started anyway), so an error is
logged and startup of the network driver fails.
NB: network.conf is itself created from network.conf.in at build time,
and the advertised default setting of firewall_backend (in a commented
out line) is set from the meson_options.txt setting
"firewall_backend_default_1". This way the conf file will have correct
information no matter what ordering is chosen for default backend at
build time (as more backends are added, settings will be added for
"firewall_backend_default_n", and those will be settable in
meson_options.txt and on the meson commandline to change the ordering
of the auto-detection when no backend is set in network.conf).
virNetworkLoadDriverConfig() may look more complicated than necessary,
but as additional backends are added, it will be easier to add checks
for those backends (and to re-order the checks based on builders'
preferences).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This file is generated from network.conf.in because it will soon have
an item that must be modified according to meson buildtime config.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(This paragraph is for historical reference only, described only to
avoid confusion of past use of the name with its new use) In a past
life, virFirewallBackend had been a private static in virfirewall.c
that was set at daemon init time, and used to globally (i.e. for all
drivers in the daemon) determine whether to directly execute iptables
commands, or to run them indirectly via the firewalld passthrough
API. This was removed in commit d566cc55, since we decided that using
the firewalld passthrough API is never appropriate.
Now the same enum, virFirewallBackend, is being reintroduced, with a
different meaning and usage pattern. It will be used to pick between
using nftables commands or iptables commands (in either case directly
handled by libvirt, *not* via firewalld). Additionally, rather than
being a static known only within virfirewall.c and applying to all
firewall commands for all drivers, each virFirewall object will have
its own backend setting, which will be set during virFirewallNew() by
the driver who wants to add a firewall rule.
This will allow the nwfilter and network drivers to each have their
own backend setting, even when they coexist in a single unified
daemon. At least as important as that, it will also allow an instance
of the network driver to remove iptables rules that had been added by
a previous instance, and then add nftables rules for the new instance
(in the case that an admin, or possibly an update, switches the driver
backend from iptables to nftable)
Initially, the enum will only have one usable value -
VIR_FIREWALL_BACKEND_IPTABLES, and that will be hardcoded into all
calls to virFirewallNew(). The other enum value (along with a method
of setting it for each driver) will be added later, when it can be
used (when the nftables backend is in the code).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We know at the time a virFirewallCmd is created (with
virFirewallAddCmd*()) whether or not we will later want to ignore
errors encountered when attempting to apply that command - if
ignoreErrors is set in the AddCmd or if the group has already had
VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS set, then we ignore the errors.
Rather than setting the fwCmd->ignoreErrors only according to the arg
sent to virFirewallAddCmdFull(), and then later (at ApplyCmd-time)
combining that with the group transactionFlags setting (and passing it
all the way down the call chain), just combine the two flags right
away and store this final value in fwCmd->ignoreErrors when the
virFirewallCmd is created (thus avoiding the need to look at anything
other than fwCmd->ignoreErrors at the time the command is applied). Once
that is done, we can simply grab ignoreErrors from the object down in
virFirewallApply() rather than cluttering up the argument list on the
entire call chain.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We will already need a separate function for virFirewallApplyCmd for
iptables vs. nftables, but the only reason for needing a separate
function for virFirewallAddCmd* is that iptables/ebtables need to have
an extra arg added for locking (to prevent multiple iptables commands
from running at the same time). We can just as well add in the
-w/--concurrent during virFirewallApplyCmd, so move the arg-add to
ApplyCmd to keep AddCmd simple.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In normal practice a virFirewallCmd should never have 0 args by the
time it gets to the Apply stage, but at some time while debugging one
of the other patches in this series, exactly that happened (due to a
bug that was since squashed), and having a check for it helped
debugging, so let's permanently check for it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
I had originally named these as VIR_NETFILTER_* because I assumed the
same enum would eventually be used by our nftables backend as well as
iptables. But it turns out that in most cases it's not possible to
delete an nftables rule, so we just never used the enum anyway, so
this patch is renaming the values to IPTABLES_ACTION_*, and taking
advantage of the newly defined (via VIR_ENUM_DECL/IMPL)
iptablesActionTypeToString() to replace all the ternary operators used
to translate the enum into a string for the iptables commandline with
iptablesActionTypeToString().
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
These objects aren't rules, they are commands that are executed that
may create a firewall rule, delete a firewall rule, or simply list the
existing firewall rules. It's confusing for the objects to be called
"Rule" (especially in the case of the function
virFirewallRemoveRule(), which doesn't remove a rule from the
firewall, it takes one of the objects out of the list of commands to
execute! In order to remove a rule from the host's firewall, you have
to Add a "rule" (now "cmd" aka command) to the list that will, when
applied/run, remove a rule from the host firewall.)
Changing the name to virFirewallCmd makes it all much less confusing.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the toplevel iptables functions have been moved out of the
linux bridge driver into network_iptables.c, all of the utility
functions are used only within that same file, so simplify it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Although initially we will add exactly the same rules for the nftables
backend, the two may (hopefully) soon diverge as we take advantage of
nftables features that weren't available in iptables. When we do that,
there will need to be a different version of these functions (currently in
bridge_driver_linux.c) for each backend:
networkAddFirewallRules()
networkRemoveFirewallRules()
networkSetupPrivateChains()
Although it will mean duplicating some amount of code (with just the
function names changed) for the nftables backend, this patch moves all
of the rule-related code in the above three functions into iptables*()
functions in network_iptables.c, and changes the functions in
bridge_driver_linux.c to call the iptables*() functions. When we make
a different backend, it will only need to make equivalents of those 3
functions publicly available to the upper layer.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
These functions are only ever used by the network driver, and are so
specific to the network driver's usage of iptables that they likely
won't ever be used elsewhere. The files are renamed to
network_iptables.[ch] to be more in line with driver-specific file
naming conventions.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The virStateDriver struct has .stateInitialize callback which is
declared to return virDrvStateInitResult enum. But some drivers
return a plain int in their implementation which is UB.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There was no test for this and we mistakenly used 'B' rather than 'T'
when constructing the json value for this parameter. Thus, a value of
'off' was VIR_TRISTATE_SWITCH_OFF=2, which was translated to a boolean
value of 'true'.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Unlike other input types, evdev is not a true device since it's backed by
'-object'. We must use object-add/object-del monitor commands instead of
device-add/device-del in this particular case.
This patch adds support for handling live attachment and
detachment of evdev type devices.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/529
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we don't have any distro stuck with glib-2.56.0, we can
bump the glib version. In fact, this is needed, because of
g_clear_pointer. Since v7.4.0-rc1~301 we declare at compile time
what version of glib APIs we want to use (by setting
GLIB_VERSION_MIN_REQUIRED = GLIB_VERSION_MAX_ALLOWED = 2.56.0),
regardless of actual glib version in the host.
And since we currently require glib-2.56.0 and force glib to use
APIs of that version, some newer bits are slipping from us. For
instance: regular function version of g_clear_pointer() is used
instead of a fancy macro. So what? Well, g_clear_pointer()
function typecasts passed free function to void (*)(void *) and
then calls it. Well, this triggers UBSAN, understandably. But
with glib-2.58.0 the g_clear_pointer() becomes a macro which
calls the free function directly, with no typecasting and thus no
undefined behavior.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The .probe member of virSecurityDriver struct is declared to
return virSecurityDriverStatus enum. But there are two instances
(AppArmorSecurityManagerProbe() and
virSecuritySELinuxDriverProbe()) where callbacks are defined to
return an integer. This is an undefined behavior because integer
has strictly bigger space of possible values than the enum.
Defined those aforementioned callbacks so that they return the
correct enum instead of int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previously, the network device hotplug logic would try to ensure only CCW or
PCI addresses. With recent support for the usb-net model, this patch will
ensure USB addresses for usb-net network devices.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/14
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It can be safely removed from the VMX, VMWare will still boot the
machine and once another ethernet is added it is updated in the VMX to
zero. So do not require it and default to zero too since this part of
the XML is done as best effort and it is mentioned even in our
documentation.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virQEMUCapsSearchData' has been unused since
commit bc33b8c63911 ("qemu: capabilities: Drop the
virQEMUCapsCacheLookupByArch function")
Remove it.
Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
An incorrect check for domainRegister caused the DNS server for a
virtual domain to be registered with systemd-resolved even if
register='no' attribute was present. Only omitting the attribute
completely would disable the registration.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The nodedev code unhelpfully reports
couldn't convert node device def to mdevctl JSON
which hides the actual error message
No JSON parser implementation is available
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virdrivermoduletest will attempt to dlopen() each driver module,
so they must be build before the test can run.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We fail to express an ordering between the custom target that
generates the combined augeas test input file, and the meson
test command.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In case when the interface is being detached/reattached it may happen
that udev will return NULL from 'udev_device_get_sysname()'.
As the RPC code requires nonnull strings in the return array it fails to
serialize such reply:
libvirt: XML-RPC error : Unable to encode message payload
Fix this by simply ignoring such interfaces as there's nothing we can
report in such case.
A similar fix was done to 'udevConnectListAllInterfaces' in commit
2ca94317ac6.
Resolves: https://issues.redhat.com/browse/RHEL-34615
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make the array-filling operation of udevListInterfacesByStatus optional
and replace the completely redundant udevNumOfInterfacesByStatus by it.
Further patches fixing the listing will not need to be duplicated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some public objects (like virDomain, virInterface, and so on) are
missing g_autoptr() cleanup functions. Provide missing
declarations. Note, this is only for our internal use - hence
datatypes.h.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When starting a domain and there's no vCPU/emulator pinning set,
we query the list of all online physical CPUs and set affinity of
the child process (which eventually becomes QEMU) to that list.
We can't assume libvirtd itself had affinity to all online CPUs
and since affinity of the child process is inherited, we should
fix it afterwards. But that's not necessarily correct. Users
might isolate some physical CPUs and we should avoid touching
them unless explicitly told so (i.e. vCPU/emulator pinning told
us so).
Therefore, when attempting to set affinity to all online CPUs
subtract the isolated ones.
Before this commit:
root@localhost:~# cat /sys/devices/system/cpu/isolated
19,21,23
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 14835's current affinity list: 0-23
After:
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 17153's current affinity list: 0-18,20,22
Resolves: https://issues.redhat.com/browse/RHEL-33082
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This is a helper that parses /sys/devices/system/cpu/isolated
into a virBitmap. It's going to be needed soon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virFileReadValueBitmap() fails in the latter case, unfortunately.
Introduce a slightly modified version that accepts empty files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virBitmapParseUnlimited() fails in the latter case,
unfortunately. Introduce a slightly modified version that accepts
empty files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Extend the list of supported formats, update and clarify comment
in qemu.conf.in (removed misleading sentence about the order of
compression format types).
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/589
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Return value of a function 'virDomainChrDefNew' is dereferenced
at hyperv_driver.c without checking for NULL, which can lead to
NULL dereference immediately after.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Oleg Sviridov <oleg.sviridov@red-soft.ru>
Reviewed-by: Kristína Hanicová <khanicov@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Similar to commit 57d084febe, another case of the libxl driver not
adapting to modular daemons. When converting configuration that
contains a type='network' interface, the converter calls
virNetworkLookupByName, passing the hypervisor connection object
instead of a connection to virtnetworkd. E.g.
> cat dom.xml
...
<interface type='network'>
<source network='default'/>
</interface>
...
> virsh net-info default
Name: default
UUID: 25a5b089-1e71-4956-99aa-df2213bbb407
Active: yes
Persistent: no
Autostart: no
Bridge: virbr0
> virsh domxml-to-native xen-xl dom.xml
error: Network not found: default
Acquire a connection to virtnetworkd and use it when calling
virNetwork* APIs.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The mpx feature was removed from the corresponding qemu cpu models.
With mpx in the libvirt cpu models, libvirt believes the feature
to be implicitly enabled when creating qemu VMs, while in fact it is
disabled.
This became an issue when commit 94eacd5a5f introduced new vmx-*
features, of which some are dependent on mpx (see "feature_dependencies"
table in qemu target/i386/cpu.c), e.g. vmx-exit-clear-bndcfgs and
vmx-entry-load-bndcfgs. These features cannot be enabled by qemu
without also mpx being enabled, leading to the error message
error: Failed to create domain from testdomain.xml
error: operation failed: guest CPU doesn't match
specification: missing features: mpx,vmx-exit-clear-bndcfgs,
vmx-entry-load-bndcfgs
when trying to create a VM with a "host-model" cpu on a host that
does support mpx and the mentioned vmx-* features:
<domain>
...
<cpu mode='host-model' check='full' />
...
</domain>
Resolve the issue by removing mpx from libvirt's cpu models as well.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Features removed from a CPU model are marked with "removed='yes'"
attribute in the CPU map. Such features will always be present in a CPU
definition produced by libvirt regardless on their state. In other words
a running domain (even saved in a file) will always explicitly contain
states of all features removed from the specified CPU model. This
enables migration to older libvirt which would otherwise think the
affected features should be enabled as they are still included in the
CPU model in the older version of CPU map. Migration from an old libvirt
to a new one would be broken as the new libvirt would think the removed
features should be disabled (because they are not included in the CPU
model anymore), which might not be the case on the source host. Thus we
were refusing to remove CPU features unless they were never working and
no domain could even be running with those features enabled.
This patch removes the limitation. When handling CPU definitions with
missing features marked as removed in the specified CPU model, we know
whether it comes from a running domain, in which case it must have been
created by older libvirt where the missing CPU features were not removed
yet. This means the features must have been enabled on the source and we
can automatically fix the definition by adding the missing features with
correct states.
We can safely remove any CPU feature from our CPU models now, but it
should only be used for features removed from all versions of a given
CPU model in QEMU because unversioned models correspond to v1.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virCPUUpdate check the CPU definition for features that were marked as
removed in the specified CPU model and explicitly adds those that were
not mentioned in the definition. So far such features were added with
VIR_CPU_FEATURE_DISABLE policy, but the caller may want to use a
different policy in some situations, which is now possible via the
removedPolicy parameter.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCPUDefAddFeatureInternal helper function only fails if it is
called with VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE, which is only used in
virCPUDefAddFeature. The other callers (virCPUDefUpdateFeature and
virCPUDefAddFeatureIfMissing) will never get anything but 0 from
virCPUDefAddFeatureInternal and their return type can be changed to
void.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
address for it is saved in serialX.vspc in which case the
serialX.fileName is most probably something we can't get any useful
information from and we also fail during the parsing rendering any
dumpxml and similar tries unsuccessful.
Instead of parsing the vspc URL with something along the lines of
`virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
information that is very prune to misuse (the vSPC seemingly has a
protocol on top of the telnet connection; redefining the domain would
change the behaviour; the URL might have a fragment we are not saving;
etc.) or adding more XML knobs to indicate vSPC usage (which we would
not be able to configure; we'd have to properly error out everywhere;
etc.) let's just report dummy serial port that leads to nowhere (i.e.
type="null").
Resolves: https://issues.redhat.com/browse/RHEL-32182
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Users are seeing periodic segfaults from libvirt client apps,
especially thread heavy ones like virt-manager. A typical
stack trace would end up in the virNetClientIOEventFD method,
with illegal access to stale stack data. eg
==238721==ERROR: AddressSanitizer: stack-use-after-return on address 0x75cd18709788 at pc 0x75cd3111f907 bp 0x75cd181ff550 sp 0x75cd181ff548
WRITE of size 4 at 0x75cd18709788 thread T11
#0 0x75cd3111f906 in virNetClientIOEventFD /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1634:15
#1 0x75cd3210d198 (/usr/lib/libglib-2.0.so.0+0x5a198) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#2 0x75cd3216c3be (/usr/lib/libglib-2.0.so.0+0xb93be) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#3 0x75cd3210ddc6 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x5adc6) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#4 0x75cd3111a47c in virNetClientIOEventLoop /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1722:9
#5 0x75cd3111a47c in virNetClientIO /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2002:10
#6 0x75cd3111a47c in virNetClientSendInternal /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2170:11
#7 0x75cd311198a8 in virNetClientSendWithReply /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2198:11
#8 0x75cd31111653 in virNetClientProgramCall /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclientprogram.c:318:9
#9 0x75cd31241c8f in callFull /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6054:10
#10 0x75cd31241c8f in call /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6076:12
#11 0x75cd31241c8f in remoteNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/src/remote/remote_client_bodies.h:5959:9
#12 0x75cd31410ff7 in virNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/libvirt-network.c:952:15
The root cause is a bad assumption in the virNetClientIOEventLoop
method. This method is run by whichever thread currently owns the
buck, and is responsible for handling I/O. Inside a for(;;) loop,
this method creates a temporary GSource, adds it to the event loop
and runs g_main_loop_run(). When I/O is ready, the GSource callback
(virNetClientIOEventFD) will fire and call g_main_loop_quit(), and
return G_SOURCE_REMOVE which results in the temporary GSource being
destroyed. A g_autoptr() will then remove the last reference.
What was overlooked, is that a second thread can come along and
while it can't enter virNetClientIOEventLoop, it will register an
idle source that uses virNetClientIOWakeup to interrupt the
original thread's 'g_main_loop_run' call. When this happens the
virNetClientIOEventFD callback never runs, and so the temporary
GSource is not destroyed. The g_autoptr() will remove a reference,
but by virtue of still being attached to the event context, there
is an extra reference held causing GSource to be leaked. The
next time 'g_main_loop_run' is called, the original GSource will
trigger its callback, and access data that was allocated on the
stack by the previous thread, and likely SEGV.
To solve this, the thread calling 'g_main_loop_run' must call
g_source_destroy, immediately upon return, to guarantee that
the temporary GSource is removed.
CVE-2024-4418
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Martin Shirokov <shirokovmartin@gmail.com>
Tested-by: Martin Shirokov <shirokovmartin@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
