Egor Makrushin [Wed, 20 Dec 2023 12:38:08 +0000 (15:38 +0300)]
conf: fix integer overflow in virDomainControllerDefParseXML
Multiplication results in integer overflow.
Thus, replace it with ULLONG_MAX and change
def->opts.pciopts.pcihole64size type to ULL.
Update variable usage according to new type.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Egor Makrushin <emakrushin@astralinux.ru> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ján Tomko [Tue, 19 Dec 2023 15:46:53 +0000 (16:46 +0100)]
remote: DeserializeDomainDiskErrors: remove dead code
As of commit b2d079c113a which converted this function to use g_strdup,
the error label is only reached when i = 0, rendering it useless.
Remove it.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/572 Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Mon, 18 Dec 2023 23:48:13 +0000 (16:48 -0700)]
apparmor: Add capabilities for PCI passthrough to virtxend profile
When splitting out the apparmor modular daemon profiles from the
libvirtd profile, the net_admin and sys_admin capabilities were
dropped from the virtxend profile. It was not known at the time
that these capabilities were needed for PCI passthrough. Without
the capabilities, the following messages are emitted from the audit
subsystem
It appears sys_admin is needed to simply read from the PCI dev's
sysfs config file. The net_admin capability is needed when setting
the MAC address of an SR-IOV virtual function.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ján Tomko [Wed, 13 Dec 2023 13:05:32 +0000 (14:05 +0100)]
qemu: virtiofs: auto-fill idmap for unprivileged use
If the user did not specify any uid mapping, map its own
user ID to ID 0 inside the container and the rest of the IDs
to the first found user's authorized range in /etc/sub[ug]id
Ján Tomko [Thu, 7 Sep 2023 14:22:43 +0000 (16:22 +0200)]
qemu: virtiofs: do not force UID 0
Remove the explicit setting of uid 0 when running virtiofsd.
It is not required for privileged mode, where virtiofsd will be run
as root anyway. And for unprivileged mode, virtiofsd no longer requires
to be run as root.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 11 Dec 2023 14:37:51 +0000 (15:37 +0100)]
qemuDomainBlockResize: Properly resize disks with storage slice
Until now resizing a disk with a storage slice would break in one of the
following ways:
1) for a non-raw format, the virtual size would change, but the slice
would still remain in place
2) for raw disks qemu would refuse to change the size
The only reasonable scenario we want to support is a 'raw' image with 0
offset (inside a block device), where we can just drop the slice.
Anything else comes from a non-standard storage setup that we don't want
to touch.
To facilitate the resize, we first remove the 'size' parameter in qemu
thus dropping the slice and then instructing qemu to resize the disk.
Resolves: https://issues.redhat.com/browse/RHEL-18782 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 23 Oct 2023 13:46:26 +0000 (15:46 +0200)]
qemu: block: Make 'slice' layer effective for 'raw' storage source
Rather than pulling the configuration of the storage slice into the
'format' layer make the 'slice' layer effective for raw disks with a
storage slice. This was made possible by the recent refactors which made
the 'format' layer optional if not needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Resizing of block-backed storage requires the user to pass the exact
capacity of the device. Implement code which will query it instead so
the user doesn't need to do that.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/449 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow users to easily resize 'raw' images on block devices to the full
capacity of the block device. Obviously this won't work on file-backed
storage (filling the remaining capacity is most likely wrong) or for
formats with metadata due to the overhead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 6 Dec 2023 14:57:16 +0000 (15:57 +0100)]
qemu: migration: Automatically fix non-shared-storage migration to bigger block devices
QEMU's blockdev-mirror job doesn't allow copy into a destination which
isn't exactly the same size as source. This is a problem for
non-shared-storage migration when migrating into a raw block device, as
there it's very hard to ensure that the destination size will match the
source size.
Rather than failing the migration, we can add a storage slice in such
case automatically and thus make the migration pass.
To do this we need to probe the size of the block device on the
destination and if it differs form the size detected on the source we'll
install the 'slice'.
An additional handling is required when persisting the VM as we want to
propagate the slice even there to ensure that the device sizes won't
change.
Resolves: https://issues.redhat.com/browse/RHEL-4607 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 6 Dec 2023 14:42:29 +0000 (15:42 +0100)]
qemu: Move and export qemuDomainStorageUpdatePhysical and dependencies
Move qemuDomainStorageUpdatePhysical, qemuDomainStorageOpenStat,
qemuDomainStorageCloseStat to qemu_domain.c and export them. They'll be
reused in the migration code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 7 Dec 2023 14:22:58 +0000 (15:22 +0100)]
qemu: migration: Improve handling of VIR_MIGRATE_PARAM_DEST_XML with VIR_MIGRATE_PERSIST_DEST
When a user provides a migration XML via the VIR_MIGRATE_PARAM_DEST_XML
it's expected that they want to change ABI-compatible aspects of the XML
such as the disk paths or similar.
If the user requests persisting of the VM but does not provide an
explicit persistent XML libvirt would take the persistent XML from the
source of the migration as the persistent config. This usually involves
the old paths to images.
Doing this would result into failure to start the VM.
It makes more sense to take the XML used for migration and use that as
the base for persisting the config.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 6 Dec 2023 13:47:01 +0000 (14:47 +0100)]
qemuMigrationDstPrecreateStorage: Fix and clarify logic
While it's intended that qemuMigrationDstPrecreateDisk is called with
any kind of the disk, the logic in qemuMigrationDstPrecreateStorage
which checks the existence of the image wouldn't properly handle e.g.
network backed disks, where it would attempt to use virFileExists() on
the disk's 'src->path'.
Fix the logic by first skipping disks not meant for migration, then do
the existence check only when 'disk->src' is local storage.
Since qemuMigrationDstPrecreateDisk has a debug statement there's no
need to have an extra one right before calling into it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Fri, 8 Dec 2023 01:38:22 +0000 (18:38 -0700)]
remote: Fix cached hypervisor type
Commit 501825011c switched the remote driver to using g_auto, but missed
one case of needing to steal a pointer holding the hypervisor type.
Without it, memory is freed and the output of 'virsh version' has random
output
Compiled against library: libvirt 10.0.0
Using library: libvirt 10.0.0
Using API: ��%�U 10.0.0
Running hypervisor: ��U 8.1.3
Ths change also fixes random SIGABRT from perl processes running
libvirt-tck tests.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Thu, 7 Dec 2023 12:01:34 +0000 (13:01 +0100)]
virNodeDeviceCapStorageDefFormatBlocksize: Report sector size and count together
Report both block count and size together when either one of them is
present equivalently to what the schema type 'blockData' in
'schemas/nodedev.rng' defines.
Resolves: https://issues.redhat.com/browse/RHEL-18165 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 7 Dec 2023 11:56:14 +0000 (12:56 +0100)]
virNodeDeviceCapStorageDefFormat: Extract formatting of block size data
Unfortunately the XML is designed in a weird way, where based on whether
media in the device is removable the sizing is either part of a
subelement or placed directly on top level. The logic itself is
identical so it can be extracted into a function to simplify the
formatter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 6 Dec 2023 07:36:13 +0000 (08:36 +0100)]
virsh: migrate: Fix logic bug in interlock of --copy-storage-synchronous-writes flag
As the error message states we want to check that one of
'--copy-storage-all' or '--copy-storage-inc' is used, but the condition
mentioned VIR_MIGRATE_NON_SHARED_DISK twice.
Fixes: 1c2bd205edd
Resolves: https://issues.redhat.com/browse/RHEL-17596 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 5 Dec 2023 15:17:57 +0000 (16:17 +0100)]
qemu: migration: Validate migration XML
There's no point in skiping the validation step:
- on the source, the VM is parsed for ABI stability checking, thus the
equivalent config was validated when the VM was started
- on the destination, the XML will be validated inside qemuProcessInit
very soon after it is parsed
This fixes problems such as if the user uses a relative path in the disk
source or omits the source, as the disk migration code reasonably
expects that all checks were performed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
DO_TEST_CPUID(arch, host, json) is a multipart test. It consists of the
following tests:
- cpuTestHostCPUID()
- cpuTestGuestCPUID(with JSON_* flag)
- cpuTestCPUIDSignature()
- DO_TEST_JSON():
- if json==JSON_MODELS:
- cpuTestGuestCPUID(without JSON_* flag)
- cpuTestJSONCPUID()
- cputestJSONSignature()
Notice that for tests with json==JSON_MODELS, cpuTestGuestCPUID() is
actually called twice but with different arguments. The first one passes
JSON_MODELS to the test function, while the second one passes 0.
The main difference in behavior when calling cpuTestGuestCPUID() with or
without the flag is that in the first case, it parses the captured qemu
output from $ARCH-cpuid-$CPU.json. It extracts the cpu model list from
that JSON, and uses that to filter out possible cpu models to match.
In other words, it tries to match the cpu to a model that was supported
by the qemu version that was used to generate this JSON file. When it
finds a match, it generates a cpu definition and compares the xml form
of that definition with the file $ARCH-cpuid-$CPU-guest.xml.
When called without the JSON_MODELS flag, it simply attempts to match it
against the full libvirt cpu map and doesn't attempt to filter out any
matches based on the JSON qemu cpu model list. After it finds a match,
it generates an xml definition for the cpu and compares it to the same
file listed above. So if these two invocations disagree on the cpu match
(e.g. because libvirt has added a cpu model to its cpu map that matches
better than one that was supported by the version of qemu that generated
the JSON file) the test will fail.
This duplicate call to cpuTestGuestCPUID() was originally added in
commit 49c945a6f5c885394507f88086cc2f9461df7c27. The original
justification for that commit was to fix test failures when the Qemu
driver was disabled. But since DO_TEST_JSON() is #defined empty when
qemu is disabled, this particular invocation would not even be executed
in this scenario, so it doesn't seem relevant.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If libvirt is built in client only mode, the libvirtd/virtqemud/etc
daemons won't exist. If the client is told to connect to a local
hypervisor, it'll see the socket doesn't exist, try to spawn the
daemon and then re-try connecting to the socket for a few seconds.
Ultimately this will fail because the daemon doesn't exist and the
user gets an error message
error: Failed to connect socket to '/run/user/1000/libvirt/virtqemud-sock': No such file or directory
technically this is accurate, but it doesn't help identify the root
cause. With this change it will now report
error: binary 'virtqemud' does not exist in $PATH: No such file or directory
and will skip all the socket connect retries
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On device-update, when user requests change of
trustGuestRxFilters we currently do nothing. Nor error out, nor
act on the request. While we can just throw an error,
implementing this is pretty trivial.
Resolves: https://issues.redhat.com/browse/RHEL-735 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuMonitorJSONQueryRxFilter: Allow @filter to be NULL
Sometimes it may be handy to just issue the query-rx-filter
monitor command without actually parsing the output. Adapt
qemuMonitorJSONQueryRxFilter() to this behavior.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 30 Nov 2023 12:26:14 +0000 (13:26 +0100)]
qemu: Relax check for memory device coldplug
When cold plugging a memory device we check whether there's
enough free memory slots to accommodate new module. Well, this
checks makes sense only for those memory devices that are plugged
into DIMM slots (DIMM and NVDIMM models). Other memory device
models, like VIRTIO_MEM, VIRTIO_PMEM or SGX_EPC are attached into
PCI bus, or no bus at all.
Resolves: https://issues.redhat.com/browse/RHEL-15480 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
qemu: Move memory device coldplug into a separate function
The code that handles coldplug of a memory device is pretty
trivial and such could continue to live in the huge switch()
where other devices are handled. But the code is about to get
more complicated. To help with code readability, move it into a
separate function.
And while at it, make the function accept a double pointer to the
memory device definition to make the ownership transfer obvious
(the device is part of the domain on successful run).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Andrea Bolognani [Thu, 30 Nov 2023 17:22:54 +0000 (18:22 +0100)]
rpm: Add Requires on scrub
We currently have a BuildDepends on it, which doesn't help
ensuring that the tool is available at runtime, when the storage
driver might need to spawn it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 29 Nov 2023 16:33:35 +0000 (17:33 +0100)]
rpm: Drop MinGW BuildRequires on libgcrypt/libgpg-error
They've been there since the start, but we don't seem to be using
them for anything. The native part doesn't have BuildRequires on
the native equivalents.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The fact that we use an external search provider instead of a
built-in search functionality might come as a surprise to users,
and some of them might not be comfortable with the arrangement.
Mention it in the search interface to avoid surprises.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Ideally we'd just perform the search ourselves, but as long as
we have to rely on an external provider, at least make it a
somewhat privacy-conscious one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Having the name of the project as part of the prefix was useful
back when we used the mailing list for all subprojects, but
these days the only patches that are sent to the list are for
the main library so it no longer makes sense to include this.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 28 Nov 2023 15:06:30 +0000 (16:06 +0100)]
qemu_command: Don't open code virPCIDeviceAddressAsString()
When building a hostdev props, its PCI address is formatted via
g_strdup_printf(VIR_PCI_DEVICE_ADDRESS_FMT, ...); Well, we have a
function that does exactly that: virPCIDeviceAddressAsString().
Use the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
test_driver: Remove unused variable in testDomainAttachDeviceLiveAndConfig()
The hotplug functionality added earlier really supports only live
addition of devices, no coldplug yet rendering @devConf in
testDomainAttachDeviceLiveAndConfig() an unused variable. Remove
it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 30 Nov 2023 18:34:55 +0000 (19:34 +0100)]
rpcgen: Skip generator tests on macOS
The generator can produce different code on Linux and macOS:
specifically, on the former we want to use xdr_uint64_t while
the latter needs xdr_u_int64_t instead.
This is clearly a problem for tests that involve comparing the
output produced against some expected output that's stored in
the git repository.
In the long run, we need to find a better way to handle this,
but since 9.10.0 is going to be released very soon and we don't
want it to have a broken test suite on macOS, simply skip the
generator tests on that platform for now.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
macOS XDR library is an oddball using xdr_u_int64_t instead of
xdr_uint64_t which everyone else has.
The code generator already does the right thing, but the test
program previously generated with the Linux rpcgen program
does not compile on macOS due to this.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Wed, 29 Nov 2023 14:50:30 +0000 (15:50 +0100)]
rpcgen: Don't skip all tests when pytest is missing
Some of the files used by test_demo.c can only be regenerated
when pytest is present, but we have pre-generated copies in the
repository, so overall we just need the C compiler to build and
run that specific test program.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 1 Dec 2023 09:28:48 +0000 (10:28 +0100)]
qemuProcessStartWithMemoryState: Don't start qemu with '-loadvm SNAP' and '-incoming defer' together
A bug in qemuProcessStartWithMemoryState caused that we would start qemu
with '-loadvm SNAP' and '-incoming defer' together. qemu doesn't expect
that and crashes on an assertion failure [1].
[1]: https://issues.redhat.com/browse/RHEL-16782
Fixes: 8a88d3e5860881f430e528d3e5e8d6455ded4d1d
Resolves: https://issues.redhat.com/browse/RHEL-17841 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Wed, 29 Nov 2023 11:30:16 +0000 (12:30 +0100)]
rpcgen: tests: Run cleanly on platforms where char is unsigned
There are some platforms where 'char' is unsigned, by default
(RPi, s390x to name a few). And because of how test_demo is
written we are experiencing some test cases failing there. For
instance: /xdr/struct-scalar is failing. This is because in the
test (test_struct_scalar()), we have a struct with two chars. One
is initialized to 0xca, the other 0xfe (note that both have the
MSB set). The XDR encoder (xdr_TestStructScalar()) then calls
xdr_char() on both of them. But XDR itself has no notion of
char type, so under the hood, it expands it to int [1] and calls
xdr_int(). And this is where the problem lies. On platforms where
char is signed, the integer expansion results in 0xffffffca, but
on platforms where char is unsigned it results in 0x000000ca. Two
distinct results.
The test then goes and compares the encoded buffer with an
expected one (memcmp(), read from the disk earlier).
This poses no problem for real life use, because when decoding
those chars back, the padding is thrown away.
To avoid tickling this issue, use values that don't have the MSB
set.
Fixes: 40cbaa8fbe rpcgen: add test case for XDR serialization Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Michal Privoznik [Wed, 29 Nov 2023 11:22:21 +0000 (12:22 +0100)]
rpcgen: tests: Allow running test_demo from anywhere
The test_demo program compares whether XDR encoded data match the
expected output as read from a file. But the file path is not
absolute and thus relative to CWD which means the program can run
only from one specific directory.
Do what we do in the rest of our test suite: define 'abs_srcdir'
macro and prefix the path with it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Tue, 28 Nov 2023 15:35:22 +0000 (16:35 +0100)]
ch: Don't leak ch_driver->chCaps
During CH driver initialization (chStateInitialize()) the
driver's capabilities bitmap is allocated
(virCHCapsInitCHVersionCaps()), but corresponding free call is
missing in chStateCleanup().
And while at it, reorder calls to virObjectUnref() inside of
chStateCleanup() to be the reverse order of that in
chStateInitialize() so that it's easier to spot missing
free/unref call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Tue, 28 Nov 2023 17:51:04 +0000 (18:51 +0100)]
rpm: Drop rpcgen Build-Requires once again
We no longer use it, so commit a62486b95fee correctly dropped
the Build-Requires; shortly afterwards, however, I accidentally
reintroduced it by mistake.
Fixes: 3df8cc658ed0ac2f84089ad0db61ba20eb8b8aa7 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 14 Nov 2023 16:19:10 +0000 (17:19 +0100)]
qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource: Don't setup 'raw' layer for migration NBD connection
The raw driver layer is not needed in this case and can be dropped.
Removing the nodename will cause other pieces of the code to pick up and
stop adding the layer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 14 Nov 2023 16:07:44 +0000 (17:07 +0100)]
qemuMigrationSrcNBDCopyCancel: Use qemuBlockStorageSourceAttachRollback to detach migration NBD blockdevs
Rewrite the code to use the common tooling for removing blockdevs
instead of the ad-hoc qemuBlockStorageSourceDetachOneBlockdev helper.
Use of the common infrastructure will properly handle cases when the raw
driver is ommited from the block graph.
Since the TLS data object is shared for all migration QMP commands and
objects we need to strip its alias from the definition of the storage
source before attempting to detach it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / libvirt.git / log