[PATCH] virauth: Report error on empty auth result
by Michal Privoznik
When opening a connection, it may be necessary to provide user
credentials, or some additional info (e.g. whether to trust an
ssh key). We have a special API for that: virConnectOpenAuth()
where and additional callback can be passed. This callback is
then called with _virConnectCredential struct filled partially
and it's callback's responsibility to get desired data (e.g. by
prompting user) and store it into .result member of the struct.
But we document the callback behaviour as:
If an interaction cannot be filled, fill in NULL and 0.
(meaning fill in NULL and return 0)
But this does not really fly with the way callback is called. So
far, we have three places where the callback is called:
1) remoteAuthInteract()
2) virAuthGetUsernamePath()
3) virAuthAskCredential()
Now, 1) just grabs whatever the client side provided and sends it
to the other side via RPC. All interesting work takes place at 2)
and 3). And the usual pattern in which these two are called is:
if (!(cred = wrapper()))
return -1;
where wrapper() is a function that firstly tries to get data from
a config file or URI itself. The wrappers are named
virAuthGetUsername() for virAuthGetUsernamePath() and
virAuthGetPassword() for virAuthAskCredential().
All wrappers do report error on failure. Except, when the user
provided callback set the struct member to NULL. Then they both
return NULL without setting any error. This then leads to the
generic error message:
error : An error occurred, but the cause is unknown
Since we failed to get desired credential data, we can't proceed
(what data should we pass down to the auth layer, say ssh?) and
have to fail. But, we can produce an error message that'll
hopefully point users in the right direction.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181235
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virauth.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index 7b4a1bd8a5..d1f32f8e6d 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -176,7 +176,8 @@ virAuthGetUsernamePath(const char *path,
cred.result = NULL;
cred.resultlen = 0;
- if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0) {
+ if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0 ||
+ !cred.result) {
virReportError(VIR_ERR_AUTH_FAILED, "%s",
_("Username request failed"));
VIR_FREE(cred.result);
@@ -310,7 +311,8 @@ virAuthAskCredential(virConnectAuthPtr auth,
ret->prompt = prompt;
- if (auth->cb(ret, 1, auth->cbdata) < 0) {
+ if (auth->cb(ret, 1, auth->cbdata) < 0 ||
+ !ret->result) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("failed to retrieve user response for authentication callback"));
return NULL;
--
2.39.2
2 years
[RFC] Seeking advice on support for generic vDPA device
by jiangdongxu
vDPA devices allow high-performance devices in a virtual machine by
providing a wire-speed data path. These devices require a vendor-specific host
driver but the data path follows the virtio specification.
The support for generic-vdpa device was recently added to qemu in v8.0.0-rc version.
https://gitlab.com/qemu-project/qemu/-/commit/b430a2bd2303b9940cc80ec7468...
This allows libvirt to support these deviecs regardless of device type.
Currently, libvirt supports vDPA network devices, We need a new solution
to support generic-vdpa devices.
At present, we have considered several options.
Solution A:
Add type vdpa under the hostdev device，the device xml looks like
<devices>
<hostdev mode='subsystem' type='vdpa'>
<source dev='/dev/vhost-vdpa-0'/>
</hostdev>
</devices>
This solution is recommended. This configuration requires only that the
device supports the vDPA framework, easy for future expansion and same
as the qemu solution design idea.
Solution B:
The current libvirt mode is used to add vDPA devices under different devices.
device xml looks like:
network:
<devices>
<interface type='generic-vdpa'>
<source dev='/dev/vhost-vdpa-0'/>
</interface>
</devices>
storage:
<devices>
<disk type='generic-vdpa’>
<source dev='/dev/vhost-vdpa-0'/>
</disk>
</devices>
The network interface has the vdpa type. Therefore, a new type is required to
distinguish the "generic-vdpa" type. However, this method differentiates
device types, which is not convenient for the expansion of other types
of vDPA devices. Not recommended.
Solution C:
Referring to PCI passthrough, Add the driver type (appropriate vendor-specific driver) to the driver of the PCI device.
The unbind/bind of the vDPA device driver and the creation of VDPA device are implemented by libvirt.
device xml looks like:
<devices>
<hostdev mode='subsystem' type='pci' managed='yes'>
<driver name='vp-vdpa'/>
<source>
<address domain='0x0000' bus='0x04' slot='0x01' function='0x01'/>
</source>
</hostdev>
</devices>
This solution is related to the vendor-specific driver and supports only the vdpa created by the PCI device.
vdpa devices created in vduse mode are not supported. This solution is not universal and is not recommended.
What's your proposal?
2 years
Possible Bug in Libvirt::StorageVol in ruby-libvirt?
by dateirunner
Dear Support Team
I want to kindly ask for help
I am running almalinux with the packages
libvirt
libvirt-devel
I installed the gem 'libvirt-ruby'
When running the following script the execution breaks with:
./libvirt-test.rb:25:in `name': Expected Connection object (ArgumentError)
from ./libvirt-test.rb:25:in `<main>'
#!/usr/bin/env ruby
require 'bundler/inline'
gemfile do
source 'https://rubygems.org'
gem 'ruby-libvirt'
end
require 'libvirt'
#Starts connection #works
conn = Libvirt::open()
# Lists all pools #works
pools = conn.list_all_storage_pools
# Saves all volumes of all pools #
volumes = pools.map {|p| p.list_all_volumes}.flatten
# Gets first volume
first_volume = volumes.first
puts volumes.first
#Breaks Should return the name of the volume as string
first_volume.name
#Breaks Should return the path of the volume as string
first_volume.path
--------------
Can someone confirm this issue?
Is there a possible fix or did I misread the documentation?
Thank you for your help
Regards
R. Koch
2 years
[PATCH v1 0/4] Introduce virsh hypervisor-cpu-models
by Collin Walling
Allows for the query of hypervisor-known CPU models via the simple
command: virsh hypervisor-cpu-models. For the QEMU driver, the models
are queried via the capabilities file. Each model is printed to the
terminal on its own line similar to the cpu-models command, and there
is no order to the listing.
The models "qemu", "host", and "max" have been excluded from this list
since they are not architecture specific. The code can be easily modified
to include them if desired.
Collin Walling (4):
Introduce virConnectGetHypervisorCPUNames public API
remote: Implement virConnectGetHypervisorCPUNames
virsh: Introduce new hypervisor-cpu-models command
qemu_driver: Implement qemuConnectGetHypervisorCPUNames
docs/manpages/virsh.rst | 20 ++++++++++
include/libvirt/libvirt-host.h | 6 +++
src/driver-hypervisor.h | 9 +++++
src/libvirt-host.c | 54 ++++++++++++++++++++++++++
src/libvirt_public.syms | 5 +++
src/qemu/qemu_driver.c | 62 ++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 20 +++++++++-
src/remote_protocol-structs | 11 ++++++
tools/virsh-host.c | 70 ++++++++++++++++++++++++++++++++++
10 files changed, 257 insertions(+), 1 deletion(-)
--
2.39.0
2 years
[libvirt PATCH] NEWS: document my user-visible bugfixes
by Ján Tomko
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
NEWS.rst | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst
index c187d35d3b..cc32fb948e 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -60,6 +60,17 @@ v9.2.0 (unreleased)
so, as affinity to NUMA nodes inaccessible to emulator thread might have
been requested.
+ * rpc: fix typo in admin code generation
+
+ Fix the bug in the remote `virt-admin` code generator, that resulted
+ in a crash. Introduced in libvirt 9.1.0.
+
+ * qemu: relax shared memory check for vhostuser daemons
+
+ Fix hotplug of virtiofs `filesystem` after restarting libvirtd.
+ Before, libvirtd would incorrectly complain about missing shared
+ memory.
+
v9.1.0 (2023-03-01)
===================
--
2.39.2
2 years
[libvirt PATCH] qemu_snapshot: external: don't error out when updating metadata
by Pavel Hrdina
Attaching disk into running VM the offilne definition may not be
updated and we will end up with that disk existing only in live
definition. Creating snapshot with this state saves both live and
offline definition into snapshot metadata.
When we are deleting an external snapshot we are updating these
definitions in the snapshot metadata so we should just skip over
non-existing disks instead of reporting error.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2174700
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 14353c6f0d..e36d6288ac 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2464,8 +2464,8 @@ qemuSnapshotUpdateDisksSingle(virDomainMomentObj *snap,
{
virDomainDiskDef *disk = NULL;
- if (!(disk = qemuDomainDiskByName(def, snapDisk->name)))
- return -1;
+ if (!(disk = virDomainDiskByName(def, snapDisk->name, true)))
+ return 0;
if (virDomainSnapshotIsExternal(snap)) {
virDomainDiskDef *parentDisk = NULL;
--
2.39.2
2 years
libvirt-shim: libvirt to run qemu in a different container
by Itamar Holder
Hey all,
I'm Itamar Holder, a Kubevirt developer.
Lately we came across a problem w.r.t. properly supporting VMs with
dedicated CPUs on Kubernetes. The full details can be seen in this PR
<https://github.com/kubevirt/kubevirt/pull/8869> [1], but to make a very
long story short, we would like to use two different containers in the
virt-launcher pod that is responsible to run a VM:
- "Managerial container": would be allocated with a shared cpuset. Would
run all of the virtualization infrastructure, such as libvirtd and its
dependencies.
- "Emulator container": would be allocated with a dedicated cpuset.
Would run the qemu process.
There are many reasons for choosing this design, but in short, the main
reasons are that it's impossible to allocate both shared and dedicated cpus
to a single container, and that it would allow finer-grained control and
isolation for the different containers.
Since there is no way to start the qemu process in a different container, I
tried to start qemu in the "managerial" container, then move it into the
"emulator" container. This fails however, since libvirt uses
sched_setaffinity to pin the vcpus into the dedicated cpuset, which is not
allocated to the managerial container, resulting in an EINVAL error.
Therefore, I thought about discussing a new approach - introducing a small
shim that could communicate with libvirtd in order to start and control the
qemu process that would run on a different container.
As I see it, the main workflow could be described as follows:
- The emulator container would start with the shim.
- libvirtd, running in the managerial container, would ask for some
information from the target, e.g. cpuset.
- libvirtd would create the domain xml and would transfer to the shim
everything needed in order to launch the guest.
- The shim, running in the emulator container, would run the
qemu-process.
What do you think? Feedback is much appreciated.
Best Regards,
Itamar Holder.
[1] https://github.com/kubevirt/kubevirt/pull/8869
2 years
Entering freeze for libvirt-9.2.0
by Jiri Denemark
I have just tagged v9.2.0-rc1 in the repository and pushed signed
tarballs and source RPMs to https://libvirt.org/sources/
Please give the release candidate some testing and in case you find a
serious issue which should have a fix in the upcoming release, feel
free to reply to this thread to make sure the issue is more visible.
If you have not done so yet, please update NEWS.rst to document any
significant change you made since the last release.
Thanks,
Jirka
2 years
[PATCH] Revert "ci: Disable optimization on macos-12"
by Martin Kletzander
This reverts commit 1f76b5365ec78b1e9a36038db8e13ec0025bbe7a.
There were two issues with this commit. First is the missing propagation
of CFLAGS into the build environment and second is the fact that this is
not enough to disable the check for -fsemantic-interposition. The
proper fix would require setting MESON_OPTS or similar and also add the
propagation of such variable into the cirrus builds etc., but at this
point I burned so much time on this trivial piece of rubbish that I
think it's easier to just wait for macos to gain a newer clang =D
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
ci/gitlab/builds.yml | 1 -
ci/manifest.yml | 2 --
2 files changed, 3 deletions(-)
diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml
index f070db637c80..545478d3516d 100644
--- a/ci/gitlab/builds.yml
+++ b/ci/gitlab/builds.yml
@@ -865,7 +865,6 @@ aarch64-macos-12:
needs: []
allow_failure: false
variables:
- CFLAGS: -O0
CIRRUS_VM_IMAGE_NAME: ghcr.io/cirruslabs/macos-monterey-base:latest
CIRRUS_VM_IMAGE_SELECTOR: image
CIRRUS_VM_INSTANCE_TYPE: macos_instance
diff --git a/ci/manifest.yml b/ci/manifest.yml
index 2f631376b24b..b68c7361abb4 100644
--- a/ci/manifest.yml
+++ b/ci/manifest.yml
@@ -179,8 +179,6 @@ targets:
variables:
PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin:/usr/local/opt/libpcap/bin:/usr/local/opt/libxslt/bin:/usr/local/opt/rpcgen/bin
PKG_CONFIG_PATH: /usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/libpcap/lib/pkgconfig:/usr/local/opt/libxml2/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
- # Remove once macos has clang with -fsemantic-interposition
- CFLAGS: -O0
ubuntu-2004:
jobs:
--
2.40.0
2 years
[PATCH] virt-host-validate: Detect SMMU support on ARMs
by Michal Privoznik
In vir-host-validate we do two checks related to IOMMU:
1) hardware support, and
2) kernel support.
While users are usually interested in the latter, the former also
makes sense. And for the former (hardware support) we have this
huge if-else block for nearly every architecture, except ARM.
Now, IOMMU is called SMMU in ARM world, and while there's
certainly a definitive way of detecting SMMU support (e.g. via
dumping some registers in asm), we can work around this - just
like we do for Intel and AMD - and check for an ACPI table
presence.
In ARM world, there's I/O Remapping Table (IORT) which describes
SMMU capabilities on given host and is exposed in sysfs
(regardless of arm_smmu module).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2178885
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virt-host-validate-common.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index a41bb346d2..49d3c4083b 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -388,6 +388,15 @@ int virHostValidateIOMMU(const char *hvname,
return VIR_HOST_VALIDATE_FAILURE(VIR_HOST_VALIDATE_NOTE);
}
virHostMsgPass();
+ } else if (ARCH_IS_ARM(arch)) {
+ if (access("/sys/firmware/acpi/tables/IORT", F_OK) == 0) {
+ virHostMsgPass();
+ } else {
+ virHostMsgFail(level,
+ "No ACPI IORT table found, IOMMU not "
+ "supported by this hardware platform");
+ return VIR_HOST_VALIDATE_FAILURE(level);
+ }
} else {
virHostMsgFail(level,
"Unknown if this platform has IOMMU support");
--
2.39.2
2 years
