[libvirt] [PATCH 0/2] Improve error message for missing backing image format
by Peter Krempa
Improve the error message and mention it in the news.
Peter Krempa (2):
util: storage: Link to knowledge base when reporting missing image
backing format
news: Mention problems with backing image format probing
docs/news.xml | 13 +++++++++++++
src/util/virstoragefile.c | 3 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
--
2.24.1
4 years, 11 months
[libvirt] [PATCH v3 0/5] introduce support for an embedded driver mode
by Daniel P. Berrangé
This is a followup to:
https://www.redhat.com/archives/libvir-list/2019-May/msg00467.html
https://www.redhat.com/archives/libvir-list/2019-December/msg00050.html
This series implements support for an embedded driver mode for libvirt,
with initial support in the QEMU and secrets drivers.
In this mode of operation, the driver stores all its config and state
under a private directory tree. See the individual patches for the
illustrated directory hierarchy used.
The intent of this embedded mode is to suit cases where the application
is using virtualization as a building block for some functionality, as
opposed to running traditional "full OS" builds.
The long time posterchild example would be libguestfs, while a more
recent example could be Kata containers.
The general principal in enabling this embedded mode is that the
functionality available should be identical to that seen when the
driver is running inside libvirtd. This is achieved by loading the
exact same driver .so module as libvirtd would load, and simply
configuring it with a different directory layout.
The result of this is that when running in embedded mode, the driver
can still talk to other secondary drivers running inside libvirtd
if desired. This is useful, for example, to connect a VM to the
default virtual network.
The secondary drivers can be made to operate in embedded mode as
well, however, this will require some careful consideration for each
driver to ensure they don't clash with each other. Thus in this series
only the secret driver is enabled for embedded mode. This is required
to enable use of VMs with encrypted disks, or authenticated network
block storage.
In this series we introduce a new command line tool 'virt-qemu-run'
which is a really simple tool for launching a VM in embedded mode.
I'm not entirely sure whether we should provide this as an official
supported tool in this way, or merely put it into the 'examples'
directory as demo-ware.
With testing of the virt-qemu-run tool we can immediately see what the
next important thing to tackle is: performance. We have not really cared
too much about the startup performance of libvirtd as this is a one time
cost when the mgmt application connects. We did none the less cache
capabilities because probing caps for 30 QEMU binaries takes a long time.
Even with this caching it takes an unacceptably long time to start a VM
in embedded mode. About 100 ms to open the embedded QEMU driver,
assuming pre-cached capabilies - ~2 seconds if not cached and all 30
QEMU targets are present. Then about 300 ms to actually start the QEMU
guest.
IOW, about 400 ms to get QEMU running. NB this is measuring time from
launching the virt-run-qemu program, to the point at which the API call
'virDomainCreate' returns control. This has both libvirt & QEMU overhead
in & I don't have clear figures to distinguish, but I can see a 40 ms
delay between issuing the 'qmp_capabilities' call and getting a reply,
which is QEMU startup overead.
This is a i440fx based QEMU with a general purpose virtio-pci config
(disk, net, etc) tyupical for running a full OS. I've not tried any
kind of optimized QEMU config with microvm.
I've already started on measuring & optimizing & identified several key
areas that can be addressed, but it is all ultimately about not doing
work before we need the answers from that work (which often means we
will never do the work at all).
For example, we shouldn't probe all 30 QEMU's upfront. If the app is
only going to create an x86_64 KVM guest we should only care about that
1 QEMU. This is painful because parsing any guest XML requires a
virCapsPtr which in turn causes probing of every QEMU binary. I've got
in progress patches to eliminate virCapsPtr almost entirely and work
directly with the virQEMUCapsPtr instead.
It is possible we'll want to use a different file format for storing
the cached QEMU capabilities, and the CPU feature/model info. Parsing
this XML is a non-negligible time sink. A binary format is likely way
quicker, especially if its designed to be just mmap'able for direct
read. To be investigated...
We shouldn't probe for whether host PM suspend is possible unless
someone wants that info, or tries to issue that API call.
After starting QEMU we spend 150-200 ms issuing a massive number of
qom-get calls to check whether QEMU enabled each individual CPU feature
flag. We only need this info if someone asks for the live XML or we
intend to live migrate etc. So we shouldn't issue these qom-get calls in
the "hot path" of QEMU startup. It can be done later in a non-time
critical point. Also the QEMU API for this is horribly inefficient to
require so many qom-get calls.
There's more but I won't talk about it now. Suffice to say that I think
we can get libvirt overhead down to less than 100 ms fairly easily and
probably even down to less than 50 ms without much effort.
The exact figure will depend on what libvirt features you want enabled,
and how much work we want/need to put into optimization. We'll want to
fix the really gross mistakes & slow downs, but we'll want guidance from
likely users as to their VM startup targets to decide how much work
needs investing.
This optimization will ultimately help non-embedded QEMU mode too,
making it faster to respond & start.
Changed in v3:
- Rebased to master
- Merge some simple acked patches
Changed in v2:
- Use a simplified directory layout for embedded mode. Previously we
just put a dir prefix onto the normal paths. This has the downside
that the embedded drivers paths are needlessly different for
privileged vs unprivileged user. It also results in very long paths
which can be a problem for the UNIX socket name length limits.
- Also ported the secret driver to support embedded mode
- Check to validate that the event loop is registered.
- Add virt-qemu-run tool for embedded usage.
- Added docs for the qemu & secret driver explaining embedded mode
Daniel P. Berrangé (5):
libvirt: pass a directory path into drivers for embedded usage
libvirt: support an "embed" URI path selector for opening drivers
qemu: add support for running QEMU driver in embedded mode
secrets: add support for running secret driver in embedded mode
qemu: introduce a new "virt-qemu-run" program
build-aux/syntax-check.mk | 2 +-
docs/Makefile.am | 5 +
docs/drivers.html.in | 1 +
docs/drvqemu.html.in | 84 +++++++
docs/drvsecret.html.in | 82 ++++++
docs/manpages/index.rst | 1 +
docs/manpages/virt-qemu-run.rst | 114 +++++++++
libvirt.spec.in | 2 +
src/Makefile.am | 1 +
src/driver-state.h | 2 +
src/driver.h | 2 +
src/interface/interface_backend_netcf.c | 7 +
src/interface/interface_backend_udev.c | 7 +
src/libvirt.c | 93 ++++++-
src/libvirt_internal.h | 4 +-
src/libxl/libxl_driver.c | 7 +
src/lxc/lxc_driver.c | 8 +
src/network/bridge_driver.c | 7 +
src/node_device/node_device_hal.c | 7 +
src/node_device/node_device_udev.c | 7 +
src/nwfilter/nwfilter_driver.c | 7 +
src/qemu/Makefile.inc.am | 13 +
src/qemu/qemu_conf.c | 38 ++-
src/qemu/qemu_conf.h | 6 +-
src/qemu/qemu_driver.c | 21 +-
src/qemu/qemu_process.c | 15 +-
src/qemu/qemu_shim.c | 322 ++++++++++++++++++++++++
src/remote/remote_daemon.c | 1 +
src/remote/remote_driver.c | 1 +
src/secret/secret_driver.c | 41 ++-
src/storage/storage_driver.c | 7 +
src/vz/vz_driver.c | 7 +
tests/domaincapstest.c | 2 +-
tests/testutilsqemu.c | 3 +-
34 files changed, 901 insertions(+), 26 deletions(-)
create mode 100644 docs/drvsecret.html.in
create mode 100644 docs/manpages/virt-qemu-run.rst
create mode 100644 src/qemu/qemu_shim.c
--
2.23.0
4 years, 11 months
[libvirt] [PATCH v3 0/3] Mark and document restore with managed save as risky
by Michael Weiser
This series marks restore of an inactive qemu snapshot while there is
managed saved state as risky due to the reasons explained in patch 1 and
3. Patch 2 is a simple reformatting of the documentation with no other
changes in preparation of addition of more reasons why reverts might
need to be forced.
Changes from v2:
- fix manpage typo exising -> existing
- change manpage qemu hypervisor name case to QEMU
Changes from v1:
- reword error message to "error: revert requires force: snapshot
without memory state, removal of existing managed saved state strongly
recommended to avoid corruption"
- add documentation of the new behaviour
Michael Weiser (3):
qemu: Warn of restore with managed save being risky
docs: Reformat snapshot-revert force reasons
docs: Add snapshot-revert qemu managedsave force
docs/manpages/virsh.rst | 39 ++++++++++++++++++++++++---------------
src/qemu/qemu_driver.c | 9 +++++++++
2 files changed, 33 insertions(+), 15 deletions(-)
--
2.24.1
4 years, 11 months
[libvirt] [PATCH] tests: avoid re-execing test once for each mock
by Daniel P. Berrangé
When debugging tests under GDB there is a significant time
delay each time an execve is done as GDB scans shared libraries
once again. For tests which use many mock libraries, we have
been invoking execve many times which makes the GDB experience
horrible. This changes our framework to activate the full
set of mock libraries in one single execve.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
tests/qemucapsprobe.c | 8 +++++++-
tests/testutils.c | 29 ++++++++++++++++++++++++-----
tests/testutils.h | 10 +++-------
3 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c
index 6837f2a0f6..c7e8f3309d 100644
--- a/tests/qemucapsprobe.c
+++ b/tests/qemucapsprobe.c
@@ -46,8 +46,14 @@ main(int argc, char **argv)
{
virThread thread;
virQEMUCapsPtr caps;
+ const char *mock = VIR_TEST_MOCK("qemucapsprobe");
- VIR_TEST_PRELOAD(VIR_TEST_MOCK("qemucapsprobe"));
+ if (!virFileIsExecutable(mock)) {
+ perror(mock);
+ return EXIT_FAILURE;
+ }
+
+ VIR_TEST_PRELOAD(mock);
if (argc != 2) {
fprintf(stderr, "%s QEMU_binary\n", argv[0]);
diff --git a/tests/testutils.c b/tests/testutils.c
index 6f0ef2b2f1..b490609e36 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -856,15 +856,34 @@ int virTestMain(int argc,
virLogOutputPtr *outputs = NULL;
g_autofree char *baseprogname = NULL;
const char *progname;
-
- if (getenv("VIR_TEST_FILE_ACCESS"))
- VIR_TEST_PRELOAD(VIR_TEST_MOCK("virtest"));
+ g_autofree const char **preloads = NULL;
+ size_t npreloads = 0;
+ g_autofree char *mock = NULL;
+
+ if (getenv("VIR_TEST_FILE_ACCESS")) {
+ preloads = g_renew(const char *, preloads, npreloads + 2);
+ preloads[npreloads++] = VIR_TEST_MOCK("virtest");
+ preloads[npreloads] = NULL;
+ }
va_start(ap, func);
- while ((lib = va_arg(ap, const char *)))
- VIR_TEST_PRELOAD(lib);
+ while ((lib = va_arg(ap, const char *))) {
+ if (!virFileIsExecutable(lib)) {
+ perror(lib);
+ return EXIT_FAILURE;
+ }
+
+ preloads = g_renew(const char *, preloads, npreloads + 2);
+ preloads[npreloads++] = lib;
+ preloads[npreloads] = NULL;
+ }
va_end(ap);
+ if (preloads) {
+ mock = g_strjoinv(":", (char **)preloads);
+ VIR_TEST_PRELOAD(mock);
+ }
+
progname = baseprogname = g_path_get_basename(argv[0]);
if (STRPREFIX(progname, "lt-"))
progname += 3;
diff --git a/tests/testutils.h b/tests/testutils.h
index 8a7ea38f43..2944c25e8b 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -127,19 +127,15 @@ int virTestMain(int argc,
# define MOCK_EXT ".so"
#endif
-#define VIR_TEST_PRELOAD(lib) \
+#define VIR_TEST_PRELOAD(libs) \
do { \
const char *preload = getenv(PRELOAD_VAR); \
if (preload == NULL || strstr(preload, lib) == NULL) { \
char *newenv; \
- if (!virFileIsExecutable(lib)) { \
- perror(lib); \
- return EXIT_FAILURE; \
- } \
if (!preload) { \
- newenv = (char *) lib; \
+ newenv = (char *) libs; \
} else { \
- newenv = g_strdup_printf("%s:%s", lib, preload); \
+ newenv = g_strdup_printf("%s:%s", libs, preload); \
} \
g_setenv(PRELOAD_VAR, newenv, TRUE); \
FORCE_FLAT_NAMESPACE \
--
2.24.1
4 years, 11 months
[libvirt] [PATCH 00/19] qemu: Add support for backups in combination with snapshots
by Peter Krempa
IMPORTANT:
This does NOT add support for merging the bitmaps during block jobs, so
manual deletion of snapshots will corrupt the incremental metadata!.
Allow combining snapshots and backups. During a snapshot we create a
bitmap for any active persistent bitmap to continue tracking the bitmaps
properly (as qemu doesn't support creating a dirty bitmap from the
allocation map). This also means that it works only for an active VM
since there also isn't a way to do it via qemu-img.
Thus a workaround is to start a VM as paused if this is required for
offline VMs.
We also now can merge bitmaps accross the backing chains.
You can fetch the changes at:
git fetch https://gitlab.com/pipo.sk/libvirt.git backup-snapshots
Peter Krempa (19):
virsh: Add QMP command wrapping for 'qemu-monitor-command'
virsh: Allow extracting 'return' section of QMP command in
'qemu-monitor-command'
qemu: monitor: Extract data about dirty-bimaps in
qemuMonitorBlockGetNamedNodeData
qemu: monitor: Extract internals of
qemuMonitorJSONBlockGetNamedNodeData
tests: qemublock: Add test for bitmap detection
tests: qemublocktest: Add a syntetic test case for bitmap detection
qemu: Check for explicit failure of qemuBlockSnapshotAddBlockdev
qemu: snapshot: Fold formatting of snapshot transaction into prepare
func
qemu: monitor: Add 'granularity' parameter for block-dirty-bitmap-add
qemu: snapshot: Propagate active bitmaps through external snapshots
tests: qemublock: Add test case for detecting bitmaps as we create
snapshots
qemu: backup: Return 'def' instead of 'obj' from
qemuBackupBeginCollectIncrementalCheckpoints
qemu: backup: Extract calculations of bitmaps to merge for incremental
backup
qemu: backup: Propagate bitmap metadata into
qemuBackupDiskPrepareOneBitmapsChain
qemu: backup: Export qemuBackupDiskPrepareOneBitmapsChain for tests
tests: qemublock: Add testing of bitmap merging for incremental
backups
qemu: block: Introduce qemuBlockNamedNodeDataGetBitmapByName
qemu: backup: Merge bitmaps accross the backing chain
tests: qemublock: Add tests for cross-snapshot incremental backups
docs/manpages/virsh.rst | 27 +-
src/qemu/qemu_backup.c | 145 ++-
src/qemu/qemu_backup.h | 7 +
src/qemu/qemu_block.c | 32 +
src/qemu/qemu_block.h | 5 +
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_driver.c | 84 +-
src/qemu/qemu_monitor.c | 6 +-
src/qemu/qemu_monitor.h | 18 +-
src/qemu/qemu_monitor_json.c | 96 +-
src/qemu/qemu_monitor_json.h | 6 +-
tests/qemublocktest.c | 241 +++++
.../backupmerge/basic-deep-out.json | 22 +
.../backupmerge/basic-flat-out.json | 6 +
.../backupmerge/basic-intermediate-out.json | 10 +
.../backupmerge/snapshot-deep-out.json | 38 +
.../backupmerge/snapshot-flat-out.json | 6 +
.../snapshot-intermediate-out.json | 14 +
tests/qemublocktestdata/bitmap/basic.json | 117 +++
tests/qemublocktestdata/bitmap/basic.out | 6 +
tests/qemublocktestdata/bitmap/snapshots.json | 836 ++++++++++++++++++
tests/qemublocktestdata/bitmap/snapshots.out | 14 +
tests/qemublocktestdata/bitmap/synthetic.json | 118 +++
tests/qemublocktestdata/bitmap/synthetic.out | 6 +
tests/qemumonitorjsontest.c | 2 +-
tools/virsh-domain.c | 76 +-
26 files changed, 1845 insertions(+), 95 deletions(-)
create mode 100644 tests/qemublocktestdata/backupmerge/basic-deep-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/basic-flat-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/basic-intermediate-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
create mode 100644 tests/qemublocktestdata/bitmap/basic.json
create mode 100644 tests/qemublocktestdata/bitmap/basic.out
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out
create mode 100644 tests/qemublocktestdata/bitmap/synthetic.json
create mode 100644 tests/qemublocktestdata/bitmap/synthetic.out
--
2.23.0
4 years, 11 months
[libvirt] [PATCH v2] docs: Harmonize hypervisor names for QEMU and LXC
by Michael Weiser
Trivially replace usages of qemu and lxc in the virsh manpage with their
more heavily used and (according to Wikipedia) correct upper-case
spellings QEMU and LXC.
Signed-off-by: Michael Weiser <michael.weiser(a)gmx.de>
Suggested-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
docs/manpages/virsh.rst | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..26f08b7701 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -577,7 +577,7 @@ from the domain's XML <os/> element and <type/> subelement or one from a
list of machines from the ``virsh capabilities`` output for a specific
architecture and domain type.
-For the qemu hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
+For the QEMU hypervisor, a *virttype* of either 'qemu' or 'kvm' must be
supplied along with either the *emulatorbin* or *arch* in order to
generate output for the default *machine*. Supplying a *machine*
value will generate output for the specific machine.
@@ -1072,7 +1072,7 @@ read I/O operations limit.
write I/O operations limit.
*--size-iops-sec* specifies size I/O operations limit per second.
*--group-name* specifies group name to share I/O quota between multiple drives.
-For a qemu domain, if no name is provided, then the default is to have a single
+For a QEMU domain, if no name is provided, then the default is to have a single
group for each *device*.
Older versions of virsh only accepted these options with underscore
@@ -1084,7 +1084,7 @@ An explicit 0 also clears any limit. A non-zero value for a given total
cannot be mixed with non-zero values for read or write.
It is up to the hypervisor to determine how to handle the length values.
-For the qemu hypervisor, if an I/O limit value or maximum value is set,
+For the QEMU hypervisor, if an I/O limit value or maximum value is set,
then the default value of 1 second will be displayed. Supplying a 0 will
reset the value back to the default.
@@ -1211,7 +1211,7 @@ to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to *domain* (see
also ``domblklist`` for listing these names).
*bandwidth* specifies copying bandwidth limit in MiB/s, although for
-qemu, it may be non-zero only for an online domain. For further information
+QEMU, it may be non-zero only for an online domain. For further information
on the *bandwidth* argument see the corresponding section for the ``blockjob``
command.
@@ -1642,7 +1642,7 @@ domblkstat
Get device block stats for a running domain. A *block-device* corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to *domain* (see
-also ``domblklist`` for listing these names). On a lxc or qemu domain,
+also ``domblklist`` for listing these names). On a LXC or QEMU domain,
omitting the *block-device* yields device block stats summarily for the
entire domain.
@@ -3247,7 +3247,7 @@ destination). Some hypervisors do not support this feature and will return an
error if this parameter is used.
Optional *disks-port* sets the port that hypervisor on destination side should
-bind to for incoming disks traffic. Currently it is supported only by qemu.
+bind to for incoming disks traffic. Currently it is supported only by QEMU.
migrate-compcache
@@ -7438,7 +7438,7 @@ qemu-monitor-command
qemu-monitor-command domain { [--hmp] | [--pretty] } command...
Send an arbitrary monitor command *command* to domain *domain* through the
-qemu monitor. The results of the command will be printed on stdout. If
+QEMU monitor. The results of the command will be printed on stdout. If
*--hmp* is passed, the command is considered to be a human monitor command
and libvirt will automatically convert it into QMP if needed. In that case
the result will also be converted back from QMP. If *--pretty* is given,
@@ -7457,7 +7457,7 @@ qemu-agent-command
qemu-agent-command domain [--timeout seconds | --async | --block] command...
Send an arbitrary guest agent command *command* to domain *domain* through
-qemu agent.
+QEMU agent.
*--timeout*, *--async* and *--block* options are exclusive.
*--timeout* requires timeout seconds *seconds* and it must be positive.
When *--aysnc* is given, the command waits for timeout whether success or
--
2.24.1
4 years, 11 months
[libvirt] [PATCH] tests: avoid probing host CPU from bhyve test
by Daniel P. Berrangé
bhyveargv2xmlmock calls virBhyveCapsBuild which in turn
calls virCPUProbeHost, probing the real host CPU. This
causes a test failure if the host CPU happens to contain
the 'arch-capabilities' feature as it triggers a call
to virHostCPUGetMSR() which fails on FreeBSD.
Fortunately we already have convenient code for mocking
the host CPU probing.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
tests/bhyveargv2xmlmock.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tests/bhyveargv2xmlmock.c b/tests/bhyveargv2xmlmock.c
index 1f08bebb7b..8120be29c1 100644
--- a/tests/bhyveargv2xmlmock.c
+++ b/tests/bhyveargv2xmlmock.c
@@ -2,7 +2,9 @@
#include "virnetdev.h"
#include "internal.h"
+#include "testutilshostcpus.h"
#include "util/viruuid.h"
+#include "cpu/cpu.h"
#define VIR_FROM_THIS VIR_FROM_BHYVE
@@ -25,3 +27,9 @@ virUUIDGenerate(unsigned char *uuid)
return -1;
return 0;
}
+
+virCPUDefPtr
+virCPUProbeHost(virArch arch)
+{
+ return testUtilsHostCpusGetDefForArch(arch);
+}
--
2.24.1
4 years, 11 months
[libvirt] [PATCH] qemu: Don't use NULL path from qemuDomainGetHostdevPath
by Jiri Denemark
Commit v5.10.0-290-g3a4787a301 refactored qemuDomainGetHostdevPath to
return a single path rather than an array of paths. When the function is
called on a missing device, it will now return NULL in @path rather than
a NULL array with zero items and the callers need to be adapted
properly.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 32 ++++++++++++++++++--------------
src/qemu/qemu_domain.c | 9 +++++----
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2bcc0527f6..45701b4c6e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -426,13 +426,15 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
if (qemuDomainGetHostdevPath(dev, &path, &perms) < 0)
return -1;
- VIR_DEBUG("Cgroup allow %s perms=%d", path, perms);
- rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, false);
- virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
- virCgroupGetDevicePermsString(perms),
- rv);
- if (rv < 0)
- return -1;
+ if (path) {
+ VIR_DEBUG("Cgroup allow %s perms=%d", path, perms);
+ rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
+ virCgroupGetDevicePermsString(perms),
+ rv);
+ if (rv < 0)
+ return -1;
+ }
if (qemuHostdevNeedsVFIO(dev)) {
VIR_DEBUG("Cgroup allow %s perms=%d", QEMU_DEV_VFIO, VIR_CGROUP_DEVICE_RW);
@@ -473,13 +475,15 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
if (qemuDomainGetHostdevPath(dev, &path, NULL) < 0)
return -1;
- VIR_DEBUG("Cgroup deny %s", path);
- rv = virCgroupDenyDevicePath(priv->cgroup, path,
- VIR_CGROUP_DEVICE_RWM, false);
- virDomainAuditCgroupPath(vm, priv->cgroup,
- "deny", path, "rwm", rv);
- if (rv < 0)
- return -1;
+ if (path) {
+ VIR_DEBUG("Cgroup deny %s", path);
+ rv = virCgroupDenyDevicePath(priv->cgroup, path,
+ VIR_CGROUP_DEVICE_RWM, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup,
+ "deny", path, "rwm", rv);
+ if (rv < 0)
+ return -1;
+ }
if (qemuHostdevNeedsVFIO(dev) &&
!qemuDomainNeedsVFIO(vm->def)) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..1f358544ab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -14001,7 +14001,8 @@ qemuDomainNeedsVFIO(const virDomainDef *def)
*
* For given device @dev fetch its host path and store it at
* @path. Optionally, caller can get @perms on the path (e.g.
- * rw/ro).
+ * rw/ro). When called on a missing device, the function will return success
+ * and store NULL at @path.
*
* The caller is responsible for freeing the @path when no longer
* needed.
@@ -14625,7 +14626,7 @@ qemuDomainSetupHostdev(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,
if (qemuDomainGetHostdevPath(dev, &path, NULL) < 0)
return -1;
- if (qemuDomainCreateDevice(path, data, false) < 0)
+ if (path && qemuDomainCreateDevice(path, data, false) < 0)
return -1;
if (qemuHostdevNeedsVFIO(dev) &&
@@ -15688,7 +15689,7 @@ qemuDomainNamespaceSetupHostdev(virDomainObjPtr vm,
if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0)
return -1;
- if (qemuDomainNamespaceMknodPath(vm, path) < 0)
+ if (path && qemuDomainNamespaceMknodPath(vm, path) < 0)
return -1;
if (qemuHostdevNeedsVFIO(hostdev) &&
@@ -15720,7 +15721,7 @@ qemuDomainNamespaceTeardownHostdev(virDomainObjPtr vm,
if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0)
return -1;
- if (qemuDomainNamespaceUnlinkPath(vm, path) < 0)
+ if (path && qemuDomainNamespaceUnlinkPath(vm, path) < 0)
return -1;
if (qemuHostdevNeedsVFIO(hostdev) &&
--
2.24.1
4 years, 11 months
[libvirt] backing file chains, -blockdev, and storage file autodetection
by Martin Wilck
The recent change in libvirt to pass storage arguments to qemu via
"-blockdev", explicity passing backing file chain information rather
than relying on qemu to figure it out, has bitten me quite painfully.
I've had the habit for years to use qcow2 with backing chains without
specifying the backing file format explicitly, as information like the
following was enough both for me and for qemu to figure out that the
backing file was in qcow2 format:
# qemu-img info sles15-gm-cc.qcow2
image: sles15-gm-cc.qcow2
file format: qcow2
virtual size: 20 GiB (21474836480 bytes)
disk size: 407 MiB
cluster_size: 65536
backing file: /mnt/vms/sles15-gm-base.qcow2
But if libvirt encounters a qemu-img file like this, it passes the
backing file to qemu with "driver";"raw":
-blockdev '{"driver":"file","filename":"/mnt/vms/sles15-gm-base.qcow2","node-name":"libvirt-4-storage","auto-read-only":true,"discard":
"unmap"}' \
-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw","file":"libvirt-4-storage"}' \
The effect was that some of my VMs would refuse to start with a "no
bootable disk" error message from the VM's BIOS. Others did boot (I
believe this was because the boot sector had been modified in the top
image in the backing file chain), and I'm quite grateful I did not
attempt to boot them fully, because god knows what might have happened
if the OS had later encountered garbage data from the backing file at
some random point.
It took me half a day to figure out that this effect had been caused by
the recent libvirt update.
I'm aware that documentation about this has been added recently
(https://libvirt.org/kbase/backing_chains.html (*)). Also I believe
that current libvirt master (unlike 5.10.0) would refuse to start such
images in the first place (
https://bugzilla.redhat.com/show_bug.cgi?id=1588373), perhaps providing
users with a better clue than before what was going wrong.
Meanwhile I've fixed my VM images by adding the backing file format
tag, as suggested in the documentation. However I still think that this
was quite a disruptive change and highly unexpected for users. IMO the
default behavior shouldn't have been switched like this without
appropriate warnings.
The rationale given for not autodetecting the file format is "a
malicious guest could rewrite the header of the disk leading to access
of host files". I suppose a guest would need to manipulate a raw image
to look like qcow2, qed or similar for this to happen (and set the
backing file to "/etc/shadow", maybe?). Still the malicious guest would
need to find a way to manipulate the data *on the backing store*,
because the format of the topmost image is explicit anyway.
Modifying the backing store could be difficult for the guest, because
it's normally read-only and changes go only to the top layer. Or am I
missing something? The opposite (manipulating qcow2 to look like raw)
shouldn't be possible, IMO.
While I can't deny that an attack like this might be feasible, I am
still wondering why this hasn't been an issue in past years (with qemu
auto-detecting the backing file format).
More importantly, perhaps the disruption caused by this change could be
mitigated by allowing autodetection in certain cases (e.g. if the file
name of the backing file indicates it's qcow2, as in the example
above), or by providing a configuration option to enable it in
environments (like mine, developer test environment) where evil guests
are very unlikely.
Best regards,
Martin
(*) This page is quite hard to find, googling for "libvirt backing
chain" does not pick it up prominently just yet. Actually I only found
this information by running "git grep" on the libvirt git repo.
--
Dr. Martin Wilck <mwilck(a)suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
4 years, 11 months
[libvirt] [dockerfiles PATCH 0/2] openSUSE updates
by Andrea Bolognani
Pushed under the Dockerfile update rule.
Andrea Bolognani (2):
Update openSUSE Leap 15.1 to install Meson from pip
Add openSUSE Leap 15.1 Dockerfile for libosinfo
buildenv-libosinfo-opensuse-151.zip | Bin 0 -> 570 bytes
buildenv-libvirt-opensuse-151.zip | Bin 761 -> 785 bytes
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 buildenv-libosinfo-opensuse-151.zip
--
2.24.1
4 years, 11 months
