[PATCH] nwfilter_driver: Make reload fail if racing with stateCleanup()
by Michal Privoznik
When one thread is trying to reload NWFilter driver (by running
nwfilterStateReload()) but there's another thread that's
concurrently running nwfilterStateCleanup() a crash may occur.
This is despite nwfilterStateReload() checking for driver !=
NULL, because is done so without @driverMutex held. A typical
TOCTOU. Fortunately, the mutex is always initialized, so the
mutex can be acquired at all times and @driver can be checked
with the lock held.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2075837
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/nwfilter/nwfilter_driver.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index b66ba22737..d028efafbe 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -309,6 +309,8 @@ nwfilterStateInitialize(bool privileged,
static int
nwfilterStateReload(void)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driverMutex);
+
if (!driver)
return -1;
@@ -319,15 +321,13 @@ nwfilterStateReload(void)
/* shut down all threads -- they will be restarted if necessary */
virNWFilterLearnThreadsTerminate(true);
- VIR_WITH_MUTEX_LOCK_GUARD(&driverMutex) {
- VIR_WITH_MUTEX_LOCK_GUARD(&driver->updateLock) {
- virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir);
- }
-
-
- virNWFilterBuildAll(driver, false);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->updateLock) {
+ virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir);
}
+
+ virNWFilterBuildAll(driver, false);
+
return 0;
}
--
2.35.1
2 years, 7 months
[libvirt PATCH 0/2] include: Fix vertical spacing
by Andrea Bolognani
Andrea Bolognani (2):
include: Fix vertical spacing inside comments
include: Fix vertical spacing between comments and symbols
include/libvirt/libvirt-common.h.in | 1 -
include/libvirt/libvirt-domain-checkpoint.h | 6 -
include/libvirt/libvirt-domain-snapshot.h | 7 -
include/libvirt/libvirt-domain.h | 154 --------------------
include/libvirt/libvirt-event.h | 1 -
include/libvirt/libvirt-host.h | 33 -----
include/libvirt/libvirt-interface.h | 5 -
include/libvirt/libvirt-network.h | 17 ---
include/libvirt/libvirt-nodedev.h | 7 -
include/libvirt/libvirt-nwfilter.h | 6 -
include/libvirt/libvirt-secret.h | 7 -
include/libvirt/libvirt-storage.h | 25 ----
include/libvirt/libvirt-stream.h | 3 -
include/libvirt/virterror.h | 6 -
14 files changed, 278 deletions(-)
--
2.35.1
2 years, 7 months
[libvirt] [PATCH RESEND v2 0/4] re-introduce <acpi-hotplug-bridge>
by Ani Sinha
Change log:
v2: rebased the patchset. Laine's response is appended at the end.
I am re-introducing the patchset for <acpi-hotplug-bridge> which got
reverted here few months back:
https://www.spinics.net/linux/fedora/libvir/msg224089.html
The reason for the reversal was that there seemed to be some
instability/issues around the use of the qemu commandline which this
patchset tries to support. In particular, some guest operating systems
did not like the way QEMU was trying to disable native hotplug on pcie
root ports. Subsequently, in QEMU 6.2, we have changed our mechanism
using which we disable native hotplug. As I understand, we do not have
any reported issues so far in 6.2 around this area. QEMU will enter a
soft feature freeze in the first week of march in prep for 7.0 release.
Libvirt is also entering a new release cycle phaze. Hence, I am
introducing this patchset early enough in the release cycles so that if
we do see any issues on the qemu side during the rc0, rc1 cycles and if
reversal of this patchset is again required, it can be done in time
before the next libvirt release end of March.
All the patches in this series had been previously reviewed. Some
subsequent fixes were made after my initial patches were pushed. I have
squashed all those fixes and consolidated them into four patches. I have
also updated the documentation to reflect the new changes from the QEMU
side and rebased my changes fixing the tests in the process.
What changed in QEMU post version 6.1 ?
=========================================
We have made basically two major changes in QEMU. First is this change:
(1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8
Author: Julia Suvorova <jusual(a)redhat.com>
Date: Fri Nov 12 06:08:56 2021 -0500
hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
There are two ways to enable ACPI PCI Hot-plug:
* Disable the Hot-plug Capable bit on PCIe slots.
This was the first approach which led to regression [1-2], as
I/O space for a port is allocated only when it is hot-pluggable,
which is determined by HPC bit.
* Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
method.
This removes the (future) ability of hot-plugging switches with PCIe
Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
bridges. If the user wants to explicitely use this feature, they can
disable ACPI PCI Hot-plug with:
--global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
instead of PCIe Native.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual(a)redhat.com>
Signed-off-by: Igor Mammedov <imammedo(a)redhat.com>
Message-Id: <20211112110857.3116853-5-imammedo(a)redhat.com>
Reviewed-by: Ani Sinha <ani(a)anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst(a)redhat.com>
Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com>
The patch description says it all. Instead of masking out the HPC bit in
pcie slots, we keep them turned on. Instead, we do not advertize native
hotplug capability for PCIE using _OSC control method. See section
6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for
these slots so now the guest OS can select ACPI hotplug instead.
The second change is introduction of a property with which we keep the
existing behavior for pc-q35-6.1 machines. This means HPC bit is masked
and ACPI hotplug is enabled by default for pcie root ports.
The QEMU commit is:
(2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb
Author: Julia Suvorova <jusual(a)redhat.com>
Date: Fri Nov 12 06:08:54 2021 -0500
hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type
To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be
turned on, while the switch to ACPI Hot-plug will be done in the
DSDT table.
Introducing 'x-keep-native-hpc' property disables the HPC bit only
in 6.1 and as a result keeps the forced 'reserve-io' on
pcie-root-ports in 6.1 too.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual(a)redhat.com>
Signed-off-by: Igor Mammedov <imammedo(a)redhat.com>
Message-Id: <20211112110857.3116853-3-imammedo(a)redhat.com>
Reviewed-by: Michael S. Tsirkin <mst(a)redhat.com>
Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com>
Lastly, as a related side note, because from QEMU 6.2 onwards, we do not
mask out HPC bit in PCIE, the work done by this patch is no longer
needed:
(3) commit e2a6290aab578b2170c1f5909fa556385dc0d820
Author: Marcel Apfelbaum <marcel.apfelbaum(a)gmail.com>
Date: Mon Aug 2 12:00:57 2021 +0300
hw/pcie-root-port: Fix hotplug for PCI devices requiring IO
Q35 has now ACPI hotplug enabled by default for PCI(e) devices.
As opposed to native PCIe hotplug, guests like Fedora 34
will not assign IO range to pcie-root-ports not supporting
native hotplug, resulting into a regression.
Reproduce by:
qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio
device_add e1000,bus=p1
In the Guest OS the respective pcie-root-port will have the IO range
disabled.
Fix it by setting the "reserve-io" hint capability of the
pcie-root-ports so the firmware will allocate the IO range instead.
Acked-by: Igor Mammedov <imammedo(a)redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel(a)redhat.com>
Message-Id: <20210802090057.1709775-1-marcel(a)redhat.com>
Reviewed-by: Michael S. Tsirkin <mst(a)redhat.com>
Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com>
This is what commit (2) alludes to. In pc-q35-6.1 machines we do need
patch (3) since we mask out HPC bit from pcie ports.
I know this is convoluted mess. In fairness I am trying all I can in my
spare time to help from the QEMU side. I am determined to see this
patchset through into libvirt.
Thanks
Laine's comments ...
My memory isn't completely clear, but I think there was also the issue
that the option claims to enable ACPI hotplug when set to on, but
instead what it actually does (in the Q35 case at least) is to enable
native PCI hotplug when set to off (without actually disabling ACPI
hotplug) and disable native PCI hotplug when set to on, or something
like that. This ends up leaving it up to the guest OS to decide which
type of hotplug to use, meaning its decision could override what's in
the libvirt config, thus confusing everyone. Again, I probably have the
details mixed up, but it was something like this.
I asked mst about this this morning, and he suggested something that
you've already done - Cc'ing the series to qemu-devel and the relevant
maintainers so we can have a discussion with all involved parties about
their opinions on whether we really should expose this existing option
in libvirt, or if we should instead have two new options that are more
orthogonal about enabling/disabling the two types of hotplug, so that
libvirt config can more accurately represent what is being presented to
the guest rather than a "best guess" of what we think the guest is going
to do with what is presented.
(Michael did also say that, with the current flurry of bug reports for
the QEMU rc's, this discusion may not happen until closer to release
when the bug reports die down. I know this doesn't mesh with your desire
to "push now to allow for testing" (which in general would be a good
thing if we were certain that we wanted the option like this and were
just expecting some minor bugs that could be fixed), but my opinion is
that 1) it's possible for anyone interested to test the functionality
using <qemu:commandline>, and 2) we should avoid turning libvirt git
into a revolving door of experiments. The only practical difference
between using <qemu:commandline> and having a dedicated option is that
the use of <qemu:commandline> causes the domain to be tainted, and the
XML is a bit more complicated. But since the people we're talking about
here will already have built their own libvirt binaries, the tainted
status of any guests is irrelevant and the extra complexity of using
<qemu:commandline> is probably trivial to them :-).
Ani Sinha (4):
qemu: capablities: detect acpi-pci-hotplug-with-bridge-support
conf: introduce support for acpi-bridge-hotplug feature
qemu: command: add support for acpi-bridge-hotplug feature
NEWS: document new acpi pci hotplug config option
NEWS.rst | 8 ++
docs/formatdomain.rst | 32 +++++++
docs/schemas/domaincommon.rng | 15 ++++
src/conf/domain_conf.c | 89 ++++++++++++++++++-
src/conf/domain_conf.h | 9 ++
src/qemu/qemu_capabilities.c | 4 +
src/qemu/qemu_capabilities.h | 3 +
src/qemu/qemu_command.c | 19 ++++
src/qemu/qemu_validate.c | 42 +++++++++
.../caps_6.1.0.x86_64.xml | 1 +
.../caps_6.2.0.x86_64.xml | 1 +
.../caps_7.0.0.x86_64.xml | 1 +
...-hotplug-bridge-disable.aarch64-latest.err | 1 +
.../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++
...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++
...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 +
...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++
.../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++
.../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++
tests/qemuxml2argvtest.c | 7 ++
...i-hotplug-bridge-disable.x86_64-latest.xml | 1 +
...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 +
...i-hotplug-bridge-disable.x86_64-latest.xml | 1 +
...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 +
tests/qemuxml2xmltest.c | 4 +
27 files changed, 504 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml
--
2.25.1
2 years, 7 months
[libvirt PATCH v3 0/2] iohelper refactoring of io copy function
by Claudio Fontana
changes v1 -> v2:
* fix missing write
changes v2 -> v3:
* move error reporting back into the copy function
* split into two patches, one introducing the IO parameters struct,
and another introducing the new function.
Claudio Fontana (2):
iohelper: introduce new struct to carry copy operation parameters
iohelper: refactor copy operation as a separate function
src/util/iohelper.c | 170 ++++++++++++++++++++++++++------------------
1 file changed, 100 insertions(+), 70 deletions(-)
--
2.34.1
2 years, 7 months
[libvirt PATCH v2] iohelper: some refactoring
by Claudio Fontana
while doing research with alternative implementations of runIO,
it seemed necessary to do some refactoring, in order to separate
parameter setting from the actual copy, so that alternative
copy methods can be researched and hopefully eventually implemented.
No functional changes are expected.
Signed-off-by: Claudio Fontana <cfontana(a)suse.de>
---
src/util/iohelper.c | 197 +++++++++++++++++++++++++++-----------------
1 file changed, 122 insertions(+), 75 deletions(-)
changes v1 -> v2:
* fix missing write
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 2c91bf4f93..ca569b5ae9 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -45,21 +45,38 @@
# define O_DIRECT 0
#endif
-static int
-runIO(const char *path, int fd, int oflags)
+struct runIOParams {
+ bool isBlockDev;
+ bool isDirect;
+ bool isWrite;
+ int fdin;
+ const char *fdinname;
+ int fdout;
+ const char *fdoutname;
+};
+
+/* runIOCopy: execute the IO copy based on the passed parameters */
+/**
+ * runIOCopy:
+ * @p: the IO parameters
+ *
+ * Execute the copy based on the passed parameters.
+ *
+ * Returns: size transfered, or < 0 on error.
+ * Errors: -1 = read/write error
+ * -2 = read error
+ * -3 = write error
+ * -4 = truncate error
+ */
+
+static off_t
+runIOCopy(const struct runIOParams p)
{
g_autofree void *base = NULL; /* Location to be freed */
char *buf = NULL; /* Aligned location within base */
size_t buflen = 1024*1024;
intptr_t alignMask = 64*1024 - 1;
- int ret = -1;
- int fdin, fdout;
- const char *fdinname, *fdoutname;
- unsigned long long total = 0;
- bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0);
- off_t end = 0;
- struct stat sb;
- bool isBlockDev = false;
+ off_t total = 0;
#if WITH_POSIX_MEMALIGN
if (posix_memalign(&base, alignMask + 1, buflen))
@@ -71,50 +88,6 @@ runIO(const char *path, int fd, int oflags)
buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
#endif
- if (fstat(fd, &sb) < 0) {
- virReportSystemError(errno,
- _("Unable to access file descriptor %d path %s"),
- fd, path);
- goto cleanup;
- }
- isBlockDev = S_ISBLK(sb.st_mode);
-
- switch (oflags & O_ACCMODE) {
- case O_RDONLY:
- fdin = fd;
- fdinname = path;
- fdout = STDOUT_FILENO;
- fdoutname = "stdout";
- /* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
- if (!isBlockDev && direct && ((end = lseek(fd, 0, SEEK_CUR)) != 0)) {
- virReportSystemError(end < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT read needs entire seekable file"));
- goto cleanup;
- }
- break;
- case O_WRONLY:
- fdin = STDIN_FILENO;
- fdinname = "stdin";
- fdout = fd;
- fdoutname = path;
- /* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
- if (!isBlockDev && direct && (end = lseek(fd, 0, SEEK_END)) != 0) {
- virReportSystemError(end < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT write needs empty seekable file"));
- goto cleanup;
- }
- break;
-
- case O_RDWR:
- default:
- virReportSystemError(EINVAL,
- _("Unable to process file with flags %d"),
- (oflags & O_ACCMODE));
- goto cleanup;
- }
-
while (1) {
ssize_t got;
@@ -124,53 +97,127 @@ runIO(const char *path, int fd, int oflags)
* writes will be aligned.
* In other cases using saferead reduces number of syscalls.
*/
- if (fdin == fd && direct) {
- if ((got = read(fdin, buf, buflen)) < 0 &&
+ if (!p.isWrite && p.isDirect) {
+ if ((got = read(p.fdin, buf, buflen)) < 0 &&
errno == EINTR)
continue;
} else {
- got = saferead(fdin, buf, buflen);
- }
-
- if (got < 0) {
- virReportSystemError(errno, _("Unable to read %s"), fdinname);
- goto cleanup;
+ got = saferead(p.fdin, buf, buflen);
}
+ if (got < 0)
+ return -2;
if (got == 0)
break;
total += got;
/* handle last write size align in direct case */
- if (got < buflen && direct && fdout == fd) {
+ if (got < buflen && p.isDirect && p.isWrite) {
ssize_t aligned_got = (got + alignMask) & ~alignMask;
memset(buf + got, 0, aligned_got - got);
- if (safewrite(fdout, buf, aligned_got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), fdoutname);
- goto cleanup;
+ if (safewrite(p.fdout, buf, aligned_got) < 0) {
+ return -3;
}
-
- if (!isBlockDev && ftruncate(fd, total) < 0) {
- virReportSystemError(errno, _("Unable to truncate %s"), fdoutname);
- goto cleanup;
+ if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
+ return -4;
}
-
break;
}
- if (safewrite(fdout, buf, got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), fdoutname);
+ if (safewrite(p.fdout, buf, got) < 0) {
+ return -3;
+ }
+ }
+ return total;
+}
+
+static int
+runIO(const char *path, int fd, int oflags)
+{
+ int ret = -1;
+ off_t total = 0;
+ struct stat sb;
+ struct runIOParams p;
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Unable to access file descriptor %d path %s"),
+ fd, path);
+ goto cleanup;
+ }
+ p.isBlockDev = S_ISBLK(sb.st_mode);
+ p.isDirect = O_DIRECT && (oflags & O_DIRECT);
+
+ switch (oflags & O_ACCMODE) {
+ case O_RDONLY:
+ p.isWrite = false;
+ p.fdin = fd;
+ p.fdinname = path;
+ p.fdout = STDOUT_FILENO;
+ p.fdoutname = "stdout";
+ break;
+ case O_WRONLY:
+ p.isWrite = true;
+ p.fdin = STDIN_FILENO;
+ p.fdinname = "stdin";
+ p.fdout = fd;
+ p.fdoutname = path;
+ break;
+ case O_RDWR:
+ default:
+ virReportSystemError(EINVAL,
+ _("Unable to process file with flags %d"),
+ (oflags & O_ACCMODE));
+ goto cleanup;
+ }
+ /* To make the implementation simpler, we give up on any
+ * attempt to use O_DIRECT in a non-trivial manner. */
+ if (!p.isBlockDev && p.isDirect) {
+ off_t end;
+ if (p.isWrite) {
+ if ((end = lseek(fd, 0, SEEK_END)) != 0) {
+ virReportSystemError(end < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT write needs empty seekable file"));
+ goto cleanup;
+ }
+ } else if ((end = lseek(fd, 0, SEEK_CUR)) != 0) {
+ virReportSystemError(end < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT read needs entire seekable file"));
goto cleanup;
}
}
+ total = runIOCopy(p);
+
+ if (total < 0) {
+ switch (total) {
+ case -1:
+ virReportSystemError(errno, _("Unable to move data from %s to %s"),
+ p.fdinname, p.fdoutname);
+ break;
+ case -2:
+ virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
+ break;
+ case -3:
+ virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
+ break;
+ case -4:
+ virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname);
+ break;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown runIOCopy error %ld"), total);
+ break;
+ }
+ goto cleanup;
+ }
+
/* Ensure all data is written */
- if (virFileDataSync(fdout) < 0) {
+ if (virFileDataSync(p.fdout) < 0) {
if (errno != EINVAL && errno != EROFS) {
/* fdatasync() may fail on some special FDs, e.g. pipes */
- virReportSystemError(errno, _("unable to fsync %s"), fdoutname);
+ virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname);
goto cleanup;
}
}
--
2.35.1
2 years, 7 months
[RFC PATCH v1 0/5] Add virDomainGetSevAttestationReport API
by Tyler Fanelli
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a
virsh command "domgetsevreport"), with initial QEMU support via the
"query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is
supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of
being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr
exchanged between the client and libvirtd and how they interact together, as I
have seen no other API follow the method I used. Namely, the same
virTypedParameterPtr is used for both input _AND_ output. The same
virTypedParameterPtr containing the original mnonce string inputted to the API is
also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a
virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting
and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested
improvements.
Tyler Fanelli (5):
libvirt: Introduce virDomainGetSevAttestationReport public API
remote: add RPC support for the virDomainGetSevAttestationReport API
qemu_capabilities: Introduce QEMU_CAPS_SEV_GET_ATTESTATION_REPORT
qemu: Implement the virDomainGetSevAttestationReport API
tools: add domgetsevreport virsh command
docs/manpages/virsh.rst | 18 ++++
include/libvirt/libvirt-domain.h | 22 +++++
src/driver-hypervisor.h | 7 ++
src/libvirt-domain.c | 63 ++++++++++++++
src/libvirt_public.syms | 4 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 86 +++++++++++++++++++
src/qemu/qemu_monitor.c | 11 +++
src/qemu/qemu_monitor.h | 5 ++
src/qemu/qemu_monitor_json.c | 40 +++++++++
src/qemu/qemu_monitor_json.h | 5 ++
src/remote/remote_daemon_dispatch.c | 44 ++++++++++
src/remote/remote_driver.c | 55 ++++++++++++
src/remote/remote_protocol.x | 21 ++++-
src/remote_protocol-structs | 12 +++
.../caps_6.1.0.x86_64.xml | 1 +
.../caps_6.2.0.x86_64.xml | 1 +
.../caps_7.0.0.x86_64.xml | 1 +
tools/virsh-domain.c | 68 +++++++++++++++
20 files changed, 466 insertions(+), 1 deletion(-)
--
2.34.1
2 years, 7 months
[PATCH RESEND] qemu: disarm fake reboot flag on reset
by Nikolay Shirokovskiy
From: Maxim Nestratov <mnestratov(a)virtuozzo.com>
This is a quite an old (created at 2016) patch fixing an issue for at
that time contemporary Fedora 23. virsh reboot returns success (yet
after hanging for a while), VM is rebooted sucessfully too but then
shutdown from inside guest causes reboot and not shutdown.
VM has agent installed. So virsh reboot first tries to reboot VM thru
the agent. The agent calls 'shutdown -r' command. Typically it returns
instantly but on this distro for some reason it takes time. I did not
investigate the cause but the command waits in dbus client code,
probably waits for reply. The libvirt waits 60s for agent command to
execute and then errors out. Next reboot API falls back to ACPI shutdown
which returns successfully thus the reboot command return success too.
Yet shutdown command in guest eventually successfull and guest is truly
rebooted. So libvirt does not receive SHUTDOWN event and fake reboot
flag which is armed on fallback path stays armed. Thus next shutdown
from guest leads to reboot.
The issue has 100% repro on Fedora 23. On modern distros I can't
reproduce it at all. Shutdown command is asynchronous and returns
immediately even if I start some service that ignores TERM signal and
thus shutdown procedure waits for 90s (if I not mistaken) before sending
KILL.
Yet I guess it is nice to have this patch to be more robust.
Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy(a)openvz.org>
---
src/qemu/qemu_process.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f63fc3389d..d81ed9391a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -435,6 +435,7 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED,
if (priv->agent)
qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_RESET);
+ qemuDomainSetFakeReboot(vm, false);
qemuDomainSaveStatus(vm);
unlock:
--
2.35.1
2 years, 7 months
[libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios
by Daniel P. Berrangé
There are a mind bending number of possible ways to configure the
firmware with/without NVRAM. Only a small portion are tested and
many error scenarios are silently ignored.
This series attempts to get coverage of every possible XML config
scenario and report explicit errors in all invalid configs.
There is an open question on patch 4. Essentially the use of NVRAM
combined with writable executable feels like an accidental feature
in libvirt that hasn't really been thought through. I'd like to
better define expectations here but there are several possible
strategies and I'm undecided which is best.
Daniel P. Berrangé (10):
qemu: fix bad indentation for qemuDomainNVRAMPathFormat
tests: add explicit test case for pflash loader lacking path
tests: add test case for NVRAM with template
conf: validate NVRAM template usage with R/W loader binary
tests: don't permit NVRAM path when using firmware auto-select
qemu: inline code for filling in per-VM NVRAM path
conf: rename struct field for NVRAM template
conf: switch nvram parsing to use XML node / property helpers
conf: move nvram parsing into virDomainLoaderDefParseXML
conf: stop ignoring <loader>/<nvram> with firmware auto-select
src/conf/domain_conf.c | 121 +++++++++++-------
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_domain.c | 18 +--
src/qemu/qemu_domain.h | 8 +-
src/qemu/qemu_firmware.c | 13 +-
src/qemu/qemu_process.c | 4 +-
tests/qemuxml2argvdata/bios-nvram-no-path.err | 1 +
tests/qemuxml2argvdata/bios-nvram-no-path.xml | 19 +++
...-nvram-rw-template-vars.x86_64-latest.args | 41 ++++++
.../bios-nvram-rw-template-vars.xml | 36 ++++++
.../bios-nvram-rw-template.err | 1 +
.../bios-nvram-rw-template.xml | 36 ++++++
.../bios-nvram-rw-vars.x86_64-latest.args | 41 ++++++
tests/qemuxml2argvdata/bios-nvram-rw-vars.xml | 36 ++++++
.../bios-nvram-template.x86_64-latest.args | 37 ++++++
.../qemuxml2argvdata/bios-nvram-template.xml | 21 +++
tests/qemuxml2argvdata/os-firmware-bios.xml | 1 -
.../os-firmware-efi-bad-loader-path.err | 1 +
.../os-firmware-efi-bad-loader-path.xml | 67 ++++++++++
.../os-firmware-efi-bad-loader-type.err | 1 +
.../os-firmware-efi-bad-loader-type.xml | 67 ++++++++++
.../os-firmware-efi-bad-nvram-path.err | 1 +
.../os-firmware-efi-bad-nvram-path.xml | 68 ++++++++++
.../os-firmware-efi-bad-nvram-template.err | 1 +
.../os-firmware-efi-bad-nvram-template.xml | 68 ++++++++++
.../os-firmware-efi-secboot.xml | 1 -
tests/qemuxml2argvdata/os-firmware-efi.xml | 1 -
tests/qemuxml2argvtest.c | 9 ++
.../os-firmware-bios.x86_64-latest.xml | 1 -
.../os-firmware-efi-secboot.x86_64-latest.xml | 1 -
.../os-firmware-efi.x86_64-latest.xml | 1 -
31 files changed, 647 insertions(+), 77 deletions(-)
create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.err
create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.err
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.xml
--
2.34.1
2 years, 7 months
[PATCH] apparmor: Allow swtpm to use its own apparmor profile
by Lena Voytek
Signed-off-by: Lena Voytek <lena.voytek(a)canonical.com>
---
src/security/apparmor/libvirt-qemu | 3 ++-
src/security/apparmor/usr.sbin.libvirtd.in | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu
index 250ba4ea58..c29168da27 100644
--- a/src/security/apparmor/libvirt-qemu
+++ b/src/security/apparmor/libvirt-qemu
@@ -180,7 +180,7 @@
audit deny /{var/,}run/qemu/*/*.so w,
# swtpm
- /{usr/,}bin/swtpm rmix,
+ /{usr/,}bin/swtpm rmpix,
/usr/{lib,lib64}/libswtpm_libtpms.so mr,
/usr/lib/(a){multiarch}/libswtpm_libtpms.so mr,
@@ -226,6 +226,7 @@
unix (send, receive) type=stream addr=none peer=(label=libvirtd),
unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd),
unix (send, receive) type=stream addr=none peer=(label=virtqemud),
+ unix (send, receive) type=stream addr=none peer=(label=swtpm),
# for gathering information about available host resources
/sys/devices/system/cpu/ r,
diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in
index f2ab6ff2aa..886f1ad518 100644
--- a/src/security/apparmor/usr.sbin.libvirtd.in
+++ b/src/security/apparmor/usr.sbin.libvirtd.in
@@ -58,6 +58,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) {
ptrace (read,trace) peer=dnsmasq,
ptrace (read,trace) peer=/usr/sbin/dnsmasq,
ptrace (read,trace) peer=libvirt-*,
+ ptrace (read,trace) peer=swtpm,
signal (send) peer=dnsmasq,
signal (send) peer=/usr/sbin/dnsmasq,
--
2.25.1
2 years, 7 months
[PATCH] qemu: Check usage count of qemu:override node
by Justin Gatzen
When <qemu:override> is the only usage of the qemu namespace the entire
section is mistakenly removed. Add check for use count.
Signed-off-by: Justin Gatzen <justin.gatzen(a)gmail.com>
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 95134a3570..00c209313b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3514,7 +3514,8 @@ qemuDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
if (nsdata->args || nsdata->num_env > 0 ||
nsdata->capsadd || nsdata->capsdel ||
- nsdata->deprecationBehavior)
+ nsdata->deprecationBehavior ||
+ nsdata->ndeviceOverride > 0)
*data = g_steal_pointer(&nsdata);
ret = 0;
--
2.35.1
2 years, 7 months
