[libvirt PATCH] qemu: snapshot: error out early when reverting snapshot for VM with non-file disk
by Pavel Hrdina
Before this patch the code would start the revert process by destroying
the VM and preparing to revert where it would fail with following error:
error: unsupported configuration: source for disk 'sdb' is not a regular file; refusing to generate external snapshot name
and leaving user with offline VM even if it was running.
Make the check before we start the revert process to not destroy VMs.
Resolves: https://issues.redhat.com/browse/RHEL-30971
Resolves: https://issues.redhat.com/browse/RHEL-79928
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index f7d6272907..c5f70f5b10 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2205,6 +2205,8 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
virDomainSnapshotDef *snapdef,
unsigned int flags)
{
+ size_t i;
+
if (!vm->persistent &&
snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING &&
snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED &&
@@ -2232,6 +2234,17 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
}
}
+ for (i = 0; i < snap->def->dom->ndisks; i++) {
+ virDomainDiskDef *disk = snap->def->dom->disks[i];
+
+ if (disk->src->type != VIR_STORAGE_TYPE_FILE) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("source disk for '%1$s' is not a regural file, reverting to snapshot is not supported"),
+ disk->dst);
+ return -1;
+ }
+ }
+
return 0;
}
--
2.48.1
1 week, 6 days
Entering freeze for libvirt-11.1.0
by Jiri Denemark
I have just tagged v11.1.0-rc1 in the repository and pushed signed
tarballs to https://download.libvirt.org/
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
1 week, 6 days
[PATCH v3 0/1] add RAPL feature in libvirt
by Anthony Harivel
Hi,
Since Fedora 41, RAPL feature in QEMU is available.
The tested version with this patch is: qemu-9.1.2-2.fc41.
Incorporating this feature into libvirt would significantly streamline
the process of testing, adopting, and further developing it, thereby
enhancing overall productivity and user experience.
Thank you for your consideration.
Anthony
Anthony Harivel (1):
qemu: Add support for RAPL MSRs feature
docs/formatdomain.rst | 2 ++
src/conf/domain_conf.c | 18 ++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/conf/schemas/domaincommon.rng | 10 ++++++++++
src/qemu/qemu_command.c | 12 ++++++++++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 47 insertions(+), 1 deletion(-)
--
2.48.1
1 week, 6 days
[RFC 0/4] meson: Enable -Wundef
by Andrea Bolognani
A few days ago I have posted a patch[1] that addresses an issue
introduced when a meson check was dropped but some uses of the
corresponding WITH_ macro were not removed at the same time.
That got me thinking about what we can do to prevent such scenarios
from happening again in the future. I have come up with something
that I think would be effective, but since applying the approach
throughout the entire codebase would require a non-trivial amount of
work, I figured I'd ask for feedback before embarking on it.
The idea is that there are two types of macros we can use for
conditional compilation: external ones, coming from the OS or other
libraries, and internal ones, which are the result of meson tests.
The external ones (e.g. SIOCSIFFLAGS, __APPLE__) are usually only
defined if they apply, so it is correct to check for their presence
with #ifdef. Using #if will also work, as undefined macros evaluate
to zero, but it's not good practice to use them that way. If -Wundef
has been passed to the compiler, those incorrect uses will be
reported (only on platforms where they are not defined, of course).
The internal ones (e.g. WITH_QEMU, WITH_STRUCT_IFREQ) are similar,
but in this case we control their definition. This means that using
means that the feature is not available on the machine we're building
on, but it could also mean that we've removed the meson check and
forgot to update all users of the macro. In this case, -Wundef would
work 100% reliably to detect the issue: if the meson check doesn't
exist, neither will the macro, regardless of what platform we're
building on.
So the approach I'm suggesting is to use a syntax-check rule to
ensure that internal macros are only ever checked with #if instead of
Of course this requires a full sweep to fix all cases in which we're
not already doing things according to the proposal. Should be fairly
easy, if annoying. A couple of examples are included here for
demonstration purposes.
The bigger impact is going to be on the build system. Right now we
generally only define WITH_ macros if the check passed, but that will
have to change and the result is going to be quite a bit of
additional meson code I'm afraid.
Thoughts?
[1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/S...
Andrea Bolognani (4):
configmake: Check for WIN32 correctly
meson: Always define WITH_*_DECL macros
syntax-check: Ensure WITH_ macros are used correctly
meson: Enable -Wundef
build-aux/syntax-check.mk | 5 +++++
configmake.h.in | 2 +-
meson.build | 3 +++
tests/virmockstathelpers.c | 28 ++++++++++++++--------------
4 files changed, 23 insertions(+), 15 deletions(-)
--
2.43.2
1 week, 6 days
[PATCH 00/10] qemu: Refactor typed parameter handling in qemuDomainGetGuestInfo
by Peter Krempa
Use virTypedParam list to construct the returned data.
Note that this applies on top of two patch(sets) that were ACK'd
upstream:
[PATCH v2 0/4] Add support for getting load averages from guest agent
[PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo
You can fetch the whole thing via:
git fetch https://gitlab.com/pipo.sk/libvirt.git agent-data-refactor
Peter Krempa (10):
qemuDomainGetGuestInfo: Prepare for refactor to virTypedParamList
qemuDomainGetGuestInfo: Convert load code to virTypedParamList
virDomainInterfaceFormatParams: Convert interface code to
virTypedParamList
qemuAgentDiskInfoFormatParams: Convert interface code to
virTypedParamList
qemuAgentFSInfoFormatParams: Convert interface code to
virTypedParamList
qemuAgentGetTimezone: Convert to virTypedParamList
qemuAgentGetOSInfo: Convert to virTypedParamList
qemuAgentGetUsers: Convert to virTypedParamList
qemuDomainGetGuestInfo: Convert hostname code to virTypedParamList
qemuDomainGetGuestInfo: Remove temporary infrastructure
src/qemu/qemu_agent.c | 52 ++-------
src/qemu/qemu_agent.h | 12 +-
src/qemu/qemu_driver.c | 249 ++++++++++++-----------------------------
tests/qemuagenttest.c | 148 +++++++++++-------------
4 files changed, 152 insertions(+), 309 deletions(-)
--
2.48.1
2 weeks
[PATCH 00/17] conf/qemu/etc: change several functions to return void instead of 0/-1
by Laine Stump
This started with me noticing one function call that checked for
failure on something that I knew couldn't fail. So I changed that
function to return void. But then I noticed another similar function
that also should return void, so I changed that one too. Then
eliminating the check for the return from those functions caused
another function to become infallible, so I changed that one too,
which led to more and more until the evening was finished and I had
this long list of tiny mechanical patches. And here it is - an easy
way to improve your review stats :-)
P.S. I know there are more of these, but forced myself to stop here.
A related question - is it possible for virObjectNew() to fail? I did
finally find (after some searching, documentation that says
g_object_new() can't return null, but I don't know enough about
g_object stuff to know if the vir*Initialize functions could fail (for
example). If virObjectNew() can't fail then that open a whole new can
of worms...
Laine Stump (17):
conf: change virDomainHostdevInsert() to return void
conf: change virDomainNetInsert() to return void
conf: change virDomainFSInsert() to return void
conf: change virDomainShmemDefInsert() to return void
conf: change virDomainDefMaybeAddInput() to return void
libxl: change xenDomainDefAddImplicitInputDevice() to return void
conf: change qemuDomainDefAddImplicitInputDevice() to return void
conf: stop checking for NULL return from virDomainControllerDefNew()
conf: stop checking for NULL return from virDomainDefAddController()
conf: change virDomainDefAddUSBController() to return void
hyperv: change hypervDomainDefAppendController() to return void
conf: change virDomainDefMaybeAddController() to return true/false
conf: change virDomainDefMaybeAddHostdevSCSIcontroller() to return
void
conf: change virDomainDefAddDiskControllersForType() to return void
conf: change virDomainDefMaybeAddVirtioSerialController() to return
void
conf: change virDomainDefMaybeAddSmartcardController() to return void
conf: change virDomainDefAddImplicitControllers() to return void
src/bhyve/bhyve_domain.c | 13 ++-
src/conf/domain_addr.c | 6 +-
src/conf/domain_conf.c | 174 ++++++++++-----------------------
src/conf/domain_conf.h | 16 +--
src/hyperv/hyperv_driver.c | 28 ++----
src/libxl/libxl_conf.c | 4 +-
src/libxl/libxl_domain.c | 11 +--
src/libxl/libxl_driver.c | 11 +--
src/libxl/xen_common.c | 15 +--
src/libxl/xen_common.h | 2 +-
src/libxl/xen_xl.c | 4 +-
src/lxc/lxc_driver.c | 6 +-
src/qemu/qemu_domain_address.c | 8 +-
src/qemu/qemu_driver.c | 14 +--
src/qemu/qemu_postparse.c | 58 ++++-------
src/qemu/qemu_process.c | 3 +-
src/vbox/vbox_common.c | 13 +--
src/vmx/vmx.c | 12 +--
src/vz/vz_driver.c | 11 +--
src/vz/vz_sdk.c | 14 +--
20 files changed, 126 insertions(+), 297 deletions(-)
--
2.47.1
2 weeks
[PATCH 0/2] cpu_map: arm: Add AmpereOne CPU models
by Jiri Denemark
Jiri Denemark (2):
cpu_arm: Report vendor ID for unknown PVRs
cpu_map: arm: Add AmpereOne CPU models
src/cpu/cpu_arm.c | 6 ++++--
src/cpu_map/arm_Ampere-1.xml | 6 ++++++
src/cpu_map/arm_Ampere-1a.xml | 6 ++++++
src/cpu_map/arm_vendors.xml | 1 +
src/cpu_map/index.xml | 5 +++++
5 files changed, 22 insertions(+), 2 deletions(-)
create mode 100644 src/cpu_map/arm_Ampere-1.xml
create mode 100644 src/cpu_map/arm_Ampere-1a.xml
--
2.48.1
2 weeks
[PATCH 1/2] virt-aa-helper: Avoid using RUNSTATEDIR
by Michal Privoznik
On some systems /run is mounted as:
tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=348508k,mode=755,inode64)
and /var/run is then just a symlink:
# ls -ld /var/run
lrwxrwxrwx 1 root root 4 Apr 23 2024 /var/run -> /run
But because we still think it's 2004 and FHS 2.3 is active we
have a rule in our meson.build which constructs RUNSTATEDIR as
the following:
runstatedir = get_option('runstatedir')
if runstatedir == ''
runstatedir = localstatedir / 'run'
endif
which (if unspecified on meson setup line) results in "/var/run".
This in turn means, when when we're generating an AppArmor
profile for a domain with allowed paths it contains stuff like:
/var/run/libvirt/qemu/swtpm/2-guest-swtpm.sock
But because of the aforementioned symlink the real path is:
/run/libvirt/qemu/swtpm/2-guest-swtpm.sock
and thus AppArmor denies access:
audit: type=1400 audit(1740480419.348:415): apparmor="DENIED" operation="connect" class="file" profile="libvirt-126f2720-6f8e-45ab-a886-ec9277079a67" name="/run/libvirt/qemu/swtpm/2-guest-swtpm.sock" pid=8080 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=64055
Fortunately, there's a nice trick: AppArmor profile variables. We
already use some of them (@{PROC}, @{HOME}, @{multiarch}) and
instead of RUNSTATEDIR we can use @{run} which is declared as:
# cat /etc/apparmor.d/tunables/run
@{run}=/run/ /var/run/
and thus covers both scenarios.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/virt-aa-helper.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c255b64f35..eabc29e9cf 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1238,8 +1238,8 @@ get_files(vahControl * ctl)
/* Unix socket for QEMU and swtpm to use */
virBufferAsprintf(&buf,
- " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n",
- RUNSTATEDIR, shortName);
+ " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n",
+ shortName);
/* Paths for swtpm to use: give it access to its state
* directory (state files and fsync on dir), log, and PID files.
*/
@@ -1253,8 +1253,8 @@ get_files(vahControl * ctl)
" \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n",
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf,
- " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
- RUNSTATEDIR, shortName);
+ " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
+ shortName);
VIR_FREE(shortName);
}
@@ -1528,10 +1528,10 @@ main(int argc, char **argv)
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf, " \"%s/lib/libvirt/qemu/domain-%d-%.*s/*\" rw,\n",
LOCALSTATEDIR, ctl->def->id, 20, ctl->def->name);
- virBufferAsprintf(&buf, " \"%s/libvirt/**/%s.pid\" rwk,\n",
- RUNSTATEDIR, ctl->def->name);
- virBufferAsprintf(&buf, " \"%s/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n",
- RUNSTATEDIR, ctl->def->name);
+ virBufferAsprintf(&buf, " \"@{run}/libvirt/**/%s.pid\" rwk,\n",
+ ctl->def->name);
+ virBufferAsprintf(&buf, " \"@{run}/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n",
+ ctl->def->name);
}
if (ctl->files)
virBufferAdd(&buf, ctl->files, -1);
--
2.45.3
2 weeks
[PATCH] virt-aa-helper: Avoid using RUNSTATEDIR
by Michal Privoznik
On some systems /run is mounted as:
tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=348508k,mode=755,inode64)
and /var/run is then just a symlink:
# ls -ld /var/run
lrwxrwxrwx 1 root root 4 Apr 23 2024 /var/run -> /run
But because we still think it's 2004 and FHS 2.3 is active we
have a rule in our meson.build which constructs RUNSTATEDIR as
the following:
runstatedir = get_option('runstatedir')
if runstatedir == ''
runstatedir = localstatedir / 'run'
endif
which (if unspecified on meson setup line) results in "/var/run".
This in turn means, when when we're generating an AppArmor
profile for a domain with allowed paths it contains stuff like:
/var/run/libvirt/qemu/swtpm/2-guest-swtpm.sock
But because of the aforementioned symlink the real path is:
/run/libvirt/qemu/swtpm/2-guest-swtpm.sock
and thus AppArmor denies access:
audit: type=1400 audit(1740480419.348:415): apparmor="DENIED" operation="connect" class="file" profile="libvirt-126f2720-6f8e-45ab-a886-ec9277079a67" name="/run/libvirt/qemu/swtpm/2-guest-swtpm.sock" pid=8080 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=64055
Fortunately, there's a nice trick: AppArmor profile variables. We
already use some of them (@{PROC}, @{HOME}, @{multiarch}) and
instead of RUNSTATEDIR we can use @{run} which is declared as:
# cat /etc/apparmor.d/tunables/run
@{run}=/run/ /var/run/
and thus covers both scenarios.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/virt-aa-helper.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c255b64f35..eabc29e9cf 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1238,8 +1238,8 @@ get_files(vahControl * ctl)
/* Unix socket for QEMU and swtpm to use */
virBufferAsprintf(&buf,
- " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n",
- RUNSTATEDIR, shortName);
+ " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n",
+ shortName);
/* Paths for swtpm to use: give it access to its state
* directory (state files and fsync on dir), log, and PID files.
*/
@@ -1253,8 +1253,8 @@ get_files(vahControl * ctl)
" \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n",
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf,
- " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
- RUNSTATEDIR, shortName);
+ " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
+ shortName);
VIR_FREE(shortName);
}
@@ -1528,10 +1528,10 @@ main(int argc, char **argv)
LOCALSTATEDIR, ctl->def->name);
virBufferAsprintf(&buf, " \"%s/lib/libvirt/qemu/domain-%d-%.*s/*\" rw,\n",
LOCALSTATEDIR, ctl->def->id, 20, ctl->def->name);
- virBufferAsprintf(&buf, " \"%s/libvirt/**/%s.pid\" rwk,\n",
- RUNSTATEDIR, ctl->def->name);
- virBufferAsprintf(&buf, " \"%s/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n",
- RUNSTATEDIR, ctl->def->name);
+ virBufferAsprintf(&buf, " \"@{run}/libvirt/**/%s.pid\" rwk,\n",
+ ctl->def->name);
+ virBufferAsprintf(&buf, " \"@{run}/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n",
+ ctl->def->name);
}
if (ctl->files)
virBufferAdd(&buf, ctl->files, -1);
--
2.45.3
2 weeks
[PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo
by Peter Krempa
KubeVirt decided to report this to the users. In order to allow them to
use proper APIs expose the field as well.
Resolves: https://issues.redhat.com/browse/RHEL-80688
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 1 +
src/libvirt-domain.c | 1 +
src/qemu/qemu_driver.c | 9 +++++++++
3 files changed, 11 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index f159c40631..baced15dec 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2999,6 +2999,7 @@ returned:
* ``disk.<num>.serial`` - optional disk serial number
* ``disk.<num>.alias`` - the device alias of the disk (e.g. sda)
* ``disk.<num>.guest_alias`` - optional alias assigned to the disk
+* ``disk.<num>.guest_bus`` - bus type as reported by the guest
*--interface* returns:
* ``if.count`` - the number of interfaces defined on this domain
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ae15ab1e5a..05101756a1 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13273,6 +13273,7 @@ virDomainSetVcpu(virDomainPtr domain,
* "disk.<num>.alias" - the device alias of the disk (e.g. sda)
* "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux
* this is a name assigned by device mapper
+ * "disk.<num>.guest_bus" - disk bus as reported by the guest OS
*
* VIR_DOMAIN_GUEST_INFO_HOSTNAME:
* Returns information about the hostname of the domain. The typed
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3b8b2c0a1e..86b945d9b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19278,6 +19278,15 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info,
param_name, diskdef->dst) < 0)
return;
}
+
+ if (address->bus_type) {
+ g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "disk.%zu.guest_bus", i);
+
+ if (virTypedParamsAddString(params, nparams, maxparams,
+ param_name, address->bus_type) < 0)
+ return;
+ }
}
if (info[i]->alias) {
--
2.48.1
2 weeks
