On a Thursday in 2020, Daniel P. Berrangé wrote:
The stub impl of virGetDeviceID just returns ENOSYS and does not
initialize the min/maj output parameters. This lead to a false
positive warning on mingw about possible use of uninitialized
variables.
Signed-off-by: Daniel P. Berrangé
---
src/util/virutil.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Thursday in 2020, Peter Krempa wrote:
'nfs' variable was set to -1 or -2 on agent failure. Cleanup then tried
to free 'nfs' elements of the array which resulted into a crash.
Make 'nfs' size_t and assign it only on successful agent call.
https://bugzilla.redhat.com/show_bug.cgi?id=1812965
Broken by commit 599ae372d8cf092
Signed-off-by: Peter Krempa
---
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_driver.c | 12
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 9f3fb9732f..dff327e8d5 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1914,7 +1914,7 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks,
return 0;
}
-/* Returns: 0 on success
+/* Returns: number of entries in '@info' on success
* -2 when agent command is not supported by the agent
* -1 otherwise
*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 02ea582767..e285e9373c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -22814,7 +22814,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
g_autofree char *hostname = NULL;
unsigned int supportedTypes = types;
int rc;
-int nfs = 0;
+size_t nfs = 0;
qemuAgentFSInfoPtr *agentfsinfo = NULL;
size_t i;
@@ -22867,9 +22867,13 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
Some separate issues:
The hostname call above also shares the same code path on unsupported
command and success, assigning NULL to the TypedParameter
Also, I'm confused about the 'types' semantics - info types unsupported
by libvirt (none so far, unless the caller passed in nonsensical values)
are quietly filtered out. But if a type was requested and the agent does
not support it, we error out without actually setting an error.
}
}
if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
-rc = nfs = qemuAgentGetFSInfo(agent, &agentfsinfo);
-if (rc < 0 && !(rc == -2 && types == 0))
-goto exitagent;
+rc = qemuAgentGetFSInfo(agent, &agentfsinfo);
+if (rc < 0) {
+if (!(rc == -2 && types == 0))
+goto exitagent;
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Thursday in 2020, Peter Krempa wrote:
The only caller doesn't check the value and also there are no real
errors to report anyways.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
We currently don't model the 'ssh' protocol properties properly and
since it seems impossible for now (agent path passed via environment
variable). To allow libguestfs to work as it used in pre-blockdev era we
must carry the properties over to the command line. For this instance we
just store it internally and format it back.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 10 ++
src/util/virstoragefile.c | 13 +
src/util/virstoragefile.h | 5 +
tests/qemublocktest.c | 1 +
.../jsontojson/ssh-passthrough-libguestfs-in.json | 1 +
.../jsontojson/ssh-passthrough-libguestfs-out.json | 14 ++
6 files changed, 44 insertions(+)
create mode 100644
tests/qemublocktestdata/jsontojson/ssh-passthrough-libguestfs-in.json
create mode 100644
tests/qemublocktestdata/jsontojson/ssh-passthrough-libguestfs-out.json
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Add testing of the interpretation of the JSON pseudo-protocol backing
store into JSON structs for blockdev. This will be used to test JSON
pseudo-URIs used by libguestfs while actually also validating the output
against the QMP schema. Since libguestfs uses obsolete/undocumented
values the outputs will differ and a benefit is that modern output is
used now.
The example test case covers the fields and values used by libguestfs
when using the https driver.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 65 +++
.../jsontojson/curl-libguestfs-in.json| 1 +
.../jsontojson/curl-libguestfs-out.json | 9 +++
3 files changed, 75 insertions(+)
create mode 100644 tests/qemublocktestdata/jsontojson/curl-libguestfs-in.json
create mode 100644 tests/qemublocktestdata/jsontojson/curl-libguestfs-out.json
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
The test was invoking the JSON formatter with the 'legacy' flag thus
formatting bunch of obsolete JSON blockdev definitions. We also should
test the modern ones. Add a boolean and re-run all the tests in both
cases.
Additionally for any modern invocation we should also validate that the
output conforms to the QAPI schema.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Move lookup of the schema root earlier so that multiple functions
can use it for validation.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
libguestfs abuses a quirk of qemu's parser to accept also other variants
of the 'sslverify' field which would be valid on the command line but
are not documented in the QMP schema.
If we encounter the 'off' string instead of an boolean handle it rather
than erroring out to continue support of pre-blockdev configurations.
Signed-off-by: Peter Krempa
---
src/util/virstoragefile.c | 21 ++---
tests/virstoragetest.c| 15 +++
2 files changed, 29 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Multiple tests require the schema. Extract the loading into a separate
variable to avoid issues with ownership of the pointer.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Pass the alias of the secret object holding the cookie data as
'cookie-secret' to qemu.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 14 +++---
.../disk-network-http.x86_64-latest.args | 11 +--
tests/qemuxml2argvdata/disk-network-http.xml | 8
3 files changed, 28 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Implement both commandline support and hotplug by adding the http cookie
handling to 'qemuBlockStorageSourceAttachData' handling functions for
it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 13 +
src/qemu/qemu_block.h | 3 +++
src/qemu/qemu_command.c | 5 +
3 files changed, 21 insertions(+)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
QEMU's curl driver requires the cookes concatenated and allows them
cookies
allows them to be
passed in via a secret. Prepare the value for the secret and encrypt it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
The http cookies can have potentially sensitive values and thus should
not be leaked into the command line. This means that we'll need to
instantiate a 'secret' object in qemu to pass the value encrypted.
This patch adds infrastructure for storing of the alias in the status
XML.t
t
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 8 +++-
src/qemu/qemu_domain.h| 3 +++
tests/qemustatusxml2xmldata/modern-in.xml | 1 +
3 files changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Upcoming patches will implement the support for sslverify, cookies,
readahead, and timeout properties. Add a test file which will collect
the cases.
Signed-off-by: Peter Krempa
---
.../disk-network-http.x86_64-latest.args | 57 +++
tests/qemuxml2argvdata/disk-network-http.xml | 50
tests/qemuxml2argvtest.c | 1 +
3 files changed, 108 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-network-http.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/disk-network-http.xml
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Ensure that the new fields are allowed only when -blockdev is used or
when they are in the detected part of the backing chain where qemu will
handle them internally.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 55 ++
1 file changed, 55 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1d551f248f..e7aaded4d5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
+
+if (src->readahead > 0) {
+if (!src->detected &&
Is this supported for non-network sources?
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("readahead setting is not supported by this QEMU
binary"));
Either way - readahead in QEMU's curl backend seems to be there for a
long time now. "supported with this QEMU binary" would be more accurate phrasing
+return -1;
+}
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Some disk backends support configuring the readahead buffer or timeout
for requests. Add the knobs to the XML.
Signed-off-by: Peter Krempa
---
docs/formatdomain.html.in | 16 +
docs/schemas/domaincommon.rng | 23 +++
src/conf/domain_conf.c| 19 +++
src/util/virstoragefile.c | 2 ++
src/util/virstoragefile.h | 3 +++
.../disk-network-http.xml | 2 ++
6 files changed, 65 insertions(+)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On 3/12/20 4:39 PM, no-re...@patchew.org wrote:
Patchew URL:
https://patchew.org/QEMU/20200312192822.3739399-1-ebl...@redhat.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC block/replication.o
CC block/throttle.o
CC block/copy-on-read.o
/tmp/qemu-test/src/block/sheepdog.c:2174:9: error: variable 'qdict' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (backing_fmt && strcmp(backing_fmt, "sheepdog") != 0) {
^~~
/tmp/qemu-test/src/block/sheepdog.c:2241:19: note: uninitialized use occurs here
---
^
= NULL
Bah, real problem (and I missed it because I compiled for debug, while
this error depends on -O2 for gcc to flag it). Squash this in:
diff --git i/block/sheepdog.c w/block/sheepdog.c
index 376f4ef74638..e0ea335131d9 100644
--- i/block/sheepdog.c
+++ w/block/sheepdog.c
@@ -2161,9 +2161,9 @@ static int coroutine_fn sd_co_create_opts(const
char *filename, QemuOpts *opts,
Error **errp)
{
BlockdevCreateOptions *create_options = NULL;
-QDict *qdict, *location_qdict;
+QDict *qdict = NULL, *location_qdict;
Visitor *v;
-char *redundancy;
+char *redundancy = NULL;
Error *local_err = NULL;
int ret;
char *backing_fmt = NULL;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On 3/11/20 1:20 PM, Michal Privoznik wrote:
The virQEMUCaps structure has usedQMP member which in the past
used to tell if qemu we are dealing with is capable of QMP. Well,
we don't support HMP anymore (minus a few HMP passthrough
commands, which are wrapped into QMP anyways) and the member is
not used really.
Signed-off-by: Michal Privoznik
---
Reviewed-by: Daniel Henrique Barboza
Patchew URL:
https://patchew.org/QEMU/20200312192822.3739399-1-ebl...@redhat.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC block/replication.o
CC block/throttle.o
CC block/copy-on-read.o
/tmp/qemu-test/src/block/sheepdog.c:2174:9: error: variable 'qdict' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (backing_fmt && strcmp(backing_fmt, "sheepdog") != 0) {
^~~
/tmp/qemu-test/src/block/sheepdog.c:2241:19: note: uninitialized use occurs here
---
^
= NULL
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/sheepdog.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=93997e5b5eb34c2f9cb85363713a7f85', '-u',
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e',
'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e',
'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache',
'-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-7l8c91wu/src/docker-src.2020-03-12-17.35.51.6480:/var/tmp/qemu:z,ro',
'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=93997e5b5eb34c2f9cb85363713a7f85
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-7l8c91wu/src'
make: *** [docker-run-test-debug@fedora] Error 2
real3m14.071s
user0m8.555s
The full log is available at
http://patchew.org/logs/20200312192822.3739399-1-ebl...@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Patchew URL:
https://patchew.org/QEMU/20200312192822.3739399-1-ebl...@redhat.com/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
In file included from /tmp/qemu-test/src/include/qapi/qmp/qdict.h:16,
from /tmp/qemu-test/src/block/sheepdog.c:20:
/tmp/qemu-test/src/block/sheepdog.c: In function 'sd_co_create_opts':
/tmp/qemu-test/src/include/qapi/qmp/qobject.h:99:13: error: 'qdict' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
if (obj && --obj->base.refcnt == 0) {
^~
/tmp/qemu-test/src/block/sheepdog.c:2164:12: note: 'qdict' was declared here
QDict *qdict, *location_qdict;
^
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/sheepdog.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=2bc4557fecf04aae88a65433b16d7773', '-u',
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=',
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e',
'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v',
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-yqol9ujz/src/docker-src.2020-03-12-17.44.12.28207:/var/tmp/qemu:z,ro',
'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=2bc4557fecf04aae88a65433b16d7773
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-yqol9ujz/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real1m54.790s
user0m8.304s
The full log is available at
http://patchew.org/logs/20200312192822.3739399-1-ebl...@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Patchew URL:
https://patchew.org/QEMU/20200312192822.3739399-1-ebl...@redhat.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
In file included from /tmp/qemu-test/src/include/qapi/qmp/qdict.h:16:0,
from /tmp/qemu-test/src/block/sheepdog.c:20:
/tmp/qemu-test/src/block/sheepdog.c: In function 'sd_co_create_opts':
/tmp/qemu-test/src/include/qapi/qmp/qobject.h:98:29: error: 'qdict' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
assert(!obj || obj->base.refcnt);
^
/tmp/qemu-test/src/block/sheepdog.c:2164:12: note: 'qdict' was declared here
---
^
cc1: all warnings being treated as errors
CC scsi/pr-manager.o
make: *** [block/sheepdog.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=42390fc6cae14dfc8db84779948e6236', '-u',
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=',
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e',
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v',
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-bmmmjgbx/src/docker-src.2020-03-12-17.41.49.21365:/var/tmp/qemu:z,ro',
'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=42390fc6cae14dfc8db84779948e6236
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-bmmmjgbx/src'
make: *** [docker-run-test-quick@centos7] Error 2
real1m47.430s
user0m7.951s
The full log is available at
http://patchew.org/logs/20200312192822.3739399-1-ebl...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
On 3/12/20 1:01 PM, Peter Krempa wrote:
See patch 2/2
Peter Krempa (2):
qemuAgentFSInfoFormatParams: Remove pointless returned value
qemuDomainGetGuestInfo: Don't try to free a negative number of entries
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_driver.c | 43 --
2 files changed, 21 insertions(+), 24 deletions(-)
Reviewed-by: Daniel Henrique Barboza
Creating an image that requires format probing of the backing image is
inherently unsafe (we've had several CVEs over the years based on
probes leaking information to the guest on a subsequent boot, although
these days tools like libvirt are aware of the issue enough to prevent
the worst effects). However, if our probing algorithm ever changes,
or if other tools like libvirt determine a different probe result than
we do, then subsequent use of that backing file under a different
format will present corrupted data to the guest. Start a deprecation
clock so that future qemu-img can refuse to create unsafe backing
chains that would rely on probing. The warnings are intentionally
emitted from the block layer rather than qemu-img (thus, all paths
into image creation or rewriting perform the check).
However, there is one time where probing is safe: if we probe raw,
then it is safe to record that implicitly in the image (but we still
warn, as it's better to teach the user to supply -F always than to
make them guess when it is safe).
iotest 114 specifically wants to create an unsafe image for later
amendment rather than defaulting to our new default of recording a
probed format, so it needs an update. While touching it, expand it to
cover all of the various warnings enabled by this patch. iotest 290
also shows a change to qcow messages; note that the fact that we now
make a probed format of 'raw' explicit now results in a double
warning, but no one should be creating new qcow images so it is not
worth cleaning up.
Signed-off-by: Eric Blake
---
docs/system/deprecated.rst | 19 +++
block.c| 21 -
qemu-img.c | 2 +-
tests/qemu-iotests/114 | 11 +++
tests/qemu-iotests/114.out | 8
tests/qemu-iotests/290.out | 5 -
6 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 4261d5589e6a..b541d52c7dc0 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -382,6 +382,25 @@ image). Rather, any changes to the backing chain should
be performed
with ``qemu-img rebase -u`` either before or after the remaining
changes being performed by amend, as appropriate.
+qemu-img backing file without format (since 5.0.0)
+''
+
+The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img
+convert`` to create or modify an image that depends on a backing file
+now recommends that an explicit backing format be provided. This is
+for safety: if qemu probes a different format than what you thought,
+the data presented to the guest will be corrupt; similarly, presenting
+a raw image to a guest allows a potential security exploit if a future
+probe sees a non-raw image based on guest writes. To avoid the
+warning message, or even future refusal to create an unsafe image, you
+must pass ``-o backing_fmt=`` (or the shorthand ``-F`` during create)
+to specify the intended backing format. You may use ``qemu-img rebase
+-u`` to retroactively add a backing format to an existing image.
+However, be aware that there are already potential security risks to
+blindly using ``qemu-img info`` to probe the format of an untrusted
+backing image, when deciding what format to add into an existing
+image.
+
``qemu-img convert -n -o`` (since 4.2.0)
diff --git a/block.c b/block.c
index cd3277ab86fd..69955f53f35a 100644
--- a/block.c
+++ b/block.c
@@ -6064,6 +6064,20 @@ void bdrv_img_create(const char *filename, const char
*fmt,
"Could not open backing image to determine
size.\n");
goto out;
} else {
+if (!backing_fmt) {
+warn_report("Deprecated use of backing file without explicit "
+"backing format (detected format of %s)",
+bs->drv->format_name);
+if (bs->drv == &bdrv_raw) {
+/*
+ * A probe of raw is always correct, so in this one
+ * case, we can write that into the image.
+ */
+backing_fmt = bs->drv->format_name;
+qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, backing_fmt,
+ NULL);
+}
+}
if (size == -1) {
/* Opened BS, have no size */
size = bdrv_getlength(bs);
@@ -6077,7 +6091,12 @@ void bdrv_img_create(const char *filename, const char
*fmt,
}
bdrv_unref(bs);
}
-} /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */
+/* (backing_file && !(flags & BDRV_O_NO_BACKING)) */
+} else if (backing_file && !backing_fmt) {
+warn_report("Deprecated use of unopened backing file without "
+"explicit backing format, use of this
Sheepdog already requires that if backing_file is present, that it be
another sheepdog image (see sd_co_create). Meanwhile, we want to move
towards always being explicit about the backing format for other
drivers where it matters. So for convenience, make qemu-img create -F
sheepdog work, while rejecting all other explicit formats (note that
this is only for QemuOpts usage; there is no change to the QAPI to
allow a format through -blockdev).
Signed-off-by: Eric Blake
---
block/sheepdog.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index cfa84338a2d6..376f4ef74638 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2166,8 +2166,16 @@ static int coroutine_fn sd_co_create_opts(const char
*filename, QemuOpts *opts,
char *redundancy;
Error *local_err = NULL;
int ret;
+char *backing_fmt = NULL;
redundancy = qemu_opt_get_del(opts, BLOCK_OPT_REDUNDANCY);
+backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+
+if (backing_fmt && strcmp(backing_fmt, "sheepdog") != 0) {
+error_setg(errp, "backing_file must be a sheepdog image");
+ret = -EINVAL;
+goto fail;
+}
qdict = qemu_opts_to_qdict(opts, NULL);
qdict_put_str(qdict, "driver", "sheepdog");
@@ -2232,6 +2240,7 @@ fail:
qapi_free_BlockdevCreateOptions(create_options);
qobject_unref(qdict);
g_free(redundancy);
+g_free(backing_fmt);
return ret;
}
@@ -3189,6 +3198,11 @@ static QemuOptsList sd_create_opts = {
.type = QEMU_OPT_STRING,
.help = "File name of a base image"
},
+{
+.name = BLOCK_OPT_BACKING_FMT,
+.type = QEMU_OPT_STRING,
+.help = "Must be 'sheepdog' if present",
+},
{
.name = BLOCK_OPT_PREALLOC,
.type = QEMU_OPT_STRING,
--
2.25.1
vmdk already requires that if backing_file is present, that it be
another vmdk image (see vmdk_co_do_create). Meanwhile, we want to
move towards always being explicit about the backing format for other
drivers where it matters. So for convenience, make qemu-img create -F
vmdk work, while rejecting all other explicit formats (note that this
is only for QemuOpts usage; there is no change to the QAPI to allow a
format through -blockdev).
Signed-off-by: Eric Blake
---
block/vmdk.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index 20e909d99794..3c7893817afe 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2611,6 +2611,14 @@ static int coroutine_fn vmdk_co_create_opts(const char
*filename, QemuOpts *opts
bool zeroed_grain;
bool compat6;
VMDKCreateOptsData data;
+char *backing_fmt = NULL;
+
+backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+if (backing_fmt && strcmp(backing_fmt, "vmdk") != 0) {
+error_setg(errp, "backing_file must be a vmdk image");
+ret = -EINVAL;
+goto exit;
+}
if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) {
ret = -EINVAL;
@@ -2669,6 +2677,7 @@ static int coroutine_fn vmdk_co_create_opts(const char
*filename, QemuOpts *opts
vmdk_co_create_opts_cb, &data, errp);
exit:
+g_free(backing_fmt);
g_free(adapter_type);
g_free(backing_file);
g_free(hw_version);
@@ -3005,6 +3014,11 @@ static QemuOptsList vmdk_create_opts = {
.type = QEMU_OPT_STRING,
.help = "File name of a base image"
},
+{
+.name = BLOCK_OPT_BACKING_FMT,
+.type = QEMU_OPT_STRING,
+.help = "Must be 'vmdk' if present",
+},
{
.name = BLOCK_OPT_COMPAT6,
.type = QEMU_OPT_BOOL,
--
2.25.1
qcow has no space in the metadata to store a backing format, and there
are existing qcow images backed both by raw or by other formats
(usually qcow) images, reliant on probing to tell the difference.
While we don't recommend the creation of new qcow images (as qcow2 is
hands-down better), we can at least insist that if the user does
request a specific format without using -u, then it must be non-raw
(as a raw backing file that gets inadvertently edited into some other
format can form a security hole); if the user does not request a
specific format or lies when using -u, then the status quo of probing
for the backing format remains intact (although an upcoming patch will
warn when omitting a format request). Thus, when this series is
complete, the only way to use a backing file for qcow without
triggering a warning is when using -F if the backing file is non-raw
to begin with. Note that this is only for QemuOpts usage; there is no
change to the QAPI to allow a format through -blockdev.
Add a new iotest 290 just for qcow, to demonstrate the new warning.
Signed-off-by: Eric Blake
---
block/qcow.c | 16 -
tests/qemu-iotests/290 | 72 ++
tests/qemu-iotests/290.out | 42 ++
tests/qemu-iotests/group | 1 +
4 files changed, 130 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/290
create mode 100644 tests/qemu-iotests/290.out
diff --git a/block/qcow.c b/block/qcow.c
index fce89898681f..47c6a40342a6 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -939,11 +939,12 @@ static int coroutine_fn qcow_co_create_opts(const char
*filename,
{
BlockdevCreateOptions *create_options = NULL;
BlockDriverState *bs = NULL;
-QDict *qdict;
+QDict *qdict = NULL;
Visitor *v;
const char *val;
Error *local_err = NULL;
int ret;
+char *backing_fmt;
static const QDictRenames opt_renames[] = {
{ BLOCK_OPT_BACKING_FILE, "backing-file" },
@@ -952,6 +953,13 @@ static int coroutine_fn qcow_co_create_opts(const char
*filename,
};
/* Parse options and convert legacy syntax */
+backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+if (backing_fmt && !strcmp(backing_fmt, "raw")) {
+error_setg(errp, "qcow cannot store backing format; an explicit "
+ "backing format of raw is unsafe");
+ret = -EINVAL;
+goto fail;
+}
qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qcow_create_opts, true);
val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT);
@@ -1017,6 +1025,7 @@ static int coroutine_fn qcow_co_create_opts(const char
*filename,
ret = 0;
fail:
+g_free(backing_fmt);
qobject_unref(qdict);
bdrv_unref(bs);
qapi_free_BlockdevCreateOptions(create_options);
@@ -1151,6 +1160,11 @@ static QemuOptsList qcow_create_opts = {
.type = QEMU_OPT_STRING,
.help = "File name of a base image"
},
+{
+.name = BLOCK_OPT_BACKING_FMT,
+.type = QEMU_OPT_STRING,
+.help = "Format of the backing image (caution: raw backing is
unsafe)",
+},
{
.name = BLOCK_OPT_ENCRYPT,
.type = QEMU_OPT_BOOL,
diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
new file mode 100755
index ..8482de58cb4b
--- /dev/null
+++ b/tests/qemu-iotests/290
@@ -0,0 +1,72 @@
+#!/usr/bin/env bash
+#
+# Test qcow backing file warnings
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see .
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow
+_supported_proto file
+_supported_os Linux
+
+size=128M
+
+echo
+echo "== qcow backed by qcow =="
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+_img_info
+_make_test_img -b "$TEST_IMG.base" -F $IMGFMT
+_img_info
+
+echo
+echo "== mismatched command line detection =="
+
+_make_test_img -b "$TEST_IMG.base" -F vmdk
+# Use of -u bypasses the backing format sanity check
+_make_test_img -u -b "$TEST_IMG.base" -F vmdk $size
+_img_info
+
+echo
+echo "== qcow backed
The use of 'qemu-img amend' to change qcow2 backing files is not
tested very well. In particular, our implementation has a bug where
if a new backing file is provided without a format, then the prior
format is blindly reused, even if this results in data corruption, but
this is not caught by iotests.
There are also situations where amending other options needs access to
the original backing file (for example, on a downgrade to a v2 image,
knowing whether a v3 zero cluster must be allocated or may be left
unallocated depends on knowing whether the backing file already reads
as zero), but the command line does not have a nice way to tell us
both the backing file to use for opening the image as well as the
backing file to install after the operation is complete.
Even if we do allow changing the backing file, it is redundant with
the existing ability to change backing files via 'qemu-img rebase -u'.
It is time to deprecate this support (leaving the existing behavior
intact, even if it is buggy), and at a point in the future, require
the use of only 'qemu-img rebase' for adjusting backing chain
relations, saving 'qemu-img amend' for changes unrelated to the
backing chain.
Signed-off-by: Eric Blake
---
docs/system/deprecated.rst | 12
docs/tools/qemu-img.rst| 4
block/qcow2.c | 5 +
tests/qemu-iotests/061.out | 1 +
tests/qemu-iotests/082.out | 2 ++
5 files changed, 24 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 0838338d8fbe..4261d5589e6a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -370,6 +370,18 @@ The above, converted to the current supported format::
Related binaries
+qemu-img amend to adjust backing file (since 5.0.0)
+'''
+
+The use of ``qemu-img amend`` to modify the name or format of a qcow2
+backing image is deprecated; this functionality was never fully
+documented or tested, and interferes with other amend operations that
+need access to the original backing image (such as deciding whether a
+v3 zero cluster may be left unallocated when converting to a v2
+image). Rather, any changes to the backing chain should be performed
+with ``qemu-img rebase -u`` either before or after the remaining
+changes being performed by amend, as appropriate.
+
``qemu-img convert -n -o`` (since 4.2.0)
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76c9..83d57586be96 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -254,6 +254,10 @@ Command description:
Amends the image format specific *OPTIONS* for the image file
*FILENAME*. Not all file formats support this operation.
+ The set of options that can be amended are dependent on the image
+ format, but note that amending the backing chain relationship should
+ instead be performed with ``qemu-img rebase``.
+
.. option:: bench [-c COUNT] [-d DEPTH] [-f FMT]
[--flush-interval=FLUSH_INTERVAL] [-i AIO] [-n] [--no-drain] [-o OFFSET]
[--pattern=PATTERN] [-q] [-s BUFFER_SIZE] [-S STEP_SIZE] [-t CACHE] [-w] [-U]
FILENAME
Run a simple sequential I/O benchmark on the specified image. If ``-w`` is
diff --git a/block/qcow2.c b/block/qcow2.c
index d44b45633dbb..97a1757156bb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5281,6 +5281,11 @@ static int qcow2_amend_options(BlockDriverState *bs,
QemuOpts *opts,
}
if (backing_file || backing_format) {
+if (g_strcmp0(backing_file, s->image_backing_file) ||
+g_strcmp0(backing_format, s->image_backing_format)) {
+warn_report("Deprecated use of amend to alter the backing file; "
+"use qemu-img rebase instead");
+}
ret = qcow2_change_backing_file(bs,
backing_file ?: s->image_backing_file,
backing_format ?: s->image_backing_format);
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 8b3091a412bc..3bf37b526f8b 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -342,6 +342,7 @@ wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: warning: Deprecated use of amend to alter the backing file; use
qemu-img rebase instead
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
No errors were found on the image.
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 9d4ed4dc9d61..94ea990a2754 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -806,10 +806,12 @@ Creation options for 'qcow2':
Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
+qemu-img: warning: De
The stub impl of virGetDeviceID just returns ENOSYS and does not
initialize the min/maj output parameters. This lead to a false
positive warning on mingw about possible use of uninitialized
variables.
Signed-off-by: Daniel P. Berrangé
---
src/util/virutil.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index f77ee05dbf..ddd8805101 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1407,9 +1407,10 @@ virGetDeviceID(const char *path, int *maj, int *min)
#else
int
virGetDeviceID(const char *path G_GNUC_UNUSED,
- int *maj G_GNUC_UNUSED,
- int *min G_GNUC_UNUSED)
+ int *maj,
+ int *min)
{
+*maj = *min = 0;
return -ENOSYS;
}
#endif
--
2.24.1
The qemuMonitorTestNew() function returns with the monitor object
locked, and expects it to still be locked when qemuMonitorTestFree
is called. The qemuhotplug test, however, explicitly unlocks the
monitor, but then forgets to lock it again. As a result the
qemuMonitorTestFree function is unlocking a mutex it doesn't own.
This bug has existed forever, but since we use normal POSIX mutexes
and don't check the return value of pthread_mutex_lock/unlock we
didn't see the error. It was harmless until the switch to the per
monitor event loop which requires the thread synchronization to
work reliably, whereupon it started crashing.
Signed-off-by: Daniel P. Berrangé
---
tests/qemuhotplugtest.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 8b411d63f0..d9244dca44 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -337,6 +337,8 @@ testQemuHotplug(const void *data)
ret = testQemuHotplugUpdate(vm, dev);
}
+virObjectLock(priv->mon);
+
cleanup:
VIR_FREE(domain_filename);
VIR_FREE(device_filename);
@@ -378,6 +380,7 @@ static void
testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData *data)
{
qemuDomainObjPrivatePtr priv;
+qemuMonitorPtr mon;
if (!data)
return;
@@ -396,6 +399,8 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData
*data)
virObjectUnref(data->vm);
}
+mon = qemuMonitorTestGetMonitor(data->mon);
+virObjectLock(mon);
qemuMonitorTestFree(data->mon);
VIR_FREE(data);
}
--
2.24.1
On Wed, 2020-03-11 at 09:53 +, Daniel P. Berrangé wrote:
> On Tue, Mar 10, 2020 at 07:25:46PM +0100, Andrea Bolognani wrote:
> > In your scenario, when you don't specify a scope you get the same
> > one as the primary driver is using (this matches the current
> > behavior): so if you are using qemu:///session, every
> > will use network:///session unless you explicitly specify
> > scope='system', at which point network:///system will be used; in
> > the same fashion, if you're connected to qemu:///embed, the default
> > for s should be network:///embed, with the possibility
> > to use either network:///session (with scope='session') or, most
> > likely, network:///system (with scope='system').
>
> No, I'm not talking about using the same URI for the secondary
> drivers, I'm talking about using the same *privilege* level.
> ie, qemu:///system and a qemu:///embed running as root should
> both use the privileged network/storage driver. The qemu:///session
> and qemu:///embed running as non-root should both default to
> the unprivileged network/storage drivers.
What's the advantage of conflating URI and privilege level? It seems
to me like it only makes things complicated when they could be
absolutely straightforward instead.
In fact, I believe libguestfs developers have long lamented the fact
that it's not really possible to have qemu:///session for the root
user, which is caused by the same kind of logic... It would be
preferable, I think, not to introduce even more instances of it.
> > > With such functionality present, it logically then also extends to
> > > cover connections to daemons running in different namespaces.
> >
> > I'm still unclear on how this scenario, which would apparently have
> > multiple eg. privileged virtnetworkd, running at the same time but in
> > different network namespaces, would work.
>
> There would need to be some selection method, most likely a way
> to explicitly specify the socket path, but this is a longish way
> into the future.
Got it!
--
Andrea Bolognani / Red Hat / Virtualization
On 3/11/20 12:12 PM, Nikolay Shirokovskiy wrote:
On 11.03.2020 12:38, Michal Privoznik wrote:
On 3/5/20 3:47 PM, Nikolay Shirokovskiy wrote:
Sync was introduced in [1] to check for ga presence. This
check is racy but in the era before serial events are available
there was not better solution I guess.
Can you elaborate more on the raciness? There should never be more than one
thread talking on the agent monitor.
The race is in another dimension) So sync checks for GA presence, it only waits
for 5s so if
there is no GA the actual command with no timeout won't block. But GA can go
down right
after successful sync so command can block. This is true if there are no serial
events
in the latter case agent monitor will be closed when GA go down and command
will unblock.
In case we have the events the sync function is different. It allows us
to flush stateless ga channel from remnants of previous communications.
But we need to do it only once. Until we get timeout on issued command
channel state is ok.
[1] qemu_agent: Issue guest-sync prior to every command
Signed-off-by: Nikolay Shirokovskiy
---
src/qemu/qemu_agent.c | 13 -
src/qemu/qemu_agent.h | 3 ++-
src/qemu/qemu_process.c | 3 ++-
tests/qemumonitortestutils.c | 3 ++-
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index cd25ef6cd3..2de53efb7a 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -104,6 +104,8 @@ struct _qemuAgent {
int watch;
bool running;
+ bool singleSync;
+ bool inSync;
I wonder if we can do this with just inSync and not have singleSync. I mean, it
looks like at all places where singleSync is used we have access to qemuCaps so
it should be as easy as:
qemuAgentOpen();
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) {
if *qemuAgentGuestSync(mon) < 0)
goto error;
mon->inSync = true;
and then qemuagentGuestSync() would be a NOP if inSync is set. But it would
also not set it. But maybe I'm pushing it too far, it's just that I'm confused
by the name 'singleSync'. Probably 'hasSingleSync' would be better? The boolean
reflect whether qemu has the VSERPORT_CHANGE capability and thus libvirt knows
when GA connects/disconnects. 'singleSync' just doesn't ring that bell at first.
If we try to sync only on monitor opening then we can't sync if for example
command timeouts and
GA stays connect (thus no serial event and monitor won't be closed). With
current patch we just
try to sync when we need to send next command to GA.
I guess singleSync is better. It just reflects that we don't need to sync
before every
command (in order to avoid hangs) in case there is serial events. Strictly
speaking
it is not correct to use sync to check for GA presence because of the race and
as to
using it as a means to flush channel it works well without sereal events as
well.
Nikolay
Alright, you've persuaded me on both.
Reviewed-by: Michal Privoznik
and resolved the merge commit on both patches that appeared meanwhile
and pushed.
Michal
On a Monday in 2020, Peter Krempa wrote:
To allow turning of verification of SSL cerificates add a new element
turning off
to the disk source XML which will allow configuring the validation
process using the 'verify' attribute.
Signed-off-by: Peter Krempa
---
docs/formatdomain.html.in | 9
docs/schemas/domaincommon.rng | 51 ++-
src/conf/domain_conf.c| 18 +++
src/util/virstoragefile.c | 1 +
src/util/virstoragefile.h | 1 +
.../disk-network-http.xml | 9
6 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7e7771725c..8f503f6967 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2857,6 +2857,7 @@
+
@@ -3383,6 +3384,14 @@
The offset and size values are in bytes.
Since 6.1.0
+ ssl
+
+For https and ftps accessed storage it's
+possible to tweak the SSL transport parameters with this element.
+The verify attribute allows to turn on or of SSL
or off
+certificate validation. Supported values are yes and
+no. Since 6.1.0
6.2.0
+
@@ -24531,6 +24545,10 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
virStorageSourceInitiatorFormatXML(&src->initiator, childBuf);
+if (src->sslverify != VIR_TRISTATE_BOOL_ABSENT)
+virBufferAsprintf(childBuf, "\n",
+ virTristateBoolTypeToString(src->sslverify));
+
Multi-line body without braces.
return 0;
}
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
There are two last callers of this function. Replace them by
qemuAliasForSecret and delete qemuDomainGetSecretAESAlias.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_alias.c| 29 -
src/qemu/qemu_alias.h| 3 ---
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_migration_params.c | 2 +-
4 files changed, 2 insertions(+), 34 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Originally there was only the secret for authentication so we didn't use
any suffix to tell it apart. With the introduction of encryption we
added a 'luks' suffix for the encryption secrets. Since encryption is
really generic and authentication is not the only secret modify the
aliases for the secrets to better describe what they are used for.
This is possible as we store the disk secrets in the status XML thus
only new machines will use the new secrets.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 4 +--
...-backing-chains-noindex.x86_64-2.12.0.args | 4 +--
...-backing-chains-noindex.x86_64-latest.args | 6 ++--
...sk-hostdev-scsi-virtio-iscsi-auth-AES.args | 6 ++--
.../disk-network-iscsi.x86_64-2.12.0.args | 12 +++
.../disk-network-iscsi.x86_64-latest.args | 8 ++---
.../disk-network-rbd.x86_64-2.12.0.args | 4 +--
.../disk-network-rbd.x86_64-latest.args | 4 +--
...isk-network-source-auth.x86_64-2.12.0.args | 10 +++---
...isk-network-source-auth.x86_64-latest.args | 8 ++---
.../disk-nvme.x86_64-latest.args | 4 +--
.../encrypted-disk-usage.args | 4 +--
tests/qemuxml2argvdata/encrypted-disk.args| 4 +--
.../luks-disks-source-qcow2.args | 24 +++---
...luks-disks-source-qcow2.x86_64-latest.args | 32 +--
tests/qemuxml2argvdata/luks-disks-source.args | 26 ---
tests/qemuxml2argvdata/luks-disks.args| 10 +++---
tests/qemuxml2argvdata/user-aliases.args | 4 +--
18 files changed, 90 insertions(+), 84 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Monday in 2020, Peter Krempa wrote:
Replace qemuDomainGetSecretAESAlias by the new function si that we can
s/si/so/
reuse qemuDomainSecretAESSetupFromSecret also for setting up other kinds
of objects.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
See patch 2/2
Peter Krempa (2):
qemuAgentFSInfoFormatParams: Remove pointless returned value
qemuDomainGetGuestInfo: Don't try to free a negative number of entries
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_driver.c | 43 --
2 files changed, 21 insertions(+), 24 deletions(-)
--
2.24.1
On a Monday in 2020, Peter Krempa wrote:
Currently we don't have infrastructure to remember the secret aliases
for hostdevs. Since an upcomming patch is going to change aliases for
upcoming
the disks, initialize the iscsi hostdevs separately so that we can keep
the alias. At the same time let's use qemuAliasForSecret instead of
qemuDomainGetSecretAESAlias when unplugging the iscsi hostdev.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 25 +++--
src/qemu/qemu_hotplug.c | 2 +-
2 files changed, 24 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On 09/03/20 10:32, Zhang, Chen wrote:
> 4. Implementation issue.
>
> The AWD script as an optional feature is OK for me.
>
> And report the triggering of the watchdog via QMP events is enough for
> current usage.
>
> But it looks have limitation to notify outside Qemu. I don't know which
> is better choice.
>
> If the QMP events solution is better, I will fix it in next version.
Good, thanks.
Naming-wise, it's ugly that we already have a WATCHDOG event for guest
watchdog devices. The following design however should allow setting up
multiple watchdogs
- Creating a watchdog from the command line:
-object watchdog,id=STR,timeout=NNN,chardev=CHR
and object_add/object-add can also be used for HMP and QMP.
- Reporting a watchdog timeout via QMP:
{ 'event': 'WATCHDOG_TIMEOUT',
'data': { 'id': 'str' } }
- Protocol: the data sent on the chardev to QEMU must be
WATCHDOG=1
optionally followed by exactly one \n character. All other data is ignored.
Paolo
On a Monday in 2020, Peter Krempa wrote:
The naming of the variables was tied to what they are used for not what
the alias represents. Since we'll need to use some of the aliases for
another type of secrets fix the name so that it makes sense.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Wednesday in 2020, LAN BAI wrote:
On Mar 11, 2020 9:30 AM, Ján Tomko wrote:
On a Sunday in 2020, Lan wrote:
One of the BiteSizedTasks
Introduce src/util/virdaemon.c/h files
Introduce a new function virDaemonSetupLogging (src/util/virdaemon.c)
for shared code in
virLockDaemonSetupLogging (src/locking/lock_daemon.c)
virLogDaemonSetupLogging (src/logging/log_daemon.c)
daemonSetupLogging (src/remote/remote_daemon.c)
Introduce virDaemonLogConfig struct for config->log_* variables in
virLockDaemonConfig/virLogDaemonConfig/daemonConfig struct
Signed-off-by: Lan Bai
---
src/libvirt_private.syms | 2 +
src/locking/lock_daemon.c | 58 ++---
src/logging/log_daemon.c | 50 ++---
src/remote/remote_daemon.c | 57 ++---
src/util/Makefile.inc.am | 4 +-
src/util/virdaemon.c | 75 ++
src/util/virdaemon.h | 35 ++
7 files changed, 126 insertions(+), 155 deletions(-)
create mode 100644 src/util/virdaemon.c
create mode 100644 src/util/virdaemon.h
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index de0c7a3133..50cbd6d7af 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1906,6 +1906,8 @@ virCryptoHashBuf;
virCryptoHashString;
virCryptoHaveCipher;
+# util/virdaemon.h
+virDaemonSetupLogging;
# util/virdbus.h
virDBusCallMethod;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 5e5a0c1089..5ba851cb55 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -46,6 +46,7 @@
#include "virstring.h"
#include "virgettext.h"
#include "virenum.h"
+#include "virdaemon.h"
#include "locking/lock_daemon_dispatch.h"
#include "locking/lock_protocol.h"
@@ -477,59 +478,6 @@ virLockDaemonErrorHandler(void *opaque G_GNUC_UNUSED,
}
-/*
- * Set up the logging environment
- * By default if daemonized all errors go to the logfile libvirtd.log,
- * but if verbose or error debugging is asked for then also output
- * informational and debug messages. Default size if 64 kB.
- */
-static int
-virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
- bool privileged,
- bool verbose,
- bool godaemon)
-{
-virLogReset();
-
-/*
- * Libvirtd's order of precedence is:
- * cmdline > environment > config
- *
- * Given the precedence, we must process the variables in the opposite
- * order, each one overriding the previous.
- */
-if (config->log_level != 0)
-virLogSetDefaultPriority(config->log_level);
-
-/* In case the config is empty, both filters and outputs will become empty,
- * however we can't start with empty outputs, thus we'll need to define and
- * setup a default one.
- */
-ignore_value(virLogSetFilters(config->log_filters));
-ignore_value(virLogSetOutputs(config->log_outputs));
-
-/* If there are some environment variables defined, use those instead */
-virLogSetFromEnv();
-
-/*
- * Command line override for --verbose
- */
-if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
-virLogSetDefaultPriority(VIR_LOG_INFO);
-
-/* Define the default output. This is only applied if there was no setting
- * from either the config or the environment.
- */
-virLogSetDefaultOutput("virtlockd", godaemon, privileged);
-
-if (virLogGetNbOutputs() == 0)
-virLogSetOutputs(virLogGetDefaultOutput());
-
-return 0;
-}
-
-
-
/* Display version information. */
static void
virLockDaemonVersion(const char *argv0)
@@ -1186,7 +1134,9 @@ int main(int argc, char **argv) {
}
VIR_FREE(remote_config_file);
-if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) {
+if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
This still does not compile for me:
../../src/locking/lock_daemon.c:1131:31: error: cast from 'unsigned int *' to
'virDaemonLogConfigPtr' (aka 'struct _virDaemonLogConfig *') increases required
alignment from 4 to 8 [-Werror,-Wcast-align]
if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
^
This is modified in following commits. If you apply the whole patch it
shouldn't be a problem.
The first step along with introducing the virDaemonLogConfig structure
is moving the three log_parameters into this structure.
So
struct _virLockDaemonConfig {
unsigned int log_level;
char *log_filters;
char *log_outputs;
unsigned int max_clients;
unsigned int admin_max_clients;
};
would become something like
struct _virLockDaemonConfig {
virDaemonLogConfig log_config;
unsigned int max_clients;
unsigned int admin_max_clients;
};
And a function like:
virDaemonLogConfigLoadOptions(virDaemonLogConfigPtr log_config, virConfPtr conf)
could be used to replace th
The updateLive CPU sub-driver function is supposed to be called only for
a subset of CPU definitions. Let's make it more obvious by turning a
negative test and return into a positive check.
Signed-off-by: Jiri Denemark
---
src/cpu/cpu.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 6d6191fe4e..c461c4839d 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -647,13 +647,14 @@ virCPUUpdateLive(virArch arch,
if (!driver->updateLive)
return 1;
-if (cpu->mode != VIR_CPU_MODE_CUSTOM)
-return 1;
+if (cpu->mode == VIR_CPU_MODE_CUSTOM) {
+if (driver->updateLive(cpu, dataEnabled, dataDisabled) < 0)
+return -1;
-if (driver->updateLive(cpu, dataEnabled, dataDisabled) < 0)
-return -1;
+return 0;
+}
-return 0;
+return 1;
}
--
2.25.1
On Thu, Mar 12, 2020 at 01:50:49PM +0100, Andrea Bolognani wrote:
> On Thu, 2020-03-12 at 12:09 +, Daniel P. Berrangé wrote:
> > On Thu, Mar 12, 2020 at 12:57:36PM +0100, Andrea Bolognani wrote:
> > > Honestly, so far I haven't been able to figure out the use case for
> > > registering libvirt VMs with machined either :)
> > >
> > > Most of the operations are either not supported (login, shell, start)
> > > or do not work as expected (list --all, reboot), so all you can
> > > really do is list the subset of libvirt VMs that happen to be running
> > > and power them off. I can't really imagine that being very useful to
> > > anyone... Am I missing something?
> >
> > Yeah, pretty much all you get is a way to report & terminate VMs via
> > systemd commands. A few others things could be wired up, but no one
> > ever made an effort todo so and I don't think it is worth it.
> >
> > So I'm getting inclined to consider machined a failed experiment from
> > POV of VMs - still makes sense for containers. That said I'd still
> > keep using it, because we need systemd to deal with cgroups creation
> > no matter what, and its no worse to talk to systemd via machined
> > than directly.
>
> Would it make sense to default to not registering with machined? It
> would probably be more honest of us, considering how severely limited
> the functionality is... Set expectations right and all that. The fact
> that not even reboot, one of the only three operations available
> through machinectl, works correctly (it will shut down the VM instead
> of restarting it) leads me to believe that nobody is actually using
> this anyway.
>
> > > Of course it's not about secrecy, but for the same reasons
> > > qemu:///embed VMs don't show up in the output of 'virsh list' it
> > > also makes sense for them to be omitted from that of 'machinectl
> > > list', I think.
> >
> > Yes, I agree with this.
> >
> > The only even slightly plausible use case for machinectl to list
> > a full set of guest OS running on the host. This just about makes
> > sense for traditional data center / cloud virt use case. I don't
> > think it makes sense when KVM is merely used as an infrastructure
> > building block embedded in applications. As such, I think we should
> > NOT register with machined or systemd at all, for embedded VMs, without
> > an explicit opt-in. We should flip to just inheriting the calling
> > processes cgroups context, to align with the goal that embedded driver
> > should generally aim to inherit all process context.
>
> Cool.
>
> Let's just make sure there is a way for qemu:///embed users to
> explicitly opt-in into qemu:///system-style CGroup handling,
> hopefully without machined getting in the way, before we flip the
> switch. Are you going to revive the old patch you linked to a few
> day ago?
Dunno how well it will still apply, but yeah, something approximately
the same as that would suit us
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
On Thu, 2020-03-12 at 12:09 +, Daniel P. Berrangé wrote:
> On Thu, Mar 12, 2020 at 12:57:36PM +0100, Andrea Bolognani wrote:
> > Honestly, so far I haven't been able to figure out the use case for
> > registering libvirt VMs with machined either :)
> >
> > Most of the operations are either not supported (login, shell, start)
> > or do not work as expected (list --all, reboot), so all you can
> > really do is list the subset of libvirt VMs that happen to be running
> > and power them off. I can't really imagine that being very useful to
> > anyone... Am I missing something?
>
> Yeah, pretty much all you get is a way to report & terminate VMs via
> systemd commands. A few others things could be wired up, but no one
> ever made an effort todo so and I don't think it is worth it.
>
> So I'm getting inclined to consider machined a failed experiment from
> POV of VMs - still makes sense for containers. That said I'd still
> keep using it, because we need systemd to deal with cgroups creation
> no matter what, and its no worse to talk to systemd via machined
> than directly.
Would it make sense to default to not registering with machined? It
would probably be more honest of us, considering how severely limited
the functionality is... Set expectations right and all that. The fact
that not even reboot, one of the only three operations available
through machinectl, works correctly (it will shut down the VM instead
of restarting it) leads me to believe that nobody is actually using
this anyway.
> > Of course it's not about secrecy, but for the same reasons
> > qemu:///embed VMs don't show up in the output of 'virsh list' it
> > also makes sense for them to be omitted from that of 'machinectl
> > list', I think.
>
> Yes, I agree with this.
>
> The only even slightly plausible use case for machinectl to list
> a full set of guest OS running on the host. This just about makes
> sense for traditional data center / cloud virt use case. I don't
> think it makes sense when KVM is merely used as an infrastructure
> building block embedded in applications. As such, I think we should
> NOT register with machined or systemd at all, for embedded VMs, without
> an explicit opt-in. We should flip to just inheriting the calling
> processes cgroups context, to align with the goal that embedded driver
> should generally aim to inherit all process context.
Cool.
Let's just make sure there is a way for qemu:///embed users to
explicitly opt-in into qemu:///system-style CGroup handling,
hopefully without machined getting in the way, before we flip the
switch. Are you going to revive the old patch you linked to a few
day ago?
--
Andrea Bolognani / Red Hat / Virtualization
On Thu, Mar 12, 2020 at 12:57:36PM +0100, Andrea Bolognani wrote:
> On Wed, 2020-03-11 at 17:32 +0100, Michal Privoznik wrote:
> > I still don't quite see the value in machinectl (maybe because I'm not
> > using systemd :-D)
>
> Honestly, so far I haven't been able to figure out the use case for
> registering libvirt VMs with machined either :)
>
> Most of the operations are either not supported (login, shell, start)
> or do not work as expected (list --all, reboot), so all you can
> really do is list the subset of libvirt VMs that happen to be running
> and power them off. I can't really imagine that being very useful to
> anyone... Am I missing something?
Yeah, pretty much all you get is a way to report & terminate VMs via
systemd commands. A few others things could be wired up, but no one
ever made an effort todo so and I don't think it is worth it.
So I'm getting inclined to consider machined a failed experiment from
POV of VMs - still makes sense for containers. That said I'd still
keep using it, because we need systemd to deal with cgroups creation
no matter what, and its no worse to talk to systemd via machined
than directly.
> > but anyway - it's a system-wide monitor of virtual
> > machines. Therefore it makes sense to register a domain started under
> > qemu:///embed there. I don't view embed mode as a way of starting VMs
> > secretly. It's a way of starting VMs privately and that's a different
> > thing. Other users might learn that my app is running a VM (plain 'ps'
> > would give it away), but they can not mangle with it in any way, e.g.
> > change its XML.
>
> Of course it's not about secrecy, but for the same reasons
> qemu:///embed VMs don't show up in the output of 'virsh list' it
> also makes sense for them to be omitted from that of 'machinectl
> list', I think.
Yes, I agree with this.
The only even slightly plausible use case for machinectl to list
a full set of guest OS running on the host. This just about makes
sense for traditional data center / cloud virt use case. I don't
think it makes sense when KVM is merely used as an infrastructure
building block embedded in applications. As such, I think we should
NOT register with machined or systemd at all, for embedded VMs, without
an explicit opt-in. We should flip to just inheriting the calling
processes cgroups context, to align with the goal that embedded driver
should generally aim to inherit all process context.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
On Wed, 2020-03-11 at 17:32 +0100, Michal Privoznik wrote:
> I still don't quite see the value in machinectl (maybe because I'm not
> using systemd :-D)
Honestly, so far I haven't been able to figure out the use case for
registering libvirt VMs with machined either :)
Most of the operations are either not supported (login, shell, start)
or do not work as expected (list --all, reboot), so all you can
really do is list the subset of libvirt VMs that happen to be running
and power them off. I can't really imagine that being very useful to
anyone... Am I missing something?
> but anyway - it's a system-wide monitor of virtual
> machines. Therefore it makes sense to register a domain started under
> qemu:///embed there. I don't view embed mode as a way of starting VMs
> secretly. It's a way of starting VMs privately and that's a different
> thing. Other users might learn that my app is running a VM (plain 'ps'
> would give it away), but they can not mangle with it in any way, e.g.
> change its XML.
Of course it's not about secrecy, but for the same reasons
qemu:///embed VMs don't show up in the output of 'virsh list' it
also makes sense for them to be omitted from that of 'machinectl
list', I think.
It's easier to make the case for virsh because the expectation is
that, if they show up, you can perform various operations such as
edit or dumpxml or whatever on them; for machinectl, since the only
operation that seems to work is poweroff anyway, and that's
apparently achieved not by calling libvirt APIs but by killing the
QEMU process directly, then the case is not as obvious - but I think
the same rationale still applies.
> > > > Right now we're already doing
> > > >
> > > >qemu-$domid-$domname
> > > >
> > > > where I'm not entirely sure how much $domid actually buys us.
> > >
> > > IIRC $domid was a hack because at one time we had problems with
> > > systemd not cleaning up the transient scope when QEMU was killed.
> > > This would prevent libvirt starting the guest again thereafter.
> > > I can't remember now if this was a bug we fixed in systemd or
> > > libvirt or both or neither.
>
> It was introduced by bd773e74f0d1d1b9ebbfcaa645178316b4f2265c and the
> commit message links to this bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=68370 which looks like
> fixed in systemd.
That doesn't look right.
>From what I can gather, $domid was added much more recently,
specifically in c3bd0019c0e3f080dbf0d4bd08245ffb2daa2765, in order
to addresses https://bugzilla.redhat.com/show_bug.cgi?id=1282846.
While I get the overall idea behind the changes, I don't quite
understand the rationale behind adding $domid specifically...
> > I see! It would be neat if we could get rid of it, assuming of course
> > it's no longer needed on the platforms we target.
>
> I don't think it's that simple. Machinectl poses some limitations on the
> name: either it has to be a FQDN or a simple name without any dots. And
> according to our code the strlen() must be <= 64 (don't know if that
> comes from machinectl or is just our own limitation). Therefore, if you
> have two domains which names would clash after our processing, the
> domain ID guarantees unique strings are passed to machined.
... but here it is! Makes sense now :)
[...]
> > > There's no question that we must fix the machined problem.
>
> I will try to post patches for this.
That'd be awesome! Thanks in advance :)
--
Andrea Bolognani / Red Hat / Virtualization
Thank you, Daniel !
I appreciate that.
Signed-off-by: Zhang Bo
Signed-off-by: Wu Qingliang
> -Original Message-
> From: Daniel P. Berrangé [mailto:berra...@redhat.com]
> Sent: Thursday, March 12, 2020 12:22 AM
> To: Zhangbo (Oscar)
> Cc: libvir-list@redhat.com; dengkai (A) ; wujing (O)
> ; wuqingliang
> Subject: Re: [PATCHv2 0/5] update tls files without restarting libvirtd
>
> On Sat, Mar 07, 2020 at 07:30:59PM +0800, Zhang Bo wrote:
> > v1:
> > https://www.redhat.com/archives/libvir-list/2020-February/msg00370.htm
> > l
> >
> > v2:
> > according to Dienial's suggestion:
> > * update each tls file one time -> update all of them at one time
> > * forced to re-create the credentials object, rather than allowing
> > to append to the original ones.
>
> Aside from some minor mistakes this all looks fine code wise.
>
> The commits, however, are missing the signed-off-by statement.
> This is required to indicate that you agree to the contribution policy at:
>
> https://developercertificate.org/
>
> Assuming you're find with this, just reply to this mail with a Signed-off-by:
> YOUR NAME and I'll add this to the commit messages &
> push to git with the minor fixes.
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
On Thu, Mar 12, 2020 at 15:17:32 +0800, Zhenyu Ye wrote:
> QEMU has added support for io_uring IO mode, see:
>
> https://git.qemu.org/git/qemu.git/ adcd6e93.
>
> This patch add support for io_uring in libvirt.
>
> Signed-off-by: Zhenyu Ye
> ---
> src/conf/domain_conf.c | 1 +
> src/conf/domain_conf.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d2d97daf80..5ced2f3b6b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -361,6 +361,7 @@ VIR_ENUM_IMPL(virDomainDiskIo,
>"default",
>"native",
>"threads",
> + "io_uring",
> );
>
> VIR_ENUM_IMPL(virDomainDeviceSGIO,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 91b776c28a..905fdecab3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -421,6 +421,7 @@ typedef enum {
> VIR_DOMAIN_DISK_IO_DEFAULT = 0,
> VIR_DOMAIN_DISK_IO_NATIVE,
> VIR_DOMAIN_DISK_IO_THREADS,
> +VIR_DOMAIN_DISK_IO_URING,
>
> VIR_DOMAIN_DISK_IO_LAST
> } virDomainDiskIo;
Hi,
this unfortunately is far from complete.
You'll need to provide:
- XML schema additions (docs/schemas/domaincommon.rng)
- documentation (docs/formatdomain.html.in)
- a qemu capability flag to check whether it's supported:
(src/qemu/qemu_capabilities.c/h), you can use the QAPI capability
query string to check it:
blockdev-add/arg-type/+file/aio/^io_uring
- a validation entry to reject config if the above capability is not
present (qemuDomainValidateStorageSource src/qemu/qemu_domain.c)
- a test case for at least qemuxml2argv and qemuxml2xmltest, enhancing
existing test files is okay (tests/qemuxml2argvdata), the test case
must show that the appropriate qemu command line is generated
Please split the above additions to patches at least:
- src/conf/ and docs/ additions
- src/qemu/qemu_capabilities additions
- validation additions
- tests
and please follow the contributor guidelines. Specifically the part tha
the whole tree must compile cleanly after every patch so the changes
must be split properly.
