virt-host-validate: Report an error if failed to detect CGroups
As a part of its checks, virt-host-validate calls virCgroupNew()
to detect CGroup controllers which are then printed out. However,
virCgroupNew() can fail (with appropriate error message set).
Let's print an error onto stderr if that happens.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
Several libvirt functions are called from virt-host-validate.
Some of these functions do report an error on failure. But
reporting an error is coupled with freeing previous error (by
calling virResetError()). But we've never called
virErrorInitialize() and thus resetting error object frees some
random pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
storage: Don't overwrite error in virISCSIDirectDisconnect()
The iscsi-direct storage pool backend works merely like this: a
connection is established to the target (usually done via
virStorageBackendISCSIDirectSetConnection()), intended action is
executed (e.g. reporting LUNs, volume wiping), and at the end the
connection is closed via virISCSIDirectDisconnect().
The problem is that virISCSIDirectDisconnect() reports its own
errors which may overwrite error that occurred during LUN
reporting, or volume wiping or whatever.
To fix this, use virErrorPreserveLast() + virErrorRestore()
combo, which either preserves previously reported error message,
or is NOP if there's no error reported.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1797879 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Thu, 3 Jun 2021 20:04:30 +0000 (14:04 -0600)]
libxl: Support firmware autoselection
Xen only supports one firmware, making autoselection easy to implement.
In fact, <os firmware='efi'> is probably preferable in the Xen driver,
where libxl supports a firmware setting with accepted values such as
bios, ovmf, uefi (currently same semantics as ovmf), seabios, etc.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Wed, 2 Jun 2021 20:04:35 +0000 (14:04 -0600)]
libxl: Introduce domain def validate callback
Introduce libxlDomainDefValidate and move the existing validation
check from libxlDomainDefPostParse. Additional validation will be
introduced in subsequent patches.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thomas Huth [Thu, 27 May 2021 10:45:54 +0000 (12:45 +0200)]
meson.build: Compile with -Walloca
We are already compiling libvirt with -Wvla - so it does not make
too much sense to still allow people to use alloca() instead. Thus
put it on the list of things we want to warn about. Fortunately,
there is currently no warning with this flag, so the current
sources should be clean.
Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Thomas Huth [Thu, 27 May 2021 10:37:36 +0000 (12:37 +0200)]
meson.build: Remove the -Wvla-larger-then flag
The flag has a typo in it, it's "...-than=..." and not "...-then=...",
so this was in fact never used. Since we're also using -Wvla (without
size), we should already get warnings about any variable length arrays
anyway, so the additional "-Wvla-larger-than" does not make much sense
and thus we can simply drop this.
Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
Currently `virt-host-validate` will fail whenever one of its calls fail,
regardless of virHostValidateLevel set.
This behaviour is not optimal and makes it not exactly reliable as a
command line tool as other tools or scripts using it would have to check
its output to figure out whether something really failed or if a warning
was mistakenly treated as failure.
With this change, the behaviour of whether to fail or not, is defined by
the caller of those functions, based on the virHostValidateLevel passed
to them.
https://gitlab.com/libvirt/libvirt/-/issues/175
Signed-off-by: Fabiano Fidêncio <fabiano@fidencio.org> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These packages have not supported Go modules historically and when we
tried to introduce modules, we hit the problem that we're not using
semver for versioning.
The only way around this is to introduce new packages under a different
namespace, that will have the exact same code, but be tagged with a
different version numbering scheme.
This change proposes:
libvirt.org/go/libvirt
libvirt.org/go/libvirtxml
Note the hyphen is removed so that the import basename matches the
Go package name.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Fri, 4 Jun 2021 12:08:40 +0000 (14:08 +0200)]
docs: formatdomain: Document disk serial truncation status quo
Disk serials are truncated arbitrarily and silently by qemu depending on
the device type and how they are configured. Since changing the current
state would lead to more regressions than we have now, document that the
truncation is arbitrary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Stefan Hajnoczi [Mon, 7 Jun 2021 13:50:24 +0000 (14:50 +0100)]
docs: virtiofs: describe memfd memory backend
Nowadays memfd is the most convenient memory backend for vhost-user
devices. Compared to file-backend memory and hugepages, there is no need
to worry about configuring the location of the shm directory or
allocating hugepages.
Cc: Michal Prívozník <mprivozn@redhat.com> Cc: Ján Tomko <jtomko@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
remoteGetUNIXSocket: Complete variable rename for WIN32
In fcdcf8f70cf the remoteGetUNIXSocket() function was changed and
one new variable was introduced (among other things): @env_name.
However, for WIN32 case the variable changed name to @env_path
which builds mingw builds.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
virnetsocket: Mark @spawnDaemonPath of virNetSocketNewConnectUNIX() unused
The virNetSocketNewConnectUNIX() function was changed in 48f66cfe3e. And its WIN32 version (which just reports an error)
was updated too, but this new argument @spawnDaemonPath was not
marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are two variables that can be freed automatically: @cmd
(which allows us to drop explicit virCommandFree() call at the
end of the function) and @help which was never freed (and thus
leaked).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
ch_driver: Don't error out if CH_CMD was not found
The CH driver needs "cloud-hypervisor" binary. And if none was
found then the initialization of the driver fails as
chStateInitialize() returns VIR_DRV_STATE_INIT_ERROR. This in
turn means that whole daemon fails to initialize. Let's return
VIR_DRV_STATE_INIT_SKIPPED in this particular case, which
disables the CH drvier but lets the daemon run.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
ch_conf: Dissolve chExtractVersionInfo() in chExtractVersion()
After previous patches, there's not much value in
chExtractVersion(). Rename chExtractVersionInfo() to
chExtractVersion() and have it use virCHDriver directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
ch_conf: Move error reporting into chExtractVersionInfo()
If chExtractVersionInfo() fails, in some cases it reports error
and in some it doesn't. Fix those places and drop reporting error
from chExtractVersion() which would just overwrite more specific
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Closes: https://gitlab.com/libvirt/libvirt/-/issues/173 Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: wire up support for timer period audio setting
Closes: https://gitlab.com/libvirt/libvirt/-/issues/171 Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When the default driver mode requests the modular daemons, we still
defaulted to spawning libvirtd if the URI was NULL, because we don't
know which driver specific daemon to spawn. virtproxyd has logic
that can handle this as it is used for compatibility when accepting
incoming TCP connections with a NULL URI.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "spawnDaemon" and "binary" parameters are co-dependant, with the
latter non-NULL, if-and-only-if the former is true. Getting rid of the
"spawnDaemon" parameter simplifies life for the callers and eliminates
an error checking scenario.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
remote: don't populate daemon path if autostart is not required
When deciding what socket to connect to, we build the daemon path
that we need to autostart. This path only needs to be populated
if we actually intend to use autostart.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
remote: change socket helper to return full daemon path
The remoteGetUNIXSocket method currently just returns the daemon name
and the caller then converts this to a path. Except the SSH helper
didn't do this, so it was relying on later code expanding $PATH, and
this doesn't allow for build root overrides.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
remote: consistently use flags for passing ro/user/autostart props
We have helper methods that return boolans for ro/user/autostart
properties. We then pack them into a flags parameter, and later
unpack them again. This makes the code consistently use flags
throughout.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
William Douglas [Wed, 12 May 2021 17:01:31 +0000 (10:01 -0700)]
Add basic driver for the Cloud-Hypervisor
Cloud-Hypervisor is a KVM virtualization using hypervisor. It
functions similarly to qemu and the libvirt Cloud-Hypervisor driver
uses a very similar structure to the libvirt driver.
The biggest difference from the libvirt perspective is that the
"monitor" socket is seperated into two sockets one that commands are
issued to and one that events are notified from. The current
implementation only uses the command socket (running over a REST API
with json encoded data) with future changes to add support for the
event socket (to better handle shutdowns from inside the VM).
This patch adds support for the following initial VM actions using the
Cloud-Hypervsior API:
* vm.create
* vm.delete
* vm.boot
* vm.shutdown
* vm.reboot
* vm.pause
* vm.resume
To use the Cloud-Hypervisor driver, the v15.0 release of
Cloud-Hypervisor is required to be installed.
Some additional notes:
* The curl handle is persistent but not useful to detect ch process
shutdown/crash (a future patch will address this shortcoming)
* On a 64-bit host Cloud-Hypervisor needs to support PVH and so can
emulate 32-bit mode but it isn't fully tested (a 64-bit kernel and
32-bit userspace is fine, a 32-bit kernel isn't validated)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: William Douglas <william.douglas@intel.com>
In the previous commit I've changed what API is called from
'virsh setmem' command. However, since virsh-optparse test is ran
only when expensive tests are enabled I've completely missed that
the expected output for virsh-optparse test must be updated too
as it contains the API.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 19 May 2021 09:58:27 +0000 (11:58 +0200)]
virsh-domain: Drop support for old APIs in cmdSetmem and cmdSetmaxmem
Some of our really old APIs are missing @flags argument. We
introduced their variants with "Flags" suffix and wired some
logic into virsh to call the new variant only if necessary. This
enables virsh to talk to older daemon which may be lacking new
APIs.
However, in case of cmdSetmem() we are talking about v0.1.1
(virDomainSetMemory()) vs. v0.9.0 (virDomainSetMemoryFlags()) and
in case of cmdSetmaxmem() we are talking about v0.0.3
(virDomainSetMaxMemory()) vs v0.9.0 (virDomainSetMemoryFlags()).
Libvirt v0.9.0 was released more than 10 years ago and recently
we dropped support for RHEL-7 which has v4.5.0 (released ~3 years
ago). Thus it is not really necessary to have support in virsh
for such old daemons.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 31 May 2021 14:42:13 +0000 (16:42 +0200)]
node_device_udev: Also process ID_TYPE=cd/dvd in udevProcessStorage()
When processing node devices, the udevProcessStorage() will be
called if the device is some form of storage. In here, ID_TYPE
attribute is queried and depending on its value one of more
specialized helper functions is called. For instance, for
ID_TYPE=="cd" the udevProcessCDROM() is called, for
ID_TYPE=="disk" the udevProcessDisk() is called, and so on.
But there's a problem with ID_TYPE and its values. Coming from
udev, we are not guaranteed that ID_TYPE will contain "cd" for
CDROM devices. In fact, there's a rule installed by sg3_utils
that will overwrite ID_TYPE to "cd/dvd" leaving us with an
unhandled type. Fortunately, this was fixed in their upstream,
but there are still versions out there, on OS platforms that we
aim to support that contain the problematic rule. Therefore, we
should accept both strings.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1848875 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 31 May 2021 14:24:18 +0000 (16:24 +0200)]
node_device_udev: Make udevGetStringProperty() return void
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 31 May 2021 14:19:12 +0000 (16:19 +0200)]
node_device_udev: Make udevGenerateDeviceName() return void
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jim Fehlig [Wed, 26 May 2021 20:05:05 +0000 (14:05 -0600)]
libxl: adjust handling of libxl_device_nic objects
libxl objects are supposed to be initialized and disposed. Adjust
libxlMakeNic to use an already initialized object owned by the caller.
Adjust libxlMakeNicList to initialize the list of objects, before they
are filled by libxlMakeNic. The libxl_domain_config object passed to
libxlMakeNicList is owned by the caller and will be disposed with
libxl_domain_config_dispose, which also disposes embedded objects such
as libxl_device_nic.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Acked-by: Olaf Hering <olaf@aepfle.de>
Pavel Hrdina [Thu, 27 May 2021 13:32:17 +0000 (15:32 +0200)]
virDomainDiskDefParseSource: parse source bits from driver element
Before the mentioned commit we always parsed the whole disk definition
for qemuDomainBlockCopy API but we only used the @src part. Based on
that assumption the code was changed to parse only the disk <source>
element.
Unfortunately that is not correct as we need to parse some parts of
<driver> element as well.
Fixes: 0202467c4ba8663db2304b140af609f93a9b3091 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Pavel Hrdina [Thu, 27 May 2021 13:25:51 +0000 (15:25 +0200)]
domain_conf: extract disk driver source bits to its own function
Attribute `type` and sub-element `metadata_cache` are internally stored
in the `virStorageSource` structure. Sometimes we only care about the
disk source bits so we need a dedicated helper for that.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Andrea Bolognani [Thu, 27 May 2021 13:20:43 +0000 (15:20 +0200)]
meson: Switch to autodetection for apparmor_profiles
Match the behavior of most other features.
This will result in a change in behavior, because profiles will
now be installed whenever AppArmor support is enabled; on the
other hand, this is probably the behavior users expected in the
first place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 26 May 2021 16:46:20 +0000 (18:46 +0200)]
meson: Use dependency() when possible
This is the preferred way to figure out whether a library is
available, and for the most part we can just adopt it right
away; in a few cases, unfortunately, we're stuck with using
cc.find_library() until further down the road, when all our
target platforms ship with pkg-config enabled versions of the
various libraries.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 26 May 2021 16:42:40 +0000 (18:42 +0200)]
meson: Rewrite libacl check
libacl is Linux-only, so we don't need to explicitly check for
either the target platform or header availability, and we can
simply rely on cc.find_library() instead. The corresponding
preprocessor define is renamed to more accurately reflect the
nature of the check.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 26 May 2021 15:33:08 +0000 (17:33 +0200)]
meson: Fix firewalld check
firewalld is Linux-only, so it should be disabled by default
everywhere else and attempts to explicitly enable firewalld
support on non-Linux targets should result in an error.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 26 May 2021 15:47:05 +0000 (17:47 +0200)]
meson: Fix vstorage detection
We're supposed to error out if the user has explicitly asked
for vstorage support to be enabled and that can't be done, but
we've been looking at the wrong option.
Fixes: 2127d53f2f90443f3e4919c1082350ee2b3096f1 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 26 May 2021 17:03:16 +0000 (19:03 +0200)]
meson: Use get_pkgconfig_variable('cflags')
Meson offers a native convenience method that can be used to
fetch pkg-config variables from a dependency, so we can use
that instead of calling pkg-config manually.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Masayoshi Mizuma [Thu, 27 May 2021 16:55:12 +0000 (12:55 -0400)]
qemuProcessSetupDisksTransientSnapshot: Skip enabling transientOverlayCreated flag
QEMU_DOMAIN_DISK_PRIVATE(disk)->transientOverlayCreated flag
gets true unexpectedly on qemuProcessSetupDisksTransientSnapshot() when
the disk has <transient shareBacking='yes'> option.
The flag should be enabled on qemuDomainAttachDiskGeneric() after the
overlay setup is completed.
Skip enabling transientOverlayCreated for the disk here.
Fixes: 75871da0ecb8b552f9e304d0f83e216839bbf82d Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jonathon Jongsma [Wed, 26 May 2021 15:40:26 +0000 (10:40 -0500)]
nodedev: Revert auto-start property for mdevs
We supported autostart of node devices via an xml element, but this
is not consistent with other libvirt objects which use an explicit API
for setting autostart status. So revert this and implement it as an
official API in a future commit.
The initial support was refactored after merging, so this commit reverts
both of those previous commits.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jonathon Jongsma [Wed, 26 May 2021 15:40:25 +0000 (10:40 -0500)]
Partial Revert of "tests: nodedevxml2xmltest: test more mdev files"
This reverts parts of commit bb8c3b61208ed0f29dcbeca857529600f04b3146
that added tests for autostart functionality (which will be reverted in
the following commit)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
remote: fix regression connecting to remote session daemon
While we couldn't historically connect to the remote session daemon
automatically, we do allow the user to set an explicit socket path
to enable the connections to work. This ability was accidentally
lost in
remote: move proxy/mode defaults after URI parsing
Currently the defaults for the proxy/mode settings are set before
parsing URI parameters. A following commit will introduce a dependancy
on the URI parsing for the defaults, so they need to move.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Thu, 27 May 2021 09:42:19 +0000 (11:42 +0200)]
virCapabilitiesHostNUMAInitReal: Don't jump over cleanup
In one of my recent commits I've done some renaming. But whilst
doing so I also mistakenly replaced 'goto cleanup' with 'return
-1' in virCapabilitiesHostNUMAInitReal() which was incorrect.
Fixes: fe25224fdaa53bbeceed3ddeef1b3a150665e656 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
virFileFindResource needs to be given the absolute build path otherwise
its results will vary according to the CWD, leading to spurious failures
in dev testing.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Masayoshi Mizuma [Wed, 26 May 2021 20:19:22 +0000 (16:19 -0400)]
qemuDomainAttachDiskGenericTransient: Add NULL check in case the overlay disk already exists
When <transient shareBacking='yes'> is set to a disk and the overlay
disk already exists because of something abnormal, libvirt is terminated
by Segmentation fault.
# virsh start Test0
error: Disconnected from qemu:///system due to end of file
error: Failed to start domain 'Test0'
error: End of file while reading data: Input/output error
Add NULL check for snapdiskdef so that the rollback can work correctly.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Fixes: 2e94002d2ace4e4a6dbfc13a84fdab28f22c5c4a Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Fri, 21 May 2021 18:51:05 +0000 (14:51 -0400)]
qemu: adjust the maxmemlock limit when hotplugging a vDPA device
and re-adjust if the hotplug fails.
This fixes a bug found during testing of
https://bugzilla.redhat.com/1939776, which was supposed to be resolved
by commit 98e22ff749, but failed to account for the case of device
hotplug.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 21 May 2021 18:32:00 +0000 (14:32 -0400)]
qemu_hotplug.c: add net devices to the domain list earlier
An upcoming patch will be checking if the addition of a new net device
requires adjusting the domain locked memory limit, which must be done
prior to sending the command to qemu to add the new device. But
qemuDomainAdjustMaxMemLock() checks all (and only) the devices that
are currently in the domain definition, and currently we are adding
new net devices to the domain definition only at the very end of the
hotplug operation, after qemu has already executed the device_add
command.
In order for the upcoming patch to work, this patch changes
qemuDomainAttachNetDevice() to add the device to the domain nets list
at an earlier time. It can't be added until after PCI address and
alias name have been determined (because both of those examine
existing devices in the domain to figure out a unique value for the
new device), but must be done before making the qemu monitor call.
Since the device has been added to the list earlier, we need to
potentially remove it on failure. This is done by replacing the
existing call to virDomainNetRemoveHostdev() (which checks if this is
a hostdev net device, and if so removes it from the hostdevs list,
since it could have already been added to that list) with a call to
the new virDomainNetRemoveByObj(), which looks for the device on both
nets and hostdevs lists, and removes it where it finds it.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 21 May 2021 18:28:10 +0000 (14:28 -0400)]
conf: new function virDomainNetRemoveByObj()
virDomainNetRemove() requires the index of the net device you want to
remove from the list, but in some cases you may not have the index
handy, only the object itself (or the object may not have been added
to the domain's list). virDomainNetRemoveByObj() first tries to find
the given object in the nets list, and deletes that if it is found.
As with virDomainNetRemove() it always unconditionally tries to remove
the device from the hostdevs list (in case it is the ridiculous
combined net+hostdev device created for <interface type='hostdev'>).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 21 May 2021 17:03:17 +0000 (13:03 -0400)]
qemu_hotplug.c: don't skip cleanup on failures of qemuDomainAttachNetDevice
We have many places where the earliest error returns from a function
skip any cleanup label at the bottom (the assumption being that it is
so early in the function that there isn't yet anything that needs to
be explicitly undone on failure). But in general it is a bad sign if
there are any direct "return" statements in a function at any time
after there has been a "goto cleanup" - that indicates someone thought
that an earlier point in the code had done something needing cleanup,
so we shouldn't be skipping it.
There were two occurences of a "return -1" after "goto cleanup" in
qemuDomainAttachDeviceNet(). The first of these has been around for a
very long time (since 2013) and my assumption is that the earlier
"goto cleanup" didn't exist at that time (so it was proper), and when
the code further up in the function was added, the this return -1 was
missed. The second was added during a mass change to check the return
from qemuInterfacePrepareSlirp() in several places (commit 99a1cfc43889c6d); in this case it was erroneous from the start.
Change both of these "return -1"s to "goto cleanup". Since we already
have code paths earlier in the function that goto cleanup, this should
not cause any new problem.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 25 May 2021 09:32:37 +0000 (11:32 +0200)]
virxml: Avoid double indentation of <metadata/> element
There was a recent change in libxml2 that caused a trouble for
us. To us, <metadata/> in domain or network XMLs are just opaque
value where management application can store whatever data it
finds fit. At XML parser/formatter level, we just make a copy of
the element during parsing and then format it back. For
formatting we use xmlNodeDump() which allows caller to specify
level of indentation. Previously, the indentation was not
applied onto the very first line, but as of v2.9.12-2-g85b1792e
libxml2 is applying indentation also on the first line.
This does not work well with out virBuffer because as soon as we
call virBufferAsprintf() to append <metadata/> element,
virBufferAsprintf() will apply another level of indentation.
Instead of version checking, let's skip any indentation added by
libxml2 before virBufferAsprintf() is called.
Note, the problem is only when telling xmlNodeDump() to use
indentation, i.e. level argument is not zero. Therefore,
virXMLNodeToString() which also calls xmlNodeDump() is safe as it
passes zero.
Tested-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Tue, 25 May 2021 09:30:33 +0000 (11:30 +0200)]
virxml: Report error if virXMLFormatMetadata() fails
I guess this is more of an academic problem, because if
<metadata/> content was problematic we would have caught the
error during parsing. Anyway, as is this function returns -1
without any error reported. Fix it by reporting one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Tue, 25 May 2021 09:21:02 +0000 (11:21 +0200)]
virxml: Introduce and use virXMLFormatMetadata()
So far, we have to places where we format <metadata/> into XMLs:
domain and network. Bot places share the same code. Move it into
a helper function and just call it from those places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Support for glusterfs with KVM is being dropped in RHEL-9 in the
virtualization stack.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
it also removed the logic that applied to RHEL-8 wrt arch list
and lost PowerPC 64 support on 8. This reverts that part of the
change but with the condition reversed to prioritize the future
state.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
projects / thirdparty / libvirt.git / log
