[PATCH] bhyve: add support
Implement "" support for bhyve driver.
As there are not really lot of options, try to find
"BHYVE_UEFI.fd" firmware which is installed by the
sysutils/uefi-edk2-bhyve FreeBSD port.
If not found, just use the first found firmware
in the firmwares directory (which is configurable via
config file).
Signed-off-by: Roman Bogorodskiy
---
Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob,
but not sure how to test this otherwise.
po/POTFILES.in| 1 +
src/bhyve/bhyve_domain.c | 5 +
src/bhyve/bhyve_firmware.c| 91 +++
src/bhyve/bhyve_firmware.h| 30 ++
src/bhyve/bhyve_process.c | 15 +++
src/bhyve/bhyve_process.h | 5 +
src/bhyve/meson.build | 1 +
.../bhyvexml2argv-firmware-efi.args | 11 +++
.../bhyvexml2argv-firmware-efi.ldargs | 1 +
.../bhyvexml2argv-firmware-efi.xml| 22 +
tests/bhyvexml2argvtest.c | 83 ++---
11 files changed, 254 insertions(+), 11 deletions(-)
create mode 100644 src/bhyve/bhyve_firmware.c
create mode 100644 src/bhyve/bhyve_firmware.h
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 80c5f145be..413783ee35 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -14,6 +14,7 @@
@SRCDIR@src/bhyve/bhyve_command.c
@SRCDIR@src/bhyve/bhyve_domain.c
@SRCDIR@src/bhyve/bhyve_driver.c
+@SRCDIR@src/bhyve/bhyve_firmware.c
@SRCDIR@src/bhyve/bhyve_monitor.c
@SRCDIR@src/bhyve/bhyve_parse_command.c
@SRCDIR@src/bhyve/bhyve_process.c
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 8fbc554a0a..209e4d3905 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
if (def->os.bootloader == NULL && def->os.loader)
return true;
+if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI)
+return true;
+
if (def->nserials || def->nconsoles)
return true;
@@ -230,6 +233,8 @@ virDomainDefParserConfig
virBhyveDriverDomainDefParserConfig = {
.domainPostParseCallback = bhyveDomainDefPostParse,
.assignAddressesCallback = bhyveDomainDefAssignAddresses,
.deviceValidateCallback = bhyveDomainDeviceDefValidate,
+
+.features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT,
};
static void
diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
new file mode 100644
index 00..ccc3a5ffc8
--- /dev/null
+++ b/src/bhyve/bhyve_firmware.c
@@ -0,0 +1,91 @@
+/*
+ * bhyve_firmware.c: bhyve firmware management
+ *
+ * Copyright (C) 2021 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *
Re: [PATCH RFC 0/3] Add checkpoint/restore support to LXC using CRIU
Hi guys, I marked this series as RFC to discuss some points. I'm interested in enhancing this specific part of LXC. So, some questions that I would like to hear as a feedback from community: 1. I decided to use a tar to compress all CRIU img files into a single file. Any other suggestions? 2. If no is the answer to question above, is there a consensus on preferring to use command line calls or libraries? I would like to use libtar for instance. I personally think that this approach is ugly. Not sure if I'm able to do that. The same for CRIU. 3. Other important opinions obviously. -- Julio Cesar Faracco Em sáb., 27 de fev. de 2021 às 01:06, Julio Faracco escreveu: > > This patch series implements a way to do checkpoint/restore to LXC driver > using > CRIU operations. This respects the other methods to save and restore processes > states: using a file with a header with some metadata. The only difference > here > is basically the way LXC drivers join the files produced by CRIU. CRIU > generates > a lots of 'img' files and it is compresses using TAR to fit into the libvirt > state file. > > Julio Faracco (3): > meson: Add support to CRIU binary into meson > lxc: Including CRIU functions and functions to support C/R. > lxc: Adding support to LXC driver to restore a container > > meson.build | 10 + > meson_options.txt| 1 + > src/lxc/lxc_conf.c | 3 + > src/lxc/lxc_conf.h | 2 + > src/lxc/lxc_container.c | 188 +- > src/lxc/lxc_container.h | 3 +- > src/lxc/lxc_controller.c | 93 - > src/lxc/lxc_criu.c | 405 +++ > src/lxc/lxc_criu.h | 50 + > src/lxc/lxc_driver.c | 341 +++- > src/lxc/lxc_process.c| 26 ++- > src/lxc/lxc_process.h| 1 + > src/lxc/meson.build | 2 + > 13 files changed, 1106 insertions(+), 19 deletions(-) > create mode 100644 src/lxc/lxc_criu.c > create mode 100644 src/lxc/lxc_criu.h > > -- > 2.27.0 >
[PATCH RFC 3/3] lxc: Adding support to LXC driver to restore a container
This patch introduces the hability to restore a saved container using CRIU.
It should be possible to start it using traditional methods: a simple container
start; or from a saved state.
Signed-off-by: Julio Faracco
---
src/lxc/lxc_conf.c | 3 +
src/lxc/lxc_conf.h | 2 +
src/lxc/lxc_container.c | 188 -
src/lxc/lxc_container.h | 3 +-
src/lxc/lxc_controller.c | 93 ++-
src/lxc/lxc_driver.c | 341 ++-
src/lxc/lxc_process.c| 26 ++-
src/lxc/lxc_process.h| 1 +
8 files changed, 638 insertions(+), 19 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index e6ad91205e..690cef7d39 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -240,6 +240,8 @@ virLXCDriverConfigNew(void)
cfg->logDir = g_strdup(LXC_LOG_DIR);
cfg->autostartDir = g_strdup(LXC_AUTOSTART_DIR);
+cfg->saveImageFormat = NULL;
+
return cfg;
}
@@ -291,4 +293,5 @@ virLXCDriverConfigDispose(void *obj)
g_free(cfg->stateDir);
g_free(cfg->logDir);
g_free(cfg->securityDriverName);
+g_free(cfg->saveImageFormat);
}
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 664bafc7b9..08bea11c02 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -61,6 +61,8 @@ struct _virLXCDriverConfig {
char *securityDriverName;
bool securityDefaultConfined;
bool securityRequireConfined;
+
+char *saveImageFormat;
};
struct _virLXCDriver {
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 2a5f8711c4..03c086d029 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -51,6 +51,7 @@
#include "virerror.h"
#include "virlog.h"
#include "lxc_container.h"
+#include "lxc_criu.h"
#include "viralloc.h"
#include "virnetdevveth.h"
#include "viruuid.h"
@@ -84,6 +85,7 @@ struct __lxc_child_argv {
char **ttyPaths;
int handshakefd;
int *nsInheritFDs;
+int restorefd;
};
static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
@@ -235,8 +237,8 @@ static virCommandPtr
lxcContainerBuildInitCmd(virDomainDefPtr vmDef,
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcContainerSetupFDs(int *ttyfd,
-size_t npassFDs, int *passFDs)
+static int lxcContainerSetupFDs(int *ttyfd, size_t npassFDs,
+int *passFDs, int restorefd)
{
int rc = -1;
int open_max;
@@ -335,6 +337,10 @@ static int lxcContainerSetupFDs(int *ttyfd,
for (fd = last_fd + 1; fd < open_max; fd++) {
int tmpfd = fd;
+
+if (tmpfd == restorefd)
+continue;
+
VIR_MASS_CLOSE(tmpfd);
}
@@ -1017,6 +1023,30 @@ static int lxcContainerMountFSDevPTS(virDomainDefPtr def,
return 0;
}
+
+static int lxcContainerMountFSDevPTSRestore(virDomainDefPtr def,
+const char *stateDir)
+{
+int flags = MS_MOVE;
+g_autofree char *path = NULL;
+
+VIR_DEBUG("Mount /dev/pts stateDir=%s", stateDir);
+
+path = g_strdup_printf("%s/%s.devpts", stateDir, def->name);
+
+VIR_DEBUG("Trying to move %s to /dev/pts", path);
+
+if (mount(path, "/dev/pts", NULL, flags, NULL) < 0) {
+virReportSystemError(errno,
+ _("Failed to mount %s on /dev/pts"),
+ path);
+return -1;
+}
+
+return 0;
+}
+
+
static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths)
{
size_t i;
@@ -1843,6 +1873,139 @@ static int lxcAttachNS(int *ns_fd)
return 0;
}
+
+/*
+ * lxcContainerChildRestore:
+ * @data: pointer to container arguments
+ */
+static int lxcContainerChildRestore(void *data)
+{
+lxc_child_argv_t *argv = data;
+virDomainDefPtr vmDef = argv->config;
+int ttyfd = -1;
+int ret = -1;
+virDomainFSDefPtr root;
+g_autofree char *ttyPath = NULL;
+g_autofree char *sec_mount_options = NULL;
+g_autofree char *stateDir = NULL;
+g_autofree char *rootfs_mount = NULL;
+
+if (NULL == vmDef) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("lxcChild() passed invalid vm definition"));
+goto cleanup;
+}
+
+if (lxcContainerWaitForContinue(argv->monitor) < 0) {
+virReportSystemError(errno, "%s",
+ _("Failed to read the container continue
message"));
+goto cleanup;
+}
+VIR_DEBUG("Received container continue message");
+
+if (lxcContainerSetID(vmDef) < 0)
+goto cleanup;
+
+root = virDomainGetFilesystemForTarget(vmDef, "/");
+
+if (argv->nttyPaths) {
+const char *tty = argv->ttyPaths[0];
+
+if (STRPREFIX(tty, "/dev/pts/"))
+tty += strlen("/dev/pts/");
+
+ttyPath = g_strdup_printf("%s/%s.devpts/%s",
+ LXC_STATE_DIR, vmDef->name, tty);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+
[PATCH RFC 0/3] Add checkpoint/restore support to LXC using CRIU
This patch series implements a way to do checkpoint/restore to LXC driver using CRIU operations. This respects the other methods to save and restore processes states: using a file with a header with some metadata. The only difference here is basically the way LXC drivers join the files produced by CRIU. CRIU generates a lots of 'img' files and it is compresses using TAR to fit into the libvirt state file. Julio Faracco (3): meson: Add support to CRIU binary into meson lxc: Including CRIU functions and functions to support C/R. lxc: Adding support to LXC driver to restore a container meson.build | 10 + meson_options.txt| 1 + src/lxc/lxc_conf.c | 3 + src/lxc/lxc_conf.h | 2 + src/lxc/lxc_container.c | 188 +- src/lxc/lxc_container.h | 3 +- src/lxc/lxc_controller.c | 93 - src/lxc/lxc_criu.c | 405 +++ src/lxc/lxc_criu.h | 50 + src/lxc/lxc_driver.c | 341 +++- src/lxc/lxc_process.c| 26 ++- src/lxc/lxc_process.h| 1 + src/lxc/meson.build | 2 + 13 files changed, 1106 insertions(+), 19 deletions(-) create mode 100644 src/lxc/lxc_criu.c create mode 100644 src/lxc/lxc_criu.h -- 2.27.0
[PATCH RFC 2/3] lxc: Including CRIU functions and functions to support C/R.
This patch adds the source code of helper functions into files
lxc_criu.{h,c} to support LXC checkpoint/restore using CRIU binary. To save
container state, LXC follows the same pattern of QEMU and libxl using a file
with a header with metadata, but as CRIU saves multiple files, it needs to
inserted in a unique file using a type of compression. Using TAR for
instance.
Signed-off-by: Julio Faracco
---
src/lxc/lxc_criu.c | 405
src/lxc/lxc_criu.h | 50 ++
src/lxc/meson.build | 2 +
3 files changed, 457 insertions(+)
create mode 100644 src/lxc/lxc_criu.c
create mode 100644 src/lxc/lxc_criu.h
diff --git a/src/lxc/lxc_criu.c b/src/lxc/lxc_criu.c
new file mode 100644
index 00..a82bd5ffde
--- /dev/null
+++ b/src/lxc/lxc_criu.c
@@ -0,0 +1,405 @@
+/*
+ * lxc_criu.c: wrapper functions for CRIU C API to be used for lxc migration
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *
[PATCH RFC 1/3] meson: Add support to CRIU binary into meson
This patch includes CRIU binary checks into meson files to support
checkpoint/restore for LXC driver.
Signed-off-by: Julio Faracco
---
meson.build | 10 ++
meson_options.txt | 1 +
2 files changed, 11 insertions(+)
diff --git a/meson.build b/meson.build
index 369548f127..115c903ab2 100644
--- a/meson.build
+++ b/meson.build
@@ -1639,6 +1639,15 @@ void main(void) {
if cc.compiles(lxc_get_free_code)
conf.set('WITH_DECL_LOOP_CTL_GET_FREE', 1)
endif
+
+ if not get_option('criu').disabled()
+criu_prog = find_program('criu')
+
+if criu_prog.found()
+ conf.set('WITH_CRIU', 1)
+ conf.set_quoted('CRIU', criu_prog.path())
+endif
+ endif
elif get_option('driver_lxc').enabled()
error('linux and remote_driver are required for LXC')
endif
@@ -2411,6 +2420,7 @@ misc_summary = {
'virt-login-shell': conf.has('WITH_LOGIN_SHELL'),
'virt-host-validate': conf.has('WITH_HOST_VALIDATE'),
'TLS priority': conf.get_unquoted('TLS_PRIORITY'),
+ 'CRIU': conf.has('WITH_CRIU'),
}
summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ')
diff --git a/meson_options.txt b/meson_options.txt
index e5d79c2b6b..de977c8775 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -102,3 +102,4 @@ option('numad', type: 'feature', value: 'auto',
description: 'use numad to manag
option('pm_utils', type: 'feature', value: 'auto', description: 'use pm-utils
for power management')
option('sysctl_config', type: 'feature', value: 'auto', description: 'Whether
to install sysctl configs')
option('tls_priority', type: 'string', value: 'NORMAL', description: 'set the
default TLS session priority string')
+option('criu', type: 'feature', value: 'auto', description: 'use CRIU to
checkpoint/restore containers')
--
2.27.0
Re: [PATCH v2 31/31] qom: Drop QemuOpts based interfaces
On 2/24/21 7:52 AM, Kevin Wolf wrote: > user_creatable_add_opts() has only a single user left, which is a test > case. Rewrite the test to use user_creatable_add_type() instead (which > is the remaining function that doesn't require a QAPI schema) and drop > the QemuOpts related functions. > > Signed-off-by: Kevin Wolf > --- > include/qom/object_interfaces.h | 59 > qom/object_interfaces.c | 81 - > tests/check-qom-proplist.c | 42 - > 3 files changed, 20 insertions(+), 162 deletions(-) Yay! Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 30/31] vl: QAPIfy -object
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This switches the system emulator from a QemuOpts-based parser for > -object to user_creatable_parse_str() which uses a keyval parser and > enforces the QAPI schema. > > Apart from being a cleanup, this makes non-scalar properties accessible. > > This adopts a similar model as -blockdev uses: When parsing the option, > create the ObjectOptions and queue them. At the later point where we > used to create objects for the collected QemuOpts, the ObjectOptions > queue is processed instead. > > A complication compared to -blockdev is that object definitions are > supported in -readconfig and -writeconfig. > > After this patch, -readconfig still works, though it still goes through > the QemuOpts parser, which means that improvements like non-scalar > properties are still not available in config files. > > -writeconfig stops working for -object. Tough luck. It has never > supported all options (not even the common ones), so supporting one less > isn't the end of the world. As object definitions from -readconfig still > go through QemuOpts, they are still included in -writeconfig output, > which at least prevents destroying your existing configuration when you > just wanted to add another option. And Paolo has submitted a patch deprecating it. > > Signed-off-by: Kevin Wolf > --- > softmmu/vl.c | 109 +++ > 1 file changed, 84 insertions(+), 25 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 29/31] qom: Add user_creatable_parse_str()
On 2/24/21 7:52 AM, Kevin Wolf wrote: > The system emulator has a more complicated way of handling command line > options in that it reorders options before it processes them. This means > that parsing object options and creating the object happen at two > different points. Split the parsing part into a separate function that > can be reused by the system emulator command line. > > Signed-off-by: Kevin Wolf > --- > include/qom/object_interfaces.h | 15 +++ > qom/object_interfaces.c | 20 ++-- > 2 files changed, 29 insertions(+), 6 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 28/31] hmp: QAPIfy object_add
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This switches the HMP command object_add from a QemuOpts-based parser to > user_creatable_add_from_str() which uses a keyval parser and enforces > the QAPI schema. > > Apart from being a cleanup, this makes non-scalar properties and help > accessible. In order for help to be printed to the monitor instead of > stdout, the printf() calls in the help functions are changed to > qemu_printf(). > > Signed-off-by: Kevin Wolf > --- > monitor/hmp-cmds.c | 17 ++--- > qom/object_interfaces.c | 11 ++- > hmp-commands.hx | 2 +- > 3 files changed, 9 insertions(+), 21 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 27/31] qom: Add user_creatable_add_from_str()
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This is a version of user_creatable_process_cmdline() with an Error > parameter that never calls exit() and is therefore usable in HMP. > > Signed-off-by: Kevin Wolf > --- > include/qom/object_interfaces.h | 16 > qom/object_interfaces.c | 29 - > 2 files changed, 40 insertions(+), 5 deletions(-) > > +/** > + * user_creatable_add_from_str: > + * @optarg: the object definition string as passed on the command line > + * @errp: if an error occurs, a pointer to an area to store the error > + * > + * Create an instance of the user creatable object by parsing optarg > + * with a keyval parser and implicit key 'qom-type', converting the > + * result to ObjectOptions and calling into qmp_object_add(). > + * > + * If a help option is given, print help instead. > + * > + * Returns: true when an object was successfully created, false when an error > + * occurred (*errp is set then) or help was printed (*errp is not set). > + */ > +bool user_creatable_add_from_str(const char *optarg, Error **errp); This could be used to fix the exit status 2 issue in qemu-img convert, if you rearrange the series a bit. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 26/31] qemu-nbd: Use user_creatable_process_cmdline() for --object
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This switches qemu-nbd from a QemuOpts-based parser for --object to > user_creatable_process_cmdline() which uses a keyval parser and enforces > the QAPI schema. > > Apart from being a cleanup, this makes non-scalar properties accessible. > > Signed-off-by: Kevin Wolf > --- > qemu-nbd.c | 34 +++--- > 1 file changed, 3 insertions(+), 31 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 25/31] qemu-img: Use user_creatable_process_cmdline() for --object
On 2/26/21 3:56 PM, Eric Blake wrote:
> On 2/24/21 7:52 AM, Kevin Wolf wrote:
>> This switches qemu-img from a QemuOpts-based parser for --object to
>> user_creatable_process_cmdline() which uses a keyval parser and enforces
>> the QAPI schema.
>>
>> Apart from being a cleanup, this makes non-scalar properties accessible.
>>
>> Signed-off-by: Kevin Wolf
>> ---
>> qemu-img.c | 239 -
>> 1 file changed, 33 insertions(+), 206 deletions(-)
>>
>
>> @@ -1423,15 +1373,9 @@ static int img_compare(int argc, char **argv)
>> case 'U':
>> force_share = true;
>> break;
>> -case OPTION_OBJECT: {
>> -QemuOpts *opts;
>> -opts = qemu_opts_parse_noisily(_object_opts,
>> - optarg, true);
>> -if (!opts) {
>> -ret = 2;
>> -goto out4;
>
> Our exit status here of 2 on failure appears to be intentional (since we
> reserve 0 for identical, 1 for mismatch, >1 for error)...
>
>> -}
>> -} break;
>> +case OPTION_OBJECT:
>> +user_creatable_process_cmdline(optarg);
>> +break;
>
> ...but becomes 1 here. Does that matter?
>
> /me goes and tests...
>
> Ouch: with current qemu.git master and none of this series applied:
>
> $ ./qemu-img compare --object foo,id=x /dev/null /dev/null
> qemu-img: invalid object type: foo
> $ echo $?
> 1
Okay, that didn't do what I expected, but this does:
$ ./qemu-img compare --object foo,id=1 /dev/null /dev/null
qemu-img: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
$ echo $?
2
> $ gdb --args ./qemu-img compare --object foo,id=x /dev/null /dev/null
> (gdb) b qemu_opts_pars
> (gdb) r
> (gdb) fin
> Run till exit from #0 qemu_opts_parse_noisily (
> list=0x5578f020 , params=0x7fffd8a8
> "foo,id=x",
> permit_abbrev=true) at ../util/qemu-option.c:948
> 0x555805f9 in img_compare (argc=5, argv=0x7fffd480)
> at ../qemu-img.c:1428
> 1428 opts = qemu_opts_parse_noisily(_object_opts,
> Value returned is $1 = (QemuOpts *) 0x5583b4b0
> (gdb) p *opts
> $3 = {id = 0x557a0d58 "`\264\203UUU", list = 0x51,
and this may be my confusion with gdb. Right after 'fin', *opts is not
the same as *$1 (apparently gdb has stopped at a point where the 'opts'
currently in scope is not the opts set by qemu_opts_parse_noisily, but
before the opts in scope has actually been assigned the returned value).
>
> That looks buggy. qemu_opts_parse_noisily() is NOT returning NULL, but
> rather a pointer to something garbage (that id pointing to a garbage
> string in the middle of qemu_trace_opts is fishy), and so we've been
> exiting with status 1 in spite of the code.
>
> Looks like we'll want a separate patch fixing that first.
So I was wrong on when qemu_opts_parse_noisily() returns NULL - it does
NOT reject unknown object names (that was the job of the
qemu_opts_foreach call later), but merely rejects bad/duplicate ids.
Thus this code was indeed giving an exit status of 2 when actually
triggered correctly,
>
>> case OPTION_IMAGE_OPTS:
>> image_opts = true;
>> break;
>> @@ -1450,13 +1394,6 @@ static int img_compare(int argc, char **argv)
>> filename1 = argv[optind++];
>> filename2 = argv[optind++];
>>
>> -if (qemu_opts_foreach(_object_opts,
>> - user_creatable_add_opts_foreach,
>> - qemu_img_object_print_help, _fatal)) {
>> -ret = 2;
>> -goto out4;
>
> Same deal with return value. Except here we used _fatal (which
> forces an exit status of 1 rather than returning), and so never even
> reach the ret=2 code. Looks like we broke that in commit 334c43e2c3,
> where we used to pass NULL instead of _fatal (although that commit
> was in turn fixing another problem).
...and THIS spot is why my original attempt to prove that your code was
causing a regression was seeing an exit status of 1, where I instead
ended up proving that we already regressed.
>
> The rest of this patch looks fine, although maybe
> user_creatable_process_cmdline() should be given an 'int status'
> parameter for specifying 1 vs. 2 (or any other non-zero value) if we
> intend to fix the status of qemu-img compare failures. (Thankfully,
> even though qemu-img check also has a variety of documented return
> values other than 1, at least it documented 1 as internal errors and was
> already using 1 for --object failures).
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 25/31] qemu-img: Use user_creatable_process_cmdline() for --object
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This switches qemu-img from a QemuOpts-based parser for --object to
> user_creatable_process_cmdline() which uses a keyval parser and enforces
> the QAPI schema.
>
> Apart from being a cleanup, this makes non-scalar properties accessible.
>
> Signed-off-by: Kevin Wolf
> ---
> qemu-img.c | 239 -
> 1 file changed, 33 insertions(+), 206 deletions(-)
>
> @@ -1423,15 +1373,9 @@ static int img_compare(int argc, char **argv)
> case 'U':
> force_share = true;
> break;
> -case OPTION_OBJECT: {
> -QemuOpts *opts;
> -opts = qemu_opts_parse_noisily(_object_opts,
> - optarg, true);
> -if (!opts) {
> -ret = 2;
> -goto out4;
Our exit status here of 2 on failure appears to be intentional (since we
reserve 0 for identical, 1 for mismatch, >1 for error)...
> -}
> -} break;
> +case OPTION_OBJECT:
> +user_creatable_process_cmdline(optarg);
> +break;
...but becomes 1 here. Does that matter?
/me goes and tests...
Ouch: with current qemu.git master and none of this series applied:
$ ./qemu-img compare --object foo,id=x /dev/null /dev/null
qemu-img: invalid object type: foo
$ echo $?
1
$ gdb --args ./qemu-img compare --object foo,id=x /dev/null /dev/null
(gdb) b qemu_opts_pars
(gdb) r
(gdb) fin
Run till exit from #0 qemu_opts_parse_noisily (
list=0x5578f020 , params=0x7fffd8a8
"foo,id=x",
permit_abbrev=true) at ../util/qemu-option.c:948
0x555805f9 in img_compare (argc=5, argv=0x7fffd480)
at ../qemu-img.c:1428
1428opts = qemu_opts_parse_noisily(_object_opts,
Value returned is $1 = (QemuOpts *) 0x5583b4b0
(gdb) p *opts
$3 = {id = 0x557a0d58 "`\264\203UUU", list = 0x51,
loc = {kind = (unknown: 0x557f08f0), num = 21845,
ptr = 0x5578f020 , prev = 0x0}, head = {
tqh_first = 0x0, tqh_circ = {tql_next = 0x0, tql_prev = 0x0}}, next = {
tqe_next = 0x5583b500, tqe_circ = {tql_next = 0x5583b500,
tql_prev = 0x5583b528}}}
(gdb)
That looks buggy. qemu_opts_parse_noisily() is NOT returning NULL, but
rather a pointer to something garbage (that id pointing to a garbage
string in the middle of qemu_trace_opts is fishy), and so we've been
exiting with status 1 in spite of the code.
Looks like we'll want a separate patch fixing that first.
> case OPTION_IMAGE_OPTS:
> image_opts = true;
> break;
> @@ -1450,13 +1394,6 @@ static int img_compare(int argc, char **argv)
> filename1 = argv[optind++];
> filename2 = argv[optind++];
>
> -if (qemu_opts_foreach(_object_opts,
> - user_creatable_add_opts_foreach,
> - qemu_img_object_print_help, _fatal)) {
> -ret = 2;
> -goto out4;
Same deal with return value. Except here we used _fatal (which
forces an exit status of 1 rather than returning), and so never even
reach the ret=2 code. Looks like we broke that in commit 334c43e2c3,
where we used to pass NULL instead of _fatal (although that commit
was in turn fixing another problem).
The rest of this patch looks fine, although maybe
user_creatable_process_cmdline() should be given an 'int status'
parameter for specifying 1 vs. 2 (or any other non-zero value) if we
intend to fix the status of qemu-img compare failures. (Thankfully,
even though qemu-img check also has a variety of documented return
values other than 1, at least it documented 1 as internal errors and was
already using 1 for --object failures).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 24/31] qemu-io: Use user_creatable_process_cmdline() for --object
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This switches qemu-io from a QemuOpts-based parser for --object to > user_creatable_process_cmdline() which uses a keyval parser and enforces > the QAPI schema. > > Apart from being a cleanup, this makes non-scalar properties accessible. > > Signed-off-by: Kevin Wolf > --- > qemu-io.c | 33 +++-- > 1 file changed, 3 insertions(+), 30 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 23/31] qom: Factor out user_creatable_process_cmdline()
On 2/24/21 7:52 AM, Kevin Wolf wrote: > The implementation for --object can be shared between > qemu-storage-daemon and other binaries, so move it into a function in > qom/object_interfaces.c that is accessible from everywhere. > > This also requires moving the implementation of qmp_object_add() into a > new user_creatable_add_qapi(), because qom/qom-qmp-cmds.c is not linked > for tools. > > user_creatable_print_help_from_qdict() can become static now. > > Signed-off-by: Kevin Wolf > --- Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 22/31] qom: Remove user_creatable_add_dict()
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This function is now unused and can be removed. > > Signed-off-by: Kevin Wolf > --- > include/qom/object_interfaces.h | 18 -- > qom/object_interfaces.c | 32 > 2 files changed, 50 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 21/31] qemu-storage-daemon: Implement --object with qmp_object_add()
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This QAPIfies --object and ensures that QMP and the command line option > behave the same. > > Signed-off-by: Kevin Wolf > --- > storage-daemon/qemu-storage-daemon.c | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 20/31] qom: Make "object" QemuOptsList optional
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This code is going away anyway, but for a few more commits, we'll be in > a state where some binaries still use QemuOpts and others don't. If the > "object" QemuOptsList doesn't even exist, we don't have to remove (or > fail to remove, and therefore abort) a user creatable object from it. > > Signed-off-by: Kevin Wolf > --- > qom/object_interfaces.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 19/31] qapi/qom: QAPIfy object-add
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This converts object-add from 'gen': false to the ObjectOptions QAPI
> type. As an immediate benefit, clients can now use QAPI schema
> introspection for user creatable QOM objects.
>
> It is also the first step towards making the QAPI schema the only
> external interface for the creation of user creatable objects. Once all
> other places (HMP and command lines of the system emulator and all
> tools) go through QAPI, too, some object implementations can be
> simplified because some checks (e.g. that mandatory options are set) are
> already performed by QAPI, and in another step, QOM boilerplate code
> could be generated from the schema.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json| 11 +--
> include/qom/object_interfaces.h | 7 ---
> hw/block/xen-block.c | 16
> monitor/misc.c | 2 --
> qom/qom-qmp-cmds.c | 25 +++--
> storage-daemon/qemu-storage-daemon.c | 2 --
> 6 files changed, 32 insertions(+), 31 deletions(-)
>
> +++ b/qapi/qom.json
> @@ -839,13 +839,6 @@
> #
> # Create a QOM object.
> #
> -# @qom-type: the class name for the object to be created
> -#
> -# @id: the name of the new object
> -#
> -# Additional arguments depend on qom-type and are passed to the backend
> -# unchanged.
> -#
> # Returns: Nothing on success
> # Error if @qom-type is not a valid class name
> #
> @@ -859,9 +852,7 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'object-add',
> - 'data': {'qom-type': 'str', 'id': 'str'},
> - 'gen': false } # so we can get the additional arguments
> +{ 'command': 'object-add', 'data': 'ObjectOptions', 'boxed': true }
So much more concise ;) A grep for TYPE_USER_CREATABLE doesn't seem to
turn up any *_class_init() functions that your earlier patches in the
series missed, so I think you captured an accurate 1:1 mapping. There
is include/chardev/char.h with the comment about "TODO: eventually use
TYPE_USER_CREATABLE" which may point to the next item to be added to
ObjectOptions, but that's not for this series.
> +++ b/qom/qom-qmp-cmds.c
>
> -void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
> +void qmp_object_add(ObjectOptions *options, Error **errp)
> {
> -user_creatable_add_dict(qdict, false, errp);
> +Visitor *v;
> +QObject *qobj;
> +QDict *props;
> +Object *obj;
> +
> +v = qobject_output_visitor_new();
> +visit_type_ObjectOptions(v, NULL, , _abort);
> +visit_complete(v, );
> +visit_free(v);
This part is nice...
> +
> +props = qobject_to(QDict, qobj);
> +qdict_del(props, "qom-type");
> +qdict_del(props, "id");
...while this part makes it seem like we still have more cleanup to come
later. But hey, progress!
> +
> +v = qobject_input_visitor_new(QOBJECT(props));
> +obj = user_creatable_add_type(ObjectType_str(options->qom_type),
> + options->id, props, v, errp);
> +object_unref(obj);
> +visit_free(v);
> }
>
Once you address Paolo's comment, you can also add
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 18/31] qapi/qom: Add ObjectOptions for x-remote-object
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the x-remote-object
> object.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index f8ff322df0..6793342e81 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -641,6 +641,20 @@
> { 'struct': 'PrManagerHelperProperties',
>'data': { 'path': 'str' } }
>
> +##
> +# @RemoteObjectProperties:
> +#
> +# Properties for x-remote-object objects.
> +#
> +# @fd: file descriptor name previously passed via 'getfd' command
> +#
> +# @devid: the id of the device to be associated with the file descriptor
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'RemoteObjectProperties',
> + 'data': { 'fd': 'str', 'devid': 'str' } }
> +
Matches hw/remote/remote-obj.c:remote_object_class_init().
> ##
> # @RngProperties:
> #
> @@ -762,7 +776,8 @@
> 'tls-creds-anon',
> 'tls-creds-psk',
> 'tls-creds-x509',
> -'tls-cipher-suites'
> +'tls-cipher-suites',
> +'x-remote-object'
>] }
>
> ##
> @@ -815,7 +830,8 @@
>'tls-creds-anon': 'TlsCredsAnonProperties',
>'tls-creds-psk': 'TlsCredsPskProperties',
>'tls-creds-x509': 'TlsCredsX509Properties',
> - 'tls-cipher-suites': 'TlsCredsProperties'
> + 'tls-cipher-suites': 'TlsCredsProperties',
> + 'x-remote-object':'RemoteObjectProperties'
>} }
>
> ##
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 17/31] qapi/qom: Add ObjectOptions for input-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the input-* objects.
>
> ui.json cannot be included in qom.json because the storage daemon can't
> use it, so move GrabToggleKeys to common.json.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/common.json | 12 ++
> qapi/qom.json| 58
> qapi/ui.json | 13 +--
> 3 files changed, 71 insertions(+), 12 deletions(-)
>
> +##
> +# @InputBarrierProperties:
> +#
> +# Properties for input-barrier objects.
> +#
> +# @name: the screen name as declared in the screens section of barrier.conf
> +#
> +# @server: hostname of the Barrier server (default: "localhost")
> +#
> +# @port: TCP port of the Barrier server (default: "24800")
I can understand this being a string (if non-numeric, it can be treated
as a well-known service name instead), but...
> +#
> +# @x-origin: x coordinate of the leftmost pixel on the guest screen
> +#(default: "0")
...why are these other fields a string instead of an integer? But you
are just doing faithful translation of what we already have.
Bummer - our naming for this member implies that it is experimental,
which is a misnomer (it is quite stable, when viewed in tandem with
y-origin). Not your fault. Would 'origin-x' and 'origin-y' be any
better as new aliases in a followup patch?
> +#
> +# @y-origin: y coordinate of he topmost pixel on the guest screen (default:
> "0")
"the", long line
> +#
> +# @width: the width of secondary screen in pixels (default: "1920")
> +#
> +# @height: the height of secondary screen in pixels (default: "1080")
> +#
> +# Since: 4.2
> +##
> +{ 'struct': 'InputBarrierProperties',
> + 'data': { 'name': 'str',
> +'*server': 'str',
> +'*port': 'str',
> +'*x-origin': 'str',
> +'*y-origin': 'str',
> +'*width': 'str',
> +'*height': 'str' } }
Matches ui/input-barrier.c:input_barrier_class_init().
> +
> +##
> +# @InputLinuxProperties:
> +#
> +# Properties for input-linux objects.
> +#
> +# @evdev: the path of the host evdev device to use
> +#
> +# @grab_all: if true, grab is toggled for all devices (e.g. both keyboard and
> +#mouse) instead of just one device (default: false)
We have inconsistent naming within this object (see grab-toggle); a good
followup would be an alias for 'grab-all'.
> +#
> +# @repeat: enables auto-repeat events (default: false)
> +#
> +# @grab-toggle: the key or key combination that toggles device grab
> +# (default: ctrl-ctrl)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'InputLinuxProperties',
> + 'data': { 'evdev': 'str',
> +'*grab_all': 'bool',
> +'*repeat': 'bool',
> +'*grab-toggle': 'GrabToggleKeys' } }
matches ui/input-linux.c.
> +
> ##
> # @IothreadProperties:
> #
> @@ -689,6 +743,8 @@
> 'filter-redirector',
> 'filter-replay',
> 'filter-rewriter',
> +'input-barrier',
> +'input-linux',
> 'iothread',
> 'memory-backend-file',
> 'memory-backend-memfd',
> @@ -741,6 +797,8 @@
>'filter-redirector': 'FilterRedirectorProperties',
>'filter-replay': 'NetfilterProperties',
>'filter-rewriter':'FilterRewriterProperties',
> + 'input-barrier': 'InputBarrierProperties',
> + 'input-linux':'InputLinuxProperties',
>'iothread': 'IothreadProperties',
>'memory-backend-file':'MemoryBackendFileProperties',
>'memory-backend-memfd': 'MemoryBackendMemfdProperties',
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 16/31] qapi/qom: Add ObjectOptions for confidential-guest-support
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the objects implementing
> the confidential-guest-support interface.
>
> pef-guest and s390x-pv-guest don't have any properties, so they only
> need to be added to the ObjectType enum without adding a new branch to
> ObjectOptions.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 37 +
> 1 file changed, 37 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index e7184122e9..d5f68b5c89 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -633,6 +633,38 @@
>'base': 'RngProperties',
>'data': { '*filename': 'str' } }
>
> +##
> +# @SevGuestProperties:
> +#
> +# Properties for sev-guest objects.
> +#
> +# @sev-device: SEV device to use (default: "/dev/sev")
> +#
> +# @dh-cert-file: guest owners DH certificate (encoded with base64)
> +#
> +# @session-file: guest owners session parameters (encoded with base64)
Matches target/i386/sev.c:sev_guest_class_init()...
> +#
> +# @policy: SEV policy value (default: 0x1)
> +#
> +# @handle: SEV firmware handle (default: 0)
> +#
> +# @cbitpos: C-bit location in page table entry (default: 0)
> +#
> +# @reduced-phys-bits: number of bits in physical addresses that become
> +# unavailable when SEV is enabled
...and sev_guest_instance_init().
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'SevGuestProperties',
> + 'data': { '*sev-device': 'str',
> +'*dh-cert-file': 'str',
> +'*session-file': 'str',
> +'*policy': 'uint32',
> +'*handle': 'uint32',
> +'*cbitpos': 'uint32',
> +'reduced-phys-bits': 'uint32' },
> + 'if': 'defined(CONFIG_SEV)' }
> +
> ##
> # @ObjectType:
> #
> @@ -661,12 +693,15 @@
> 'memory-backend-file',
> 'memory-backend-memfd',
> 'memory-backend-ram',
> +{'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' },
> 'pr-manager-helper',
> 'rng-builtin',
> 'rng-egd',
> 'rng-random',
> 'secret',
> 'secret_keyring',
> +{'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' },
> +'s390-pv-guest',
> 'throttle-group',
> 'tls-creds-anon',
> 'tls-creds-psk',
> @@ -716,6 +751,8 @@
>'rng-random': 'RngRandomProperties',
>'secret': 'SecretProperties',
>'secret_keyring': 'SecretKeyringProperties',
> + 'sev-guest': { 'type': 'SevGuestProperties',
> + 'if': 'defined(CONFIG_SEV)' },
>'throttle-group': 'ThrottleGroupProperties',
>'tls-creds-anon': 'TlsCredsAnonProperties',
>'tls-creds-psk': 'TlsCredsPskProperties',
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 15/31] qapi/qom: Add ObjectOptions for pr-manager-helper
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the pr-manager-helper
> object.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index e3357f5123..e7184122e9 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -575,6 +575,18 @@
> '*hugetlbsize': 'size',
> '*seal': 'bool' } }
>
> +##
> +# @PrManagerHelperProperties:
> +#
> +# Properties for pr-manager-helper objects.
> +#
> +# @path: the path to a Unix domain socket for connecting to the external
> helper
> +#
> +# Since: 2.11
> +##
> +{ 'struct': 'PrManagerHelperProperties',
> + 'data': { 'path': 'str' } }
> +
Matches scsi/pr-manager-helper.c:pr_manager_helper_class_init().
> ##
> # @RngProperties:
> #
> @@ -649,6 +661,7 @@
> 'memory-backend-file',
> 'memory-backend-memfd',
> 'memory-backend-ram',
> +'pr-manager-helper',
> 'rng-builtin',
> 'rng-egd',
> 'rng-random',
> @@ -697,6 +710,7 @@
>'memory-backend-file':'MemoryBackendFileProperties',
>'memory-backend-memfd': 'MemoryBackendMemfdProperties',
>'memory-backend-ram': 'MemoryBackendProperties',
> + 'pr-manager-helper': 'PrManagerHelperProperties',
>'rng-builtin':'RngProperties',
>'rng-egd':'RngEgdProperties',
>'rng-random': 'RngRandomProperties',
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 14/31] qapi/qom: Add ObjectOptions for filter-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the filter-* objects.
>
> Some parts of the interface (in particular NetfilterProperties.position)
> are very unusual for QAPI, but for now just describe the existing
> interface.
>
> net.json can't be included in qom.json because the storage daemon
> doesn't have it. NetFilterDirection is still required in the new object
> property definitions in qom.json, so move this enum to common.json.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/common.json | 20 +++
> qapi/net.json| 20 ---
> qapi/qom.json| 143 +++
> 3 files changed, 163 insertions(+), 20 deletions(-)
>
> +++ b/qapi/qom.json
> @@ -313,6 +313,137 @@
>'data': { 'addr': 'str' ,
> '*id-list': 'str' } }
>
> +##
> +# @NetfilterInsert:
> +#
> +# Indicates where to insert a netfilter relative to a given other filter.
> +#
> +# @before: insert before the specified filter
> +#
> +# @behind: insert behind the specified filter
> +#
> +# Since: 5.0
> +##
> +{ 'enum': 'NetfilterInsert',
> + 'data': [ 'before', 'behind' ] }
> +
> +##
> +# @NetfilterProperties:
> +#
> +# Properties for objects of classes derived from netfilter.
> +#
> +# @netdev: id of the network device backend to filter
> +#
> +# @queue: indicates which queue(s) to filter (default: all)
> +#
> +# @status: indicates whether the filter is enabled ("on") or disabled ("off")
> +# (default: "on")
An enum would be nicer than 'str', but your commit message is accurate.
> +#
> +# @position: specifies where the filter should be inserted in the filter
> list.
> +#"head" means the filter is inserted at the head of the filter
> list,
> +#before any existing filters.
> +#"tail" means the filter is inserted at the tail of the filter
> list,
> +#behind any existing filters (default).
> +#"id=" means the filter is inserted before or behind the
> filter
> +#specified by , depending on the @insert property.
> +#(default: "tail")
> +#
Wow, you're not kidding about this not being typical QAPI. Oh well.
> +# @insert: where to insert the filter relative to the filter given in
> @position.
> +# Ignored if @position is "head" or "tail". (default: behind)
Back to the question of if it is worth updating the QAPI generator to
allow a flat union as the branch of yet another flat union. If we did
that, we could have (untested):
{ 'enum': 'NetfilterPosition', 'data': [ 'head', 'tail', 'id' ] }
{ 'union': 'NetfilterBase',
'base': { 'position': 'NetfilterPosition',
'netdev'..., 'queue', 'status'... },
'discriminator': 'position',
'data': { 'head': {}, 'tail': {},
'id': { '*insert': 'NetfilterInsert', 'id': 'str' } }
but that is a change to our existing id=xyz parsing, so we may need an
alias or deprecation period...
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'NetfilterProperties',
> + 'data': { 'netdev': 'str',
> +'*queue': 'NetFilterDirection',
> +'*status': 'str',
> +'*position': 'str',
> +'*insert': 'NetfilterInsert' } }
> +
> +##
> +# @FilterBufferProperties:
> +#
> +# Properties for filter-buffer objects.
> +#
> +# @interval: a non-zero interval in microseconds. All packets arriving in
> the
> +#given interval are delayed until the end of the interval.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'FilterBufferProperties',
> + 'base': 'NetfilterProperties',
> + 'data': { 'interval': 'uint32' } }
matches net/filter-buffer.c:filter_buffer_class_init().
> +
> +##
> +# @FilterDumpProperties:
> +#
> +# Properties for filter-dump objects.
> +#
> +# @file: the filename where the dumped packets should be stored
> +#
> +# @maxlen: maximum number of bytes in a packet that are stored (default:
> 65536)
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'FilterDumpProperties',
> + 'base': 'NetfilterProperties',
> + 'data': { 'file': 'str',
> +'*maxlen': 'uint32' } }
Matches net/dump.c:filter_dump_class_init().
> +
> +##
> +# @FilterMirrorProperties:
> +#
> +# Properties for filter-mirror objects.
> +#
> +# @outdev: the name of a character device backend to which all incoming
> packets
> +# are mirrored
> +#
> +# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'FilterMirrorProperties',
> + 'base': 'NetfilterProperties',
> + 'data': { 'outdev': 'str',
> +'*vnet_hdr_support': 'bool' } }
Matches filter-mirror.c:filter_mirror_class_init(). For the future, can
we rename to vnet-hdr-support?
> +
> +##
> +# @FilterRedirectorProperties:
> +#
> +# Properties for filter-redirector objects.
> +#
> +# At least one of @indev or @outdev must be present. If both are present,
> they
> +# must not refer to the same character device backend.
> +#
> +# @indev: the name of
Re: [PATCH v2 13/31] qapi/qom: Add ObjectOptions for colo-compare
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the colo-compare object.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 49 +
> 1 file changed, 49 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 4b1cd4b8dc..8e4414f843 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -222,6 +222,53 @@
>'data': { 'if': 'str',
> 'canbus': 'str' } }
>
> +##
> +# @ColoCompareProperties:
> +#
> +# Properties for colo-compare objects.
> +#
> +# @primary_in: name of the character device backend to use for the primary
> +# input (incoming packets are redirected to @outdev)
> +#
> +# @secondary_in: name of the character device backend to use for secondary
> +#input (incoming packets are only compared to the input on
> +#@primary_in and then dropped)
> +#
Idea for future improvement: use aliases to shift over to 'primary-in',
'secondary-in', and so on as our preferred name. But not for this
patch, which is a mechanical conversion of what exists.
> +# @outdev: name of the character device backend to use for output
> +#
> +# @iothread: name of the iothread to run in
> +#
> +# @notify_dev: name of the character device backend to be used to communicate
> +# with the remote colo-frame (only for Xen COLO)
> +#
> +# @compare_timeout: the maximum time to hold a packet from @primary_in for
> +# comparison with an incoming packet on @secondary_in in
> +# milliseconds (default: 3000)
> +#
> +# @expired_scan_cycle: the interval at which colo-compare checks whether
> +# packets from @primary have timed out, in milliseconds
> +# (default: 3000)
> +#
> +# @max_queue_size: the maximum number of packets to keep in the queue for
> +# comparing with incoming packets from @secondary_in. If
> the
> +# queue is full and addtional packets are received, the
> +# addtional packets are dropped. (default: 1024)
> +#
> +# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'ColoCompareProperties',
> + 'data': { 'primary_in': 'str',
> +'secondary_in': 'str',
> +'outdev': 'str',
> +'iothread': 'str',
> +'*notify_dev': 'str',
> +'*compare_timeout': 'uint64',
> +'*expired_scan_cycle': 'uint32',
> +'*max_queue_size': 'uint32',
> +'*vnet_hdr_support': 'bool' } }
Matches net/colo-compare.c:colo_compare_init().
> +
> ##
> # @CryptodevBackendProperties:
> #
> @@ -456,6 +503,7 @@
> 'authz-simple',
> 'can-bus',
> 'can-host-socketcan',
> +'colo-compare',
> 'cryptodev-backend',
> 'cryptodev-backend-builtin',
> 'cryptodev-vhost-user',
> @@ -497,6 +545,7 @@
>'authz-pam': 'AuthZPAMProperties',
>'authz-simple': 'AuthZSimpleProperties',
>'can-host-socketcan': 'CanHostSocketcanProperties',
> + 'colo-compare': 'ColoCompareProperties',
>'cryptodev-backend': 'CryptodevBackendProperties',
>'cryptodev-backend-builtin': 'CryptodevBackendProperties',
>'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the can-* objects.
>
> can-bus doesn't have any properties, so it only needs to be added to the
> ObjectType enum without adding a new branch to ObjectOptions.
I somewhat prefer
'can-bus': {},
to make it explicit that we thought about it, but since we allow
defaulted union branches, your approach works too.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index f22b7aa99b..4b1cd4b8dc 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -207,6 +207,21 @@
>'returns': [ 'ObjectPropertyInfo' ],
>'allow-preconfig': true }
>
> +##
> +# @CanHostSocketcanProperties:
> +#
> +# Properties for can-host-socketcan objects.
> +#
> +# @if: interface name of the host system CAN bus to connect to
> +#
> +# @canbus: object ID of the can-bus object to connect to the host interface
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'CanHostSocketcanProperties',
> + 'data': { 'if': 'str',
> +'canbus': 'str' } }
> +
Okay, matches net/can/can_socketcan.c:can_host_socketcan_class_init()
(after chasing down the parent class in
net/can/can_host.c:can_host_class_init() to find "canbus").
> ##
> # @CryptodevBackendProperties:
> #
> @@ -439,6 +454,8 @@
> 'authz-listfile',
> 'authz-pam',
> 'authz-simple',
> +'can-bus',
> +'can-host-socketcan',
> 'cryptodev-backend',
> 'cryptodev-backend-builtin',
> 'cryptodev-vhost-user',
> @@ -479,6 +496,7 @@
>'authz-listfile': 'AuthZListFileProperties',
>'authz-pam': 'AuthZPAMProperties',
>'authz-simple': 'AuthZSimpleProperties',
> + 'can-host-socketcan': 'CanHostSocketcanProperties',
>'cryptodev-backend': 'CryptodevBackendProperties',
>'cryptodev-backend-builtin': 'CryptodevBackendProperties',
>'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded'
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the tls-* objects.
>
> The 'loaded' property doesn't seem to make sense as an external
> interface: It is automatically set to true in ucc->complete, and
> explicitly setting it to true earlier just means that additional options
> will be silently ignored.
>
> In other words, the 'loaded' property is useless. Mark it as deprecated
> in the schema from the start.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/crypto.json | 98
> qapi/qom.json| 12 +-
> 2 files changed, 108 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 0fef3de66d..7116ae9a46 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -442,3 +442,101 @@
> { 'struct': 'SecretKeyringProperties',
>'base': 'SecretCommonProperties',
>'data': { 'serial': 'int32' } }
> +
> +##
> +# @TlsCredsProperties:
> +#
> +# Properties for objects of classes derived from tls-creds.
> +#
> +# @verify-peer: if true the peer credentials will be verified once the
> +# handshake is completed. This is a no-op for anonymous
> +# credentials. (default: true)
> +#
> +# @dir: the path of the directory that contains the credential files
> +#
> +# @endpoint: whether the QEMU network backend that uses the credentials will
> be
> +#acting as a client or as a server (default: client)
> +#
> +# @priority: a gnutls priority string as described at
> +#https://gnutls.org/manual/html_node/Priority-Strings.html
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'TlsCredsProperties',
> + 'data': { '*verify-peer': 'bool',
> +'*dir': 'str',
> +'*endpoint': 'QCryptoTLSCredsEndpoint',
> +'*priority': 'str' } }
Matches crypto/tlscreds.c:qcrypto_tls_creds_class_init().
> +
> +##
> +# @TlsCredsAnonProperties:
> +#
> +# Properties for tls-creds-anon objects.
> +#
> +# @loaded: if true, the credentials are loaded immediately when applying this
> +# option and will ignore options that are processed later. Don't
> use;
> +# only provided for compatibility. (default: false)
> +#
> +# Features:
> +# @deprecated: Member @loaded is deprecated. Setting true doesn't make
> sense,
> +# and false is already the default.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'TlsCredsAnonProperties',
> + 'base': 'TlsCredsProperties',
> + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] } } }
Since we documented that 'verify-peer' is a no-op for this struct, is it
worth altering our type hierarchy to make it explicit, as in:
TlsCredsCommonProperties - dir, endpoint, priority
TlsCredsProperties - TlsCredsCommonProperties + verify-peer
TlsCredsAnonProperties - TlsCredsCommonProperties + loaded
TlsCredsPskProperties - TlsCredsProperties + loaded, username
But even if not, this matches
crypto/tlscredsanon.c:qcrypto_tls_creds_anon_class_init().
> +
> +##
> +# @TlsCredsPskProperties:
> +#
> +# Properties for tls-creds-psk objects.
> +#
> +# @loaded: if true, the credentials are loaded immediately when applying this
> +# option and will ignore options that are processed later. Don't
> use;
> +# only provided for compatibility. (default: false)
> +#
> +# @username: the username which will be sent to the server. For clients
> only.
> +#If absent, "qemu" is sent and the property will read back as an
> +#empty string.
> +#
> +# Features:
> +# @deprecated: Member @loaded is deprecated. Setting true doesn't make
> sense,
> +# and false is already the default.
> +#
> +# Since: 3.0
> +##
> +{ 'struct': 'TlsCredsPskProperties',
> + 'base': 'TlsCredsProperties',
> + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
> +'*username': 'str' } }
This matches crypto/tlscredspsk.c:qcrypto_tls_creds_psk_class_init().
Do we want to use QAPI type inheritance to declare a union where
'endpoint' is the union discriminator, and 'username' is only present
for 'endpoint':'client'? (Hmm, we'd have to improve the QAPI code
generator to allow a flat union as the branch of yet another flat union...)
> +
> +##
> +# @TlsCredsX509Properties:
> +#
> +# Properties for tls-creds-x509 objects.
> +#
> +# @loaded: if true, the credentials are loaded immediately when applying this
> +# option and will ignore options that are processed later. Don't
> use;
> +# only provided for compatibility. (default: false)
> +#
> +# @sanity-check: if true, perform some sanity checks before using the
> +#credentials (default: true)
> +#
> +# @passwordid: For the server-key.pem and client-key.pem files which contain
> +# sensitive private keys, it is possible to use an encrypted
> +# version by providing the @passwordid parameter. This provides
> +# the ID of a previously
Re: [PATCH v2 10/31] qapi/qom: Add ObjectOptions for secret*, deprecate 'loaded'
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the secret* objects.
>
> The 'loaded' property doesn't seem to make sense as an external
> interface: It is automatically set to true in ucc->complete, and
> explicitly setting it to true earlier just means that additional options
> will be silently ignored.
>
> In other words, the 'loaded' property is useless. Mark it as deprecated
> in the schema from the start.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/crypto.json | 61 ++
> qapi/qom.json | 5
> docs/system/deprecated.rst | 11 +++
> 3 files changed, 77 insertions(+)
>
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 2aebe6fa20..0fef3de66d 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -381,3 +381,64 @@
>'discriminator': 'format',
>'data': {
>'luks': 'QCryptoBlockAmendOptionsLUKS' } }
> +
> +##
> +# @SecretCommonProperties:
> +#
> +# Properties for objects of classes derived from secret-common.
> +#
> +# @loaded: if true, the secret is loaded immediately when applying this
> option
> +# and will probably fail when processing the next option. Don't use;
> +# only provided for compatibility. (default: false)
> +#
> +# @format: the data format that the secret is provided in (default: raw)
> +#
> +# @keyid: the name of another secret that should be used to decrypt the
> +# provided data. If not present, the data is assumed to be
> unencrypted.
> +#
> +# @iv: the random initialization vector used for encryption of this
> particular
> +# secret. Should be a base64 encrypted string of the 16-byte IV.
> Mandatory
> +# if @keyid is given. Ignored if @keyid is absent.
> +#
> +# Features:
> +# @deprecated: Member @loaded is deprecated. Setting true doesn't make
> sense,
> +# and false is already the default.
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'SecretCommonProperties',
> + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
> +'*format': 'QCryptoSecretFormat',
> +'*keyid': 'str',
> +'*iv': 'str' } }
Matches crypto/secret_common.c:qcrypto_secret_class_init(), and I concur
with the deprecation.
> +
> +##
> +# @SecretProperties:
> +#
> +# Properties for secret objects.
> +#
> +# Either @data or @file must be provided, but not both.
> +#
> +# @data: the associated with the secret from
> +#
> +# @file: the filename to load the data associated with the secret from
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'SecretProperties',
> + 'base': 'SecretCommonProperties',
> + 'data': { '*data': 'str',
> +'*file': 'str' } }
Matches crypto/secret.c:qcrypto_secret_class_init() (ugh, we really do
reuse the same static function name in two different files, but not your
fault)
> +
> +##
> +# @SecretKeyringProperties:
> +#
> +# Properties for secret_keyring objects.
> +#
> +# @serial: serial number that identifies a key to get from the kernel
> +#
> +# Since: 5.1
> +##
> +{ 'struct': 'SecretKeyringProperties',
> + 'base': 'SecretCommonProperties',
> + 'data': { 'serial': 'int32' } }
Matches crypto/secret_keyring.c:qcrypto_secret_keyring_class_init().
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 449dca8ec5..2668ad8369 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -7,6 +7,7 @@
> { 'include': 'authz.json' }
> { 'include': 'block-core.json' }
> { 'include': 'common.json' }
> +{ 'include': 'crypto.json' }
>
> ##
> # = QEMU Object Model (QOM)
> @@ -449,6 +450,8 @@
> 'rng-builtin',
> 'rng-egd',
> 'rng-random',
> +'secret',
> +'secret_keyring',
What is stopping us from naming this 'secret-keyring'?
> 'throttle-group'
>] }
>
> @@ -483,6 +486,8 @@
>'rng-builtin':'RngProperties',
>'rng-egd':'RngEgdProperties',
>'rng-random': 'RngRandomProperties',
> + 'secret': 'SecretProperties',
> + 'secret_keyring': 'SecretKeyringProperties',
>'throttle-group': 'ThrottleGroupProperties'
>} }
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 79991c2893..78b175cb59 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -155,6 +155,17 @@ other options have been processed. This will either
> have no effect (if
> ``opened`` was the last option) or cause errors. The property is therefore
> useless and should not be specified.
>
> +``loaded`` property of ``secret`` and ``secret_keyring`` objects (since
> 6.0.0)
> +''
> +
> +The only effect of specifying ``loaded=on`` in the command line or QMP
> +``object-add`` is that the secret is loaded immediately, possibly before all
> +other options have been processed. This will either have no
[PATCH] meson: Add documentation installation directory option
Allow the directory to be chosen at installation time, to support local
conventions e.g. versioning.
Signed-off-by: Chris Mayo
---
meson.build | 6 +-
meson_options.txt | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 369548f127..5c7a335947 100644
--- a/meson.build
+++ b/meson.build
@@ -83,8 +83,12 @@ mandir = prefix / get_option('mandir')
sbindir = prefix / get_option('sbindir')
sharedstatedir = prefix / get_option('sharedstatedir')
+docdir = get_option('docdir')
+if docdir == ''
+ docdir = datadir / 'doc' / meson.project_name()
+endif
+
confdir = sysconfdir / meson.project_name()
-docdir = datadir / 'doc' / meson.project_name()
pkgdatadir = datadir / meson.project_name()
diff --git a/meson_options.txt b/meson_options.txt
index e5d79c2b6b..2606648b64 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -7,6 +7,7 @@ option('expensive_tests', type: 'feature', value: 'auto',
description: 'set the
option('test_coverage', type: 'boolean', value: false, description: 'turn on
code coverage instrumentation')
option('git_werror', type: 'feature', value: 'auto', description: 'use -Werror
if building from GIT')
option('rpath', type: 'feature', value: 'auto', description: 'whether to
include rpath information in installed binaries and libraries')
+option('docdir', type: 'string', value: '', description: 'documentation
installation directory')
option('docs', type: 'feature', value: 'auto', description: 'whether to
generate documentation')
option('tests', type: 'feature', value: 'auto', description: 'whether to build
tests')
--
2.28.0
Re: [PATCH v2 09/31] qapi/qom: Add ObjectOptions for throttle-group
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the throttle-group object.
>
> The only purpose of the x-* properties is to make the nested options in
> 'limits' available for a command line parser that doesn't support
> structs. Any parser that will use the QAPI schema will supports structs,
> though, so they will not be needed in the schema in the future.
>
> To keep the conversion straightforward, add them to the schema anyway.
> We can then remove the options and adjust documentation, test cases etc.
> in a separate patch.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/block-core.json | 27 +++
> qapi/qom.json| 7 +--
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9f555d5c1d..a67fa0cc59 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2504,6 +2504,33 @@
> '*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
> '*iops-size' : 'int' } }
>
> +##
> +# @ThrottleGroupProperties:
> +#
> +# Properties for throttle-group objects.
Corresponds to block/throttle-groups.c:throttle_group_obj_class_init()
with its ThrottleParamInfo struct for the x- fields, and limits as-is.
> +#
> +# The options starting with x- are aliases for the same key without x- in
> +# the @limits object. As indicated by the x- prefix, this is not a stable
> +# interface and may be removed or changed incompatibly in the future. Use
> +# @limits for a supported stable interface.
> +#
> +# @limits: limits to apply for this throttle group
And I did check that qapi/block-core.json:ThrottleLimits has the same
fields as the ThrottleParamInfo x- fields. All this duplication! But
we're getting to a state where it will be easier to clean up the cruft.
> +#
> +# Since: 2.11
> +##
> +{ 'struct': 'ThrottleGroupProperties',
> + 'data': { '*limits': 'ThrottleLimits',
> +'*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
> +'*x-iops-total-max-length' : 'int', '*x-iops-read' : 'int',
> +'*x-iops-read-max' : 'int', '*x-iops-read-max-length' : 'int',
> +'*x-iops-write' : 'int', '*x-iops-write-max' : 'int',
> +'*x-iops-write-max-length' : 'int', '*x-bps-total' : 'int',
> +'*x-bps-total-max' : 'int', '*x-bps-total-max-length' : 'int',
> +'*x-bps-read' : 'int', '*x-bps-read-max' : 'int',
> +'*x-bps-read-max-length' : 'int', '*x-bps-write' : 'int',
> +'*x-bps-write-max' : 'int', '*x-bps-write-max-length' : 'int',
> +'*x-iops-size' : 'int' } }
> +
> ##
> # @block-stream:
> #
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 73f28f9608..449dca8ec5 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -5,6 +5,7 @@
> # See the COPYING file in the top-level directory.
>
> { 'include': 'authz.json' }
> +{ 'include': 'block-core.json' }
> { 'include': 'common.json' }
>
> ##
> @@ -447,7 +448,8 @@
> 'memory-backend-ram',
> 'rng-builtin',
> 'rng-egd',
> -'rng-random'
> +'rng-random',
> +'throttle-group'
>] }
>
> ##
> @@ -480,7 +482,8 @@
>'memory-backend-ram': 'MemoryBackendProperties',
>'rng-builtin':'RngProperties',
>'rng-egd':'RngEgdProperties',
> - 'rng-random': 'RngRandomProperties'
> + 'rng-random': 'RngRandomProperties',
> + 'throttle-group': 'ThrottleGroupProperties'
>} }
>
> ##
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 08/31] qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened'
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the rng-* objects.
>
> The 'opened' property doesn't seem to make sense as an external
> interface: It is automatically set to true in ucc->complete, and
> explicitly setting it to true earlier just means that trying to set
> additional options will result in an error. After the property has once
> been set to true (i.e. when the object construction has completed), it
> can never be reset to false. In other words, the 'opened' property is
> useless. Mark it as deprecated in the schema from the start.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 56 --
> docs/system/deprecated.rst | 9 ++
> 2 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 1a869006a1..73f28f9608 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -380,6 +380,52 @@
> '*hugetlbsize': 'size',
> '*seal': 'bool' } }
>
> +##
> +# @RngProperties:
> +#
> +# Properties for objects of classes derived from rng.
> +#
> +# @opened: if true, the device is opened immediately when applying this
> option
> +# and will probably fail when processing the next option. Don't use;
> +# only provided for compatibility. (default: false)
> +#
> +# Features:
> +# @deprecated: Member @opened is deprecated. Setting true doesn't make
> sense,
> +# and false is already the default.
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngProperties',
> + 'data': { '*opened': { 'type': 'bool', 'features': ['deprecated'] } } }
Matches backends/rng.c:rng_backend_class_init(), and I concur with the
deprecation.
> +
> +##
> +# @RngEgdProperties:
> +#
> +# Properties for rng-egd objects.
> +#
> +# @chardev: the name of a character device backend that provides the
> connection
> +# to the RNG daemon
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngEgdProperties',
> + 'base': 'RngProperties',
> + 'data': { 'chardev': 'str' } }
Matches backends/rng-egd.c:rng_egd_class_init().
> +
> +##
> +# @RngRandomProperties:
> +#
> +# Properties for rng-random objects.
> +#
> +# @filename: the filename of the device on the host to obtain entropy from
> +#(default: "/dev/urandom")
> +#
> +# Since: 1.3
> +##
> +{ 'struct': 'RngRandomProperties',
> + 'base': 'RngProperties',
> + 'data': { '*filename': 'str' } }
Matches backends/rng-random.c:rng_random_class_init().
> +
> ##
> # @ObjectType:
> #
> @@ -398,7 +444,10 @@
> 'iothread',
> 'memory-backend-file',
> 'memory-backend-memfd',
> -'memory-backend-ram'
> +'memory-backend-ram',
> +'rng-builtin',
> +'rng-egd',
> +'rng-random'
>] }
>
> ##
> @@ -428,7 +477,10 @@
>'iothread': 'IothreadProperties',
>'memory-backend-file':'MemoryBackendFileProperties',
>'memory-backend-memfd': 'MemoryBackendMemfdProperties',
> - 'memory-backend-ram': 'MemoryBackendProperties'
> + 'memory-backend-ram': 'MemoryBackendProperties',
> + 'rng-builtin':'RngProperties',
> + 'rng-egd':'RngEgdProperties',
> + 'rng-random': 'RngRandomProperties'
>} }
>
> ##
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 00b694e053..79991c2893 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -146,6 +146,15 @@ library enabled as a cryptography provider.
> Neither the ``nettle`` library, or the built-in cryptography provider are
> supported on FIPS enabled hosts.
>
> +``opened`` property of ``rng-*`` objects (since 6.0.0)
> +''
> +
> +The only effect of specifying ``opened=on`` in the command line or QMP
> +``object-add`` is that the device is opened immediately, possibly before all
> +other options have been processed. This will either have no effect (if
> +``opened`` was the last option) or cause errors. The property is therefore
> +useless and should not be specified.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 07/31] qapi/qom: Add ObjectOptions for memory-backend-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the memory-backend-*
> objects.
>
> HostMemPolicy has to be moved to an include file that can be used by the
> storage daemon, too, because ObjectOptions must be the same in all
> binaries if we don't want to compile the whole code multiple times.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/common.json | 20
> qapi/machine.json | 22 +
> qapi/qom.json | 118 +-
> 3 files changed, 138 insertions(+), 22 deletions(-)
>
> +++ b/qapi/qom.json
> +##
> +# @MemoryBackendProperties:
> +#
> +# Properties for objects of classes derived from memory-backend.
> +#
> +# @merge: if true, mark the memory as mergeable (default depends on the
> machine
> +# type)
> +#
> +# @dump: if true, include the memory in core dumps (default depends on the
> +#machine type)
Interesting choice to flip the description text from its previous
wording, but fine by me:
object_class_property_set_description(oc, "dump",
"Set to 'off' to exclude from core dump");
> +#
> +# @host-nodes: the list of NUMA host nodes to bind the memory to
> +#
> +# @policy: the NUMA policy (default: 'default')
> +#
> +# @prealloc: if true, preallocate memory (default: false)
Not quite in the same order as
backends/hostmem.c:host_memory_backend_class_init() (alphabetic here
instead of matching the C code declaration order), but that doesn't
impact QMP semantics, and I was able to match everything up in the end.
> +#
> +# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> +#
> +# @share: if false, the memory is private to QEMU; if true, it is shared
> +# (default: false)
> +#
> +# @size: size of the memory region in bytes
> +#
> +# @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
> +#for ramblock-id. Disable this for
> 4.0
> +#machine types or older to allow
> +#migration with newer QEMU versions.
> +#(default: false generally, but true
> +#for machine types <= 4.0)
The comment in the C code mentions that in spite of the x- prefix, we
have to treat this as a stable interface until 4.0 machines disappear.
Do we need any of that sentiment in the documentation here?
> +#
> +# Since: 2.1
> +##
> +{ 'struct': 'MemoryBackendProperties',
> + 'data': { '*dump': 'bool',
> +'*host-nodes': ['uint16'],
> +'*merge': 'bool',
> +'*policy': 'HostMemPolicy',
> +'*prealloc': 'bool',
> +'*prealloc-threads': 'uint32',
> +'*share': 'bool',
> +'size': 'size',
> +'*x-use-canonical-path-for-ramblock-id': 'bool' } }
> +
> +##
> +# @MemoryBackendFileProperties:
> +#
> +# Properties for memory-backend-file objects.
> +#
> +# @align: the base address alignment when QEMU mmap(2) @mem-path. Some
> +# backend store specified by @mem-path requires an alignment
> different
Grammar feels off. Would it read better as
...when QEMU mmap(2)s @mem-path. Some backend stores specified by
@mem-path require an...
> +# than the default one used by QEMU, e.g. the device DAX /dev/dax0.0
> +# requires 2M alignment rather than 4K. In such cases, users can
> +# specify the required alignment via this option.
> +# 0 selects a default alignment (currently the page size). (default:
> 0)
Again, not in the same order as
backends/hostmem-file.c:file_backend_class_init(), but it matches up.
> +#
> +# @discard-data: if true, the file contents can be destroyed when QEMU exits,
> +#to avoid unnecessarily flushing data to the backing file.
> Note
> +#that ``discard-data`` is only an optimization, and QEMU
> might
> +#not discard file contents if it aborts unexpectedly or is
> +#terminated using SIGKILL. (default: false)
> +#
> +# @mem-path: the path to either a shared memory or huge page filesystem mount
> +#
> +# @pmem: specifies whether the backing file specified by @mem-path is in
> +#host persistent memory that can be accessed using the SNIA NVM
> +#programming model (e.g. Intel NVDIMM).
> +#
> +# @readonly: if true, the backing file is opened read-only; if false, it is
> +#opened read-write. (default: false)
> +#
> +# Since: 2.1
> +##
> +{ 'struct': 'MemoryBackendFileProperties',
> + 'base': 'MemoryBackendProperties',
> + 'data': { '*align': 'size',
> +'*discard-data': 'bool',
> +'mem-path': 'str',
> +'*pmem': 'bool',
To match the C code, this should be
'*pmem': { 'type':'bool', 'if':'defined(CONFIG_LIBPMEM)' },
> +'*readonly': 'bool' } }
> +
> +##
> +# @MemoryBackendMemfdProperties:
>
Re: [PATCH v2 06/31] qapi/qom: Add ObjectOptions for dbus-vmstate
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the dbus-vmstate object.
>
> A list represented as a comma separated string is clearly not very
> QAPI-like, but for now just describe the existing interface.
Does your alias proposal give us a path forward for improving that down
the road? Or maybe it's not an alias we need, but a new field with
better QAPI-like semantics, deprecate the old one, and wait out the 2
release cycles?
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 1dbc95fb53..a6a5049707 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -232,6 +232,22 @@
>'base': 'CryptodevBackendProperties',
>'data': { 'chardev': 'str' } }
>
> +##
> +# @DBusVMStateProperties:
> +#
> +# Properties for dbus-vmstate objects.
> +#
> +# @addr: the name of the DBus bus to connect to
> +#
> +# @id-list: a comma separated list of DBus IDs of helpers whose data should
> be
> +# included in the VM state on migration
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'DBusVMStateProperties',
> + 'data': { 'addr': 'str' ,
> +'*id-list': 'str' } }
Matches backends/dbus-vmstate.c:dbus_vmstate_class_init(), including
splitting id-list into a GHashTable with get_id_list_set().
Since there is benefit to documenting/converting our existing API in
this series without dragging it out by also trying to fix the warts,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 05/31] qapi/qom: Add ObjectOptions for cryptodev-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the cryptodev-* objects.
>
> These interfaces have some questionable aspects (cryptodev-backend is
> really an abstract base class without function, and the queues option
> only makes sense for cryptodev-vhost-user), but as the goal is to
> represent the existing interface in QAPI, leave these things in place.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 34 ++
> 1 file changed, 34 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 30ed179bc1..1dbc95fb53 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -204,6 +204,34 @@
>'returns': [ 'ObjectPropertyInfo' ],
>'allow-preconfig': true }
>
> +##
> +# @CryptodevBackendProperties:
> +#
> +# Properties for cryptodev-backend and cryptodev-backend-builtin objects.
> +#
> +# @queues: the number of queues for the cryptodev backend. Ignored for
> +# cryptodev-backend and must be 1 for cryptodev-backend-builtin.
> +# (default: 1)
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'CryptodevBackendProperties',
> + 'data': { '*queues': 'uint32' } }
Matches backend/cryptodev.c:cryptodev_backend_class_init() and
backend/cryptodev-builtin.c:cryptodev_builtin_class_init().
> +
> +##
> +# @CryptodevVhostUserProperties:
> +#
> +# Properties for cryptodev-vhost-user objects.
> +#
> +# @chardev: the name of a unix domain socket character device that connects
> to
Should that b s/unix/Unix/ ?
> +# the vhost-user server
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'CryptodevVhostUserProperties',
> + 'base': 'CryptodevBackendProperties',
> + 'data': { 'chardev': 'str' } }
Matches backend/cryptodev-vhost-user.c:cryptodev_vhost_user_init_class().
> +
> ##
> # @IothreadProperties:
> #
> @@ -239,6 +267,9 @@
> 'authz-listfile',
> 'authz-pam',
> 'authz-simple',
> +'cryptodev-backend',
> +'cryptodev-backend-builtin',
> +'cryptodev-vhost-user',
> 'iothread'
>] }
>
> @@ -262,6 +293,9 @@
>'authz-listfile': 'AuthZListFileProperties',
>'authz-pam': 'AuthZPAMProperties',
>'authz-simple': 'AuthZSimpleProperties',
> + 'cryptodev-backend': 'CryptodevBackendProperties',
> + 'cryptodev-backend-builtin': 'CryptodevBackendProperties',
> + 'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
>'iothread': 'IothreadProperties'
>} }
>
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 04/31] qapi/qom: Add ObjectOptions for authz-*
On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the authz-* objects.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/authz.json | 62
> qapi/qom.json| 10 +
> storage-daemon/qapi/qapi-schema.json | 1 +
> 3 files changed, 73 insertions(+)
>
> diff --git a/qapi/authz.json b/qapi/authz.json
> index 42afe752d1..99d49aa563 100644
> --- a/qapi/authz.json
> +++ b/qapi/authz.json
> @@ -59,3 +59,65 @@
> ##
> { 'struct': 'QAuthZListRuleListHack',
>'data': { 'unused': ['QAuthZListRule'] } }
This hack is no longer necessary...
> +
> +##
> +# @AuthZListProperties:
> +#
> +# Properties for authz-list objects.
> +#
> +# @policy: Default policy to apply when no rule matches (default: deny)
> +#
> +# @rules: Authorization rules based on matching user
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AuthZListProperties',
> + 'data': { '*policy': 'QAuthZListPolicy',
> +'*rules': ['QAuthZListRule'] } }
...now that we have a real type using the same array and forcing the
QAPI generator to instantiate it.
Matches authz/list.c:qauthz_list_class_init().
> +
> +##
> +# @AuthZListFileProperties:
> +#
> +# Properties for authz-listfile objects.
> +#
> +# @filename: File name to load the configuration from. The file must
> +#contain valid JSON for AuthZListProperties.
> +#
> +# @refresh: If true, inotify is used to monitor the file, automatically
> +# reloading changes. If an error occurs during reloading, all
> +# authorizations will fail until the file is next successfully
> +# loaded. (default: true if the binary was built with
> +# CONFIG_INOTIFY1, false otherwise)
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AuthZListFileProperties',
> + 'data': { 'filename': 'str',
> +'*refresh': 'bool' } }
Matches authz/listfile.c:qauthz_list_file_class_init().
> +
> +##
> +# @AuthZPAMProperties:
> +#
> +# Properties for authz-pam objects.
> +#
> +# @service: PAM service name to use for authorization
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AuthZPAMProperties',
> + 'data': { 'service': 'str' } }
Matches authz/pamacct.c:qauthz_pam_class_init().
> +
> +##
> +# @AuthZSimpleProperties:
> +#
> +# Properties for authz-simple objects.
> +#
> +# @identity: Identifies the allowed user. Its format depends on the network
> +#service that authorization object is associated with. For
> +#authorizing based on TLS x509 certificates, the identity must be
> +#the x509 distinguished name.
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AuthZSimpleProperties',
> + 'data': { 'identity': 'str' } }
Matches authz/simple.c:qauthz_simple_class_init().
> diff --git a/qapi/qom.json b/qapi/qom.json
> index bf2ecb34be..30ed179bc1 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -4,6 +4,8 @@
> # This work is licensed under the terms of the GNU GPL, version 2 or later.
> # See the COPYING file in the top-level directory.
>
> +{ 'include': 'authz.json' }
> +
> ##
> # = QEMU Object Model (QOM)
> ##
> @@ -233,6 +235,10 @@
> ##
> { 'enum': 'ObjectType',
>'data': [
> +'authz-list',
> +'authz-listfile',
> +'authz-pam',
> +'authz-simple',
> 'iothread'
>] }
>
> @@ -252,6 +258,10 @@
> 'id': 'str' },
>'discriminator': 'qom-type',
>'data': {
> + 'authz-list': 'AuthZListProperties',
> + 'authz-listfile': 'AuthZListFileProperties',
> + 'authz-pam': 'AuthZPAMProperties',
> + 'authz-simple': 'AuthZSimpleProperties',
>'iothread': 'IothreadProperties'
>} }
>
> diff --git a/storage-daemon/qapi/qapi-schema.json
> b/storage-daemon/qapi/qapi-schema.json
> index 28117c3aac..67749d1101 100644
> --- a/storage-daemon/qapi/qapi-schema.json
> +++ b/storage-daemon/qapi/qapi-schema.json
> @@ -26,6 +26,7 @@
> { 'include': '../../qapi/crypto.json' }
> { 'include': '../../qapi/introspect.json' }
> { 'include': '../../qapi/job.json' }
> +{ 'include': '../../qapi/authz.json' }
> { 'include': '../../qapi/qom.json' }
> { 'include': '../../qapi/sockets.json' }
> { 'include': '../../qapi/transaction.json' }
>
Once you delete the dead QAPI hack,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 19/31] qapi/qom: QAPIfy object-add
On 24/02/21 14:52, Kevin Wolf wrote: +v = qobject_output_visitor_new(); +visit_type_ObjectOptions(v, NULL, , _abort); +visit_complete(v, ); +visit_free(v); + +props = qobject_to(QDict, qobj); +qdict_del(props, "qom-type"); +qdict_del(props, "id"); + +v = qobject_input_visitor_new(QOBJECT(props)); +obj = user_creatable_add_type(ObjectType_str(options->qom_type), + options->id, props, v, errp); +object_unref(obj); Please add a check in object_property_add_child that the id is well formed (using the id_wellformed function). This is pre-existing, but it becomes a regression for -object later in the series. Thanks, Paolo
[PATCH v6 0/5] migration/dirtyrate: Introduce APIs for getting domain memory dirty rate
V5 -> V6: split DomainGetDirtyRateInfo(domdirtyrate) API into two parts: 1. DomainStartDirtyRateCalc(domdirtyrate-calc) for starting dirty rate calculation; 2. qemuDomainGetStatsDirtyRate(domstats --dirtyrate) for querying dirty rate infomation. V4 -> V5: squash 1/7 and bits of 5/7 and 6/7 into 2/7 in v4 (to be 1/5 in v5) squash left of 5/7 into 4/7 in v4 (to be 3/5 in v5) add VIR_DOMAIN_DIRTYRATE_DEFAULT flag remove redundant error report rename virsh api to "domdirtyrate" use vshTablePtr for virsh api output add description in docs/manpages/virsh.rst other format optimize V3 -> V4: define flags to unsigned int fix some compile warnings V2 -> V3: reorganize patchset to fix compile warning V1 -> V2: replace QEMU_JOB_ASYNC with QEMU_JOB_QUERY Sometimes domain's memory dirty rate is expected by user in order to decide whether it's proper to be migrated out or not. We have already completed the QEMU part of the capability: https://patchew.org/QEMU/1600237327-33618-1-git-send-email-zhengch...@huawei.com/ And this serial of patches introduce the corresponding LIBVIRT part: 1. Calculating Introduce a new API DomainStartDirtyRateCalc and corresponding virsh api (domdirtyrate-calc) for starting dirty rate calculation by calling qmp 'calc-dirty-rate'. # virsh domdirtyrate-calc [--seconds ] 2. Querying Introduce command 'virsh domstats --dirtyrate' for reporting memory dirty rate infomation by calling qmp 'query-dirty-rate'. The info is listed as: Domain: 'vm0' dirtyrate.calc_status=measured dirtyrate.calc_start_time=502814 dirtyrate.calc_period=1 dirtyrate.megabytes_per_second=2 Hao Wang (5): migration/dirtyrate: Introduce virDomainStartDirtyRateCalc API migration/dirtyrate: Implement qemuDomainStartDirtyRateCalc migration/dirtyrate: Introduce domdirtyrate-calc virsh api migration/dirtyrate: Implement qemuMonitorQueryDirtyRate migration/dirtyrate: Introduce command 'virsh domstats --dirtyrate' docs/manpages/virsh.rst | 33 - include/libvirt/libvirt-domain.h | 5 ++ src/driver-hypervisor.h | 6 ++ src/libvirt-domain.c | 57 +++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 115 +++ src/qemu/qemu_monitor.c | 24 +++ src/qemu/qemu_monitor.h | 18 + src/qemu/qemu_monitor_json.c | 88 +++ src/qemu/qemu_monitor_json.h | 8 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++- src/remote_protocol-structs | 6 ++ tools/virsh-domain-monitor.c | 7 ++ tools/virsh-domain.c | 63 + 15 files changed, 447 insertions(+), 3 deletions(-) -- 2.23.0
[PATCH v6 4/5] migration/dirtyrate: Implement qemuMonitorQueryDirtyRate
Implement qemuMonitorQueryDirtyRate which query domain's memory
dirty rate calling qmp "query-dirty-rate".
Signed-off-by: Hao Wang
---
src/qemu/qemu_monitor.c | 12 +++
src/qemu/qemu_monitor.h | 14
src/qemu/qemu_monitor_json.c | 66
src/qemu/qemu_monitor_json.h | 4 +++
4 files changed, 96 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 05c57f5f1d..dcc529b405 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4754,3 +4754,15 @@ qemuMonitorStartDirtyRateCalc(qemuMonitorPtr mon,
return qemuMonitorJSONStartDirtyRateCalc(mon, seconds);
}
+
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+ qemuMonitorDirtyRateInfoPtr info)
+{
+VIR_DEBUG("info=%p", info);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONQueryDirtyRate(mon, info);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 94813906b4..9958657c2c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1530,3 +1530,17 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
int
qemuMonitorStartDirtyRateCalc(qemuMonitorPtr mon,
int seconds);
+
+typedef struct _qemuMonitorDirtyRateInfo qemuMonitorDirtyRateInfo;
+typedef qemuMonitorDirtyRateInfo *qemuMonitorDirtyRateInfoPtr;
+
+struct _qemuMonitorDirtyRateInfo {
+char *status; /* the status of last dirtyrate calculation */
+long long dirtyRate;/* the dirtyrate in MiB/s */
+long long startTime;/* the start time of dirtyrate calculation */
+int calcTime; /* the period of dirtyrate calculation */
+};
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+ qemuMonitorDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e6c127ab61..409fb817bf 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9505,3 +9505,69 @@ qemuMonitorJSONStartDirtyRateCalc(qemuMonitorPtr mon,
return 0;
}
+
+
+static int
+qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
+qemuMonitorDirtyRateInfoPtr info)
+{
+const char *status;
+
+if (!(status = virJSONValueObjectGetString(data, "status"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'status' data"));
+return -1;
+}
+info->status = g_strdup(status);
+
+/* `query-dirty-rate` replies `dirty-rate` data only if the status of the
latest
+ * calculation is `measured`.
+ */
+if (STREQ(info->status, "measured") &&
+(virJSONValueObjectGetNumberLong(data, "dirty-rate", >dirtyRate)
< 0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'dirty-rate'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberLong(data, "start-time", >startTime)
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'start-time'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberInt(data, "calc-time", >calcTime) <
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'calc-time'
data"));
+return -1;
+}
+
+return 0;
+}
+
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+ qemuMonitorDirtyRateInfoPtr info)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+virJSONValuePtr data = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'return' data"));
+return -1;
+}
+
+return qemuMonitorJSONExtractDirtyRateInfo(data, info);
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 32782cf681..8d4232f67b 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -715,3 +715,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
int
qemuMonitorJSONStartDirtyRateCalc(qemuMonitorPtr mon,
int seconds);
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+ qemuMonitorDirtyRateInfoPtr info);
--
2.23.0
[PATCH v6 5/5] migration/dirtyrate: Introduce command 'virsh domstats --dirtyrate'
Introduce command 'virsh domstats --dirtyrate' for reporting memory
dirty rate infomation. The info is listed as:
Domain: 'vm0'
dirtyrate.calc_status=measured
dirtyrate.calc_start_time=502814
dirtyrate.calc_period=1
dirtyrate.megabytes_per_second=2
Signed-off-by: Hao Wang
---
docs/manpages/virsh.rst | 16 +++--
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 13
src/qemu/qemu_driver.c | 56
tools/virsh-domain-monitor.c | 7
5 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 417ea444f4..b0fab3e781 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2219,7 +2219,7 @@ domstats
domstats [--raw] [--enforce] [--backing] [--nowait] [--state]
[--cpu-total] [--balloon] [--vcpu] [--interface]
- [--block] [--perf] [--iothread] [--memory]
+ [--block] [--perf] [--iothread] [--memory] [--dirtyrate]
[[--list-active] [--list-inactive]
[--list-persistent] [--list-transient] [--list-running]y
[--list-paused] [--list-shutoff] [--list-other]] | [domain ...]
@@ -2238,7 +2238,8 @@ behavior use the *--raw* flag.
The individual statistics groups are selectable via specific flags. By
default all supported statistics groups are returned. Supported
statistics groups flags are: *--state*, *--cpu-total*, *--balloon*,
-*--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*.
+*--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*,
+*--dirtyrate*.
Note that - depending on the hypervisor type and version or the domain state
- not all of the following statistics may be returned.
@@ -2431,6 +2432,17 @@ not available for statistical purposes.
bytes consumed by @vcpus that passing through all memory controllers, either
local or remote controller.
+*--dirtyrate* returns:
+
+* ``dirtyrate.calc_status`` - the status of last memory dirty rate
+ calculation
+* ``dirtyrate.calc_start_time`` - the start time of last memory dirty
+ rate calculation
+* ``dirtyrate.calc_period`` - the period of last memory dirty rate
+ calculation
+* ``dirtyrate.megabytes_per_second`` - the calculated memory dirty
+ rate in MiB/s
+
Selecting a specific statistics groups doesn't guarantee that the
daemon supports the selected group of stats. Flag *--enforce*
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7aa5ef4543..81371f54b7 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2185,6 +2185,7 @@ typedef enum {
VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
VIR_DOMAIN_STATS_IOTHREAD = (1 << 7), /* return iothread poll info */
VIR_DOMAIN_STATS_MEMORY = (1 << 8), /* return domain memory info */
+VIR_DOMAIN_STATS_DIRTYRATE = (1 << 9), /* return domain dirty rate info */
} virDomainStatsTypes;
typedef enum {
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index b1cc2ebbf3..0a03b2ca90 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11883,6 +11883,19 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* bytes consumed by @vcpus that passing through all
* memory controllers, either local or remote controller.
*
+ * VIR_DOMAIN_STATS_DIRTYRATE:
+ * Return memory dirty rate information. The typed parameter keys are in
+ * this format:
+ *
+ * "dirtyrate.calc_status" - the status of last memory dirty rate
+ * calculation
+ * "dirtyrate.calc_start_time" - the start time of last memory dirty
+ * rate calculation
+ * "dirtyrate.calc_period" - the period of last memory dirty rate
+ * calculation
+ * "dirtyrate.megabytes_per_second" - the calculated memory dirty
+ *rate in MiB/s
+ *
* Note that entire stats groups or individual stat fields may be missing from
* the output in case they are not supported by the given hypervisor, are not
* applicable for the current state of the guest domain, or their retrieval
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7420937cf7..f847650e22 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18535,6 +18535,61 @@ qemuDomainGetStatsPerf(virQEMUDriverPtr driver
G_GNUC_UNUSED,
return 0;
}
+static int
+qemuDomainGetStatsDirtyRateMon(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMonitorDirtyRateInfoPtr info)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret;
+
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorQueryDirtyRate(priv->mon, info);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
+
+static int
[PATCH v6 2/5] migration/dirtyrate: Implement qemuDomainStartDirtyRateCalc
Implement qemuDomainStartDirtyRateCalc which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".
Signed-off-by: Hao Wang
---
src/qemu/qemu_driver.c | 59
src/qemu/qemu_monitor.c | 12
src/qemu/qemu_monitor.h | 4 +++
src/qemu/qemu_monitor_json.c | 22 ++
src/qemu/qemu_monitor_json.h | 4 +++
5 files changed, 101 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9bbdf8d48..7420937cf7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20302,6 +20302,64 @@ qemuDomainGetMessages(virDomainPtr dom,
}
+#define MIN_DIRTYRATE_CALC_PERIOD 1 /* supported min dirtyrate calculating
time: 1s */
+#define MAX_DIRTYRATE_CALC_PERIOD 60 /* supported max dirtyrate calculating
time: 60s */
+
+static int
+qemuDomainStartDirtyRateCalc(virDomainPtr dom,
+ int seconds,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (seconds < MIN_DIRTYRATE_CALC_PERIOD ||
+seconds > MAX_DIRTYRATE_CALC_PERIOD) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("seconds=%d is invalid, please choose value within
[%d, %d]."),
+ seconds,
+ MIN_DIRTYRATE_CALC_PERIOD,
+ MAX_DIRTYRATE_CALC_PERIOD);
+return -1;
+}
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+return -1;
+
+if (virDomainStartDirtyRateCalcEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+goto endjob;
+}
+
+VIR_DEBUG("Calculate dirty rate in next %d seconds", seconds);
+
+priv = vm->privateData;
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorStartDirtyRateCalc(priv->mon, seconds);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+
static virHypervisorDriver qemuHypervisorDriver = {
.name = QEMU_DRIVER_NAME,
.connectURIProbe = qemuConnectURIProbe,
@@ -20544,6 +20602,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainAuthorizedSSHKeysGet = qemuDomainAuthorizedSSHKeysGet, /* 6.10.0 */
.domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */
.domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */
+.domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */
};
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 73f337a6be..05c57f5f1d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4742,3 +4742,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
return qemuMonitorJSONTransactionBackup(actions, device, jobname, target,
bitmap, syncmode);
}
+
+
+int
+qemuMonitorStartDirtyRateCalc(qemuMonitorPtr mon,
+ int seconds)
+{
+VIR_DEBUG("seconds=%d", seconds);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONStartDirtyRateCalc(mon, seconds);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d25c26343a..94813906b4 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1526,3 +1526,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
const char *target,
const char *bitmap,
qemuMonitorTransactionBackupSyncMode syncmode);
+
+int
+qemuMonitorStartDirtyRateCalc(qemuMonitorPtr mon,
+ int seconds);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b2034b3a40..e6c127ab61 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9483,3 +9483,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
migratable);
}
+
+
+int
+qemuMonitorJSONStartDirtyRateCalc(qemuMonitorPtr mon,
+ int seconds)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
+ "i:calc-time", seconds,
+ NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+return 0;
+}
diff --git
[PATCH v6 1/5] migration/dirtyrate: Introduce virDomainStartDirtyRateCalc API
Introduce virDomainStartDirtyRateCalc API for start calculation of
a domain's memory dirty rate with a specified time.
Signed-off-by: Hao Wang
---
include/libvirt/libvirt-domain.h | 4 +++
src/driver-hypervisor.h | 6 +
src/libvirt-domain.c | 44
src/libvirt_public.syms | 5
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 14 +-
src/remote_protocol-structs | 6 +
7 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8011cf9fe1..7aa5ef4543 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5128,4 +5128,8 @@ int virDomainGetMessages(virDomainPtr domain,
char ***msgs,
unsigned int flags);
+int virDomainStartDirtyRateCalc(virDomainPtr domain,
+int seconds,
+unsigned int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 05d7dfb5c7..2ec7b8b24a 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1405,6 +1405,11 @@ typedef int
char ***msgs,
unsigned int flags);
+typedef int
+(*virDrvDomainStartDirtyRateCalc)(virDomainPtr domain,
+ int seconds,
+ unsigned int flags);
+
typedef struct _virHypervisorDriver virHypervisorDriver;
typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1671,4 +1676,5 @@ struct _virHypervisorDriver {
virDrvDomainAuthorizedSSHKeysGet domainAuthorizedSSHKeysGet;
virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet;
virDrvDomainGetMessages domainGetMessages;
+virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc;
};
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4af0166872..b1cc2ebbf3 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13154,3 +13154,47 @@ virDomainGetMessages(virDomainPtr domain,
virDispatchError(conn);
return -1;
}
+
+
+/**
+ * virDomainStartDirtyRateCalc:
+ * @domain: a domain object
+ * @seconds: specified calculating time in seconds
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Calculate the current domain's memory dirty rate in next @seconds.
+ * The calculated dirty rate infomation is available by calling
+ * virConnectGetAllDomainStats.
+ *
+ * Returns 0 in case of success, -1 otherwise.
+ */
+int
+virDomainStartDirtyRateCalc(virDomainPtr domain,
+int seconds,
+unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "seconds=%d, flags=0x%x", seconds, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+virCheckReadOnlyGoto(conn->flags, error);
+
+if (conn->driver->domainStartDirtyRateCalc) {
+int ret;
+ret = conn->driver->domainStartDirtyRateCalc(domain, seconds, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index d851333eb0..51a3d7265a 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -884,4 +884,9 @@ LIBVIRT_7.1.0 {
virDomainGetMessages;
} LIBVIRT_6.10.0;
+LIBVIRT_7.2.0 {
+global:
+virDomainStartDirtyRateCalc;
+} LIBVIRT_7.1.0;
+
# define new API here using predicted next version number
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a83cd866e7..3968472931 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8575,6 +8575,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainAuthorizedSSHKeysGet = remoteDomainAuthorizedSSHKeysGet, /* 6.10.0
*/
.domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0
*/
.domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */
+.domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d3724bc305..7fdc65f029 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3811,6 +3811,12 @@ struct remote_domain_get_messages_ret {
remote_nonnull_string msgs;
};
+struct remote_domain_start_dirty_rate_calc_args {
+remote_nonnull_domain dom;
+int seconds;
+unsigned int flags;
+};
+
/*- Protocol. -*/
@@ -6733,5 +6739,11 @@ enum remote_procedure {
* @generate: none
* @acl: domain:read
*/
-REMOTE_PROC_DOMAIN_GET_MESSAGES = 426
+REMOTE_PROC_DOMAIN_GET_MESSAGES = 426,
+
+
[PATCH v6 3/5] migration/dirtyrate: Introduce domdirtyrate-calc virsh api
Introduce domdirtyrate-calc virsh api to start calculating domain's
memory dirty rate:
# virsh domdirtyrate-calc [--seconds ]
Signed-off-by: Hao Wang
---
docs/manpages/virsh.rst | 17 +++
tools/virsh-domain.c| 63 +
2 files changed, 80 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 8a4328faa0..417ea444f4 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -1704,6 +1704,23 @@ states other than "ok" or "error" the command also
prints number of
seconds elapsed since the control interface entered its current state.
+domdirtyrate-calc
+-
+
+**Syntax:**
+
+::
+
+ domdirtyrate-calc [--seconds ]
+
+Calculate an active domain's memory dirty rate which may be expected by
+user in order to decide whether it's proper to be migrated out or not.
+The ``seconds`` parameter can be used to calculate dirty rate in a
+specific time which allows 60s at most now and would be default to 1s
+if missing. The calculated dirty rate infomation is available by calling
+'domstats --dirtyrate'.
+
+
domdisplay
--
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index df33467646..ccb5d61a25 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14412,6 +14412,63 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd)
}
+/*
+ * "domdirtyrate" command
+ */
+static const vshCmdInfo info_domdirtyrate_calc[] = {
+{.name = "help",
+ .data = N_("Calculate a vm's memory dirty rate")
+},
+{.name = "desc",
+ .data = N_("Calculate memory dirty rate of a domain in order to decide
whether "
+"it's proper to be migrated out or not.\n"
+"The calculated dirty rate infomation is available by calling "
+"'domstats --dirtyrate'.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_domdirtyrate_calc[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+{.name = "seconds",
+ .type = VSH_OT_INT,
+ .help = N_("calculate memory dirty rate within specified seconds, "
+"the supported value range from 1 to 60, default to 1.")
+},
+{.name = NULL}
+};
+
+static bool
+cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+int seconds;
+int rc;
+bool ret = false;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+rc = vshCommandOptInt(ctl, cmd, "seconds", );
+if (rc < 0)
+goto cleanup;
+
+/* if no inputted seconds, default to 1s */
+if (!rc)
+seconds = 1;
+
+if (virDomainStartDirtyRateCalc(dom, seconds, 0) < 0)
+goto cleanup;
+
+vshPrint(ctl, _("Start to calculate domain's memory dirty rate
successfully.\n"));
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+return ret;
+}
+
+
const vshCmdDef domManagementCmds[] = {
{.name = "attach-device",
.handler = cmdAttachDevice,
@@ -15051,5 +15108,11 @@ const vshCmdDef domManagementCmds[] = {
.info = info_guestinfo,
.flags = 0
},
+{.name = "domdirtyrate-calc",
+ .handler = cmdDomDirtyRateCalc,
+ .opts = opts_domdirtyrate_calc,
+ .info = info_domdirtyrate_calc,
+ .flags = 0
+},
{.name = NULL}
};
--
2.23.0
