Re: [libvirt PATCH] util: don't log error if SRIOV PF has no associated netdev
Ahem.. Hello? Is this thing on?? :-)
On 3/3/21 3:34 PM, Laine Stump wrote:
ping
On 2/23/21 10:35 PM, Laine Stump wrote:
Some SRIOV PFs don't have a netdev associated with them (the spec
apparently doesn't require it). In most cases when libvirt is dealing
with an SRIOV VF, that VF must have a PF, and the PF *must* have an
associated netdev (the only way to set the MAC address of a VF is by
sending a netlink message to the netdev of that VF's PF). But there
are times when we don't need for the PF to have a netdev; in
particular, when we're just getting the Switchdev Features for a VF,
we don't need the PF netdev - the netdev of the VF (apparently) works
just as well.
Commit 6452e2f5 (libvirt 5.1.0) *kind of* made libvirt work around PFs
with no netdevs in this case - if virNetDevGetPhysicalFunction
returned an error when setting up to retrieve Switchdev feature info,
it would ignore the error, and then check if the PF netdev name was
NULL and, if so it would reset the error object and continue on rather
than returning early with a failure. The problem is that by the time
this special handling occured, the error message about missing netdev
had already been logged, which was harmless to proper operation, but
confused the user.
Fortunately there are only 2 users of virNetDevGetPhysicalFunction, so
it is easy to redefine it's API to state that a missing netdev name is
*not* an error - in that case it will still return success, but the
caller must be prepared for the PF netdev name to be NULL. After
making this change, we can modify the two callers to behave properly
with the new semantics (for one of the callers it *is* still an error,
so the error message is moved there, but for the other it is okay to
continue), and our spurious error messages are a thing of the past.
Resolves: https://bugzilla.redhat.com/1924616
Fixes: 6452e2f5e1837bd753ee465e6607ed3c4f62b815
Signed-off-by: Laine Stump
---
src/util/virnetdev.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2b4c8b6280..7b766234ec 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1339,7 +1339,8 @@ virNetDevGetVirtualFunctionIndex(const char
*pfname, const char *vfname,
*
* @ifname : name of the physical function interface name
* @pfname : Contains sriov physical function for interface ifname
- * upon successful return
+ * upon successful return (might be NULL if the PF has no
+ * associated netdev. This is *not* an error)
*
* Returns 0 on success, -1 on failure
*
@@ -1361,15 +1362,6 @@ virNetDevGetPhysicalFunction(const char
*ifname, char **pfname)
return -1;
}
- if (!*pfname) {
- /* The SRIOV standard does not require VF netdevs to have
- * the netdev assigned to a PF. */
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("The PF device for VF %s has no network
device name"),
- ifname);
- return -1;
- }
-
return 0;
}
@@ -1442,6 +1434,17 @@ virNetDevGetVirtualFunctionInfo(const char
*vfname, char **pfname,
if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
return -1;
+ if (!*pfname) {
+ /* The SRIOV standard does not require VF netdevs to have the
+ * netdev assigned to a PF, but our method of retrieving
+ * VFINFO does.
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("The PF device for VF %s has no network
device name, cannot get virtual function info"),
+ vfname);
+ return -1;
+ }
+
if (virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf) < 0)
goto cleanup;
@@ -3204,12 +3207,8 @@ virNetDevSwitchdevFeature(const char *ifname,
if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0)
return ret;
- if (is_vf == 1) {
- /* Ignore error if PF does not have netdev assigned.
- * In that case pfname == NULL. */
- if (virNetDevGetPhysicalFunction(ifname, ) < 0)
- virResetLastError();
- }
+ if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, ) < 0)
+ return ret;
pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
virNetDevGetPCIDevice(ifname);
Re: [PATCH] virnetdevbandwidth: Don't generate burst outside of boundaries
On 3/5/21 6:11 AM, Michal Privoznik wrote: When generating TC rules for domain's outbound traffic, Libvirt will use the 'average' as the default for 'burst' - it's been this way since the feature introduction in v0.9.4-rc1~22. The reason is that 'average' considers 'burst' for policing. However, when parsing its command line TC uses an unsigned int (with overflow detection) to store the 'burst' size. This means, that the upper limit for the value is UINT_MAX, well UINT_MAX / 1024 because we are putting the value in KiB onto the command line. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912210 Signed-off-by: Michal Privoznik Reviewed-by: Laine Stump
Re: [PATCH] qemu: don't raise error upon interface update without for in coalesce
On Thu, Mar 04, 2021 at 01:58:17PM +0100, Kristina Hanicova wrote: With this, incomplete XML without for in coalesce won't raise error as before. It will leave the coalesce parameter empty, thanks to passing it as a parameter and return an integer to indicate error state - previously it returned pointer (or NULL for both error and incomplete XML). The code went through some refactoring: * change of a condition * addition of a parameter * change of order, that allowed removal of VIR_FREE * removal of redundant labels and variables Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1535930 Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) The code is good, but it could use some test(s). I guess you have couple of options here: - just show that parsing it does nothing in simple qemuxml2xmltest - make sure that this makes it possible to remove the coalesce settings in qemuhotplugtest. This might not be the case and it might result in more patches because, honestly, I am not 100% sure how to handle removal of coalesce parameters versus not touching them on update. Since this is not a critical thing to do, I'll leave that up to you to decide how to approach this ;) Thanks, Martin signature.asc Description: PGP signature
Re: [PATCH v3 29/30] vl: QAPIfy -object
On 3/8/21 10:54 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. Maybe worth a tweak to this paragraph now that b979c931 has landed formally deprecating -writeconfig (all the more reason we don't care about it). > > Signed-off-by: Kevin Wolf > Acked-by: Peter Krempa > Reviewed-by: Eric Blake R-b stands either way. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object
On 3/8/21 10:54 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
> Acked-by: Peter Krempa
> ---
> qemu-img.c | 251 ++---
> 1 file changed, 45 insertions(+), 206 deletions(-)
>
> @@ -1353,7 +1303,7 @@ static int check_empty_sectors(BlockBackend *blk,
> int64_t offset,
> /*
> * Compares two images. Exit codes:
> *
> - * 0 - Images are identical
> + * 0 - Images are identical or the requested help was printed
Nice, but does the user-facing documentation need updating to match?
> * 1 - Images differ
> * >1 - Error occurred
> */
> @@ -1423,15 +1373,21 @@ 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;
> +case OPTION_OBJECT:
> +{
> +Error *local_err = NULL;
> +
> +if (!user_creatable_add_from_str(optarg, _err)) {
> +if (local_err) {
> +error_report_err(local_err);
> +exit(2);
> +} else {
> +/* Help was printed */
> +exit(EXIT_SUCCESS);
> +}
The commit message needs to be updated to call out that this bug fix was
intentional, preferably mentioning the commit where we broke it
(334c43e2c3).
The code is fine, though, so with an improved commit message (and maybe
some matching doc tweaks),
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 06/30] qapi/qom: Add ObjectOptions for memory-backend-*
On 3/8/21 10:54 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
> Acked-by: Peter Krempa
> ---
> qapi/common.json | 20
> qapi/machine.json | 22 +
> qapi/qom.json | 121 +-
> 3 files changed, 141 insertions(+), 22 deletions(-)
>
> @@ -287,7 +397,10 @@
> 'cryptodev-backend-builtin',
> 'cryptodev-vhost-user',
> 'dbus-vmstate',
> -'iothread'
> +'iothread',
> +'memory-backend-file',
> +'memory-backend-memfd',
> +'memory-backend-ram'
Another leaked enum value...
>] }
>
> ##
> @@ -315,7 +428,11 @@
>'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
>'if': 'defined(CONFIG_VIRTIO_CRYPTO)
> && defined(CONFIG_VHOST_CRYPTO)' },
>'dbus-vmstate': 'DBusVMStateProperties',
> - 'iothread': 'IothreadProperties'
> + 'iothread': 'IothreadProperties',
> + 'memory-backend-file':'MemoryBackendFileProperties',
> + 'memory-backend-memfd': { 'type': 'MemoryBackendMemfdProperties',
> + 'if': 'defined(CONFIG_LINUX)' },
> + 'memory-backend-ram': 'MemoryBackendProperties'
>} }
...when compared to the union branches.
Once fixed,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 04/30] qapi/qom: Add ObjectOptions for cryptodev-*
On 3/8/21 10:54 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
> Acked-by: Peter Krempa
> ---
> qapi/qom.json | 35 +++
> 1 file changed, 35 insertions(+)
>
> @@ -239,6 +267,9 @@
> 'authz-listfile',
> 'authz-pam',
> 'authz-simple',
> +'cryptodev-backend',
> +'cryptodev-backend-builtin',
> +'cryptodev-vhost-user',
Shouldn't the enum value be conditional...
> 'iothread'
>] }
>
> @@ -262,6 +293,10 @@
>'authz-listfile': 'AuthZListFileProperties',
>'authz-pam': 'AuthZPAMProperties',
>'authz-simple': 'AuthZSimpleProperties',
> + 'cryptodev-backend': 'CryptodevBackendProperties',
> + 'cryptodev-backend-builtin': 'CryptodevBackendProperties',
> + 'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
> + 'if': 'defined(CONFIG_VIRTIO_CRYPTO)
> && defined(CONFIG_VHOST_CRYPTO)' },
...if the union branch is likewise?
>'iothread': 'IothreadProperties'
>} }
>
>
With that fixed,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Mon, 2021-03-08 at 15:57 +, Daniel P. Berrangé wrote: > On Mon, Mar 08, 2021 at 04:32:26PM +0100, Andrea Bolognani wrote: > > On Mon, 2021-03-08 at 13:17 +, Daniel P. Berrangé wrote: > > > Since you added code to parse existing limits from /proc, I'm wondering > > > if we can just do without the config option. Simply try to use prlimit > > > and if it fails, query existing limits to determine if we sould treat > > > the prlimit as fatal or ignore it. Overall I'd prefer libvirt to > > > "just work" out of the box rather than requiring people to know about > > > setting a "make-vfio-hotplug-work=yes" flag in the config file. > > > > The problem with that approach is what to do when *lowering* the > > limit, for example as a consequence of hot-unplugging the last VFIO > > device from the VM. > > > > If we're controlling the memory locking limit ourselves, then failure > > to lower it should be an error, because leaving the limit much higher > > than necessary creates potential for DoS by a compromised QEMU; on > > the other hand, if the limit is controlled by an external process, > > all we can really do is assume they will do the right thing after > > hot-unplugging has happened. > > IMHO once QEMU vCPUs start running, immediately assume QEMU is > compromised / hostile. IOW, the DoS risk arrived the moment it > was given the higher limit. We're just failing to close off the > existing risk we've already accepted, which doesn't worry me much. > > On unplug the only thing we actually do when memory lock reduce > fails is to log a warning message, it is never treated as a > fatal error. > > So the only difference is whether we skip the warning message > when we get EPERM from prlimit(), or always emit the warning. You're right, we're currently just soft-failing when we can't lower the memlock limit on unplug. Given this and your assessment of the security implications, which I trust, we should indeed be able to avoid introducing the qemu.conf knob and just behave sanely in all scenarios out of the box. I'll give it a try. -- Andrea Bolognani / Red Hat / Virtualization
[PATCH v3 22/30] qom: Factor out user_creatable_process_cmdline()
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
include/qom/object_interfaces.h | 41 +++
qom/object_interfaces.c | 50 +++-
qom/qom-qmp-cmds.c | 20 +--
storage-daemon/qemu-storage-daemon.c | 24 ++---
4 files changed, 80 insertions(+), 55 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 5299603f50..1e6c51b541 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -2,6 +2,7 @@
#define OBJECT_INTERFACES_H
#include "qom/object.h"
+#include "qapi/qapi-types-qom.h"
#include "qapi/visitor.h"
#define TYPE_USER_CREATABLE "user-creatable"
@@ -86,6 +87,18 @@ Object *user_creatable_add_type(const char *type, const char
*id,
const QDict *qdict,
Visitor *v, Error **errp);
+/**
+ * user_creatable_add_qapi:
+ * @options: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object according to the
+ * options passed in @opts as described in the QAPI schema documentation.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
+
/**
* user_creatable_add_opts:
* @opts: the object definition
@@ -131,6 +144,21 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * user_creatable_process_cmdline:
+ * @optarg: the object definition string as passed on the command line
+ *
+ * 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 and exit.
+ *
+ * This function is only meant to be called during command line parsing.
+ * It exits the process on failure or after printing help.
+ */
+void user_creatable_process_cmdline(const char *optarg);
+
/**
* user_creatable_print_help:
* @type: the QOM type to be added
@@ -145,19 +173,6 @@ int user_creatable_add_opts_foreach(void *opaque,
*/
bool user_creatable_print_help(const char *type, QemuOpts *opts);
-/**
- * user_creatable_print_help_from_qdict:
- * @args: options to create
- *
- * Prints help considering the other options given in @args (if "qom-type" is
- * given and valid, print properties for the type, otherwise print valid types)
- *
- * In contrast to user_creatable_print_help(), this function can't return that
- * no help was requested. It should only be called if we know that help is
- * requested and it will always print some help.
- */
-void user_creatable_print_help_from_qdict(QDict *args);
-
/**
* user_creatable_del:
* @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 02c3934329..2eaf9971f5 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -2,10 +2,13 @@
#include "qemu/cutils.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
#include "qom/object_interfaces.h"
#include "qemu/help_option.h"
#include "qemu/id.h"
@@ -113,6 +116,29 @@ out:
return obj;
}
+void user_creatable_add_qapi(ObjectOptions *options, Error **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);
+
+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);
+visit_free(v);
+}
+
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
Visitor *v;
@@ -256,7 +282,7 @@ bool user_creatable_print_help(const char *type, QemuOpts
*opts)
return false;
}
-void user_creatable_print_help_from_qdict(QDict *args)
+static void user_creatable_print_help_from_qdict(QDict *args)
{
[PATCH v3 30/30] qom: Drop QemuOpts based interfaces
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
include/qom/object_interfaces.h | 59
qom/object_interfaces.c | 81 -
tests/check-qom-proplist.c | 42 -
3 files changed, 20 insertions(+), 162 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index fb32330901..ac6c33ceac 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -99,51 +99,6 @@ Object *user_creatable_add_type(const char *type, const char
*id,
*/
void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
-/**
- * user_creatable_add_opts:
- * @opts: the object definition
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object whose type
- * is defined in @opts by the 'qom-type' option, placing it
- * in the object composition tree with name provided by the
- * 'id' field. The remaining options in @opts are used to
- * initialize the object properties.
- *
- * Returns: the newly created object or NULL on error
- */
-Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
-
-
-/**
- * user_creatable_add_opts_predicate:
- * @type: the QOM type to be added
- *
- * A callback function to determine whether an object
- * of type @type should be created. Instances of this
- * callback should be passed to user_creatable_add_opts_foreach
- */
-typedef bool (*user_creatable_add_opts_predicate)(const char *type);
-
-/**
- * user_creatable_add_opts_foreach:
- * @opaque: a user_creatable_add_opts_predicate callback or NULL
- * @opts: options to create
- * @errp: unused
- *
- * An iterator callback to be used in conjunction with
- * the qemu_opts_foreach() method for creating a list of
- * objects from a set of QemuOpts
- *
- * The @opaque parameter can be passed a user_creatable_add_opts_predicate
- * callback to filter which types of object are created during iteration.
- * When it fails, report the error.
- *
- * Returns: 0 on success, -1 when an error was reported.
- */
-int user_creatable_add_opts_foreach(void *opaque,
-QemuOpts *opts, Error **errp);
-
/**
* user_creatable_parse_str:
* @optarg: the object definition string as passed on the command line
@@ -190,20 +145,6 @@ bool user_creatable_add_from_str(const char *optarg, Error
**errp);
*/
void user_creatable_process_cmdline(const char *optarg);
-/**
- * user_creatable_print_help:
- * @type: the QOM type to be added
- * @opts: options to create
- *
- * Prints help if requested in @type or @opts. Note that if @type is neither
- * "help"/"?" nor a valid user creatable type, no help will be printed
- * regardless of @opts.
- *
- * Returns: true if a help option was found and help was printed, false
- * otherwise.
- */
-bool user_creatable_print_help(const char *type, QemuOpts *opts);
-
/**
* user_creatable_del:
* @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 62d7db7629..61d6d74a26 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -10,13 +10,10 @@
#include "qapi/qobject-input-visitor.h"
#include "qapi/qobject-output-visitor.h"
#include "qom/object_interfaces.h"
-#include "qemu/help_option.h"
#include "qemu/id.h"
#include "qemu/module.h"
#include "qemu/option.h"
#include "qemu/qemu-print.h"
-#include "qapi/opts-visitor.h"
-#include "qemu/config-file.h"
bool user_creatable_complete(UserCreatable *uc, Error **errp)
{
@@ -140,60 +137,6 @@ void user_creatable_add_qapi(ObjectOptions *options, Error
**errp)
visit_free(v);
}
-Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
-{
-Visitor *v;
-QDict *pdict;
-Object *obj;
-const char *id = qemu_opts_id(opts);
-char *type = qemu_opt_get_del(opts, "qom-type");
-
-if (!type) {
-error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-return NULL;
-}
-if (!id) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
-qemu_opt_set(opts, "qom-type", type, _abort);
-g_free(type);
-return NULL;
-}
-
-qemu_opts_set_id(opts, NULL);
-pdict = qemu_opts_to_qdict(opts, NULL);
-
-v = opts_visitor_new(opts);
-obj = user_creatable_add_type(type, id, pdict, v, errp);
-visit_free(v);
-
-qemu_opts_set_id(opts, (char *) id);
-qemu_opt_set(opts, "qom-type", type, _abort);
-g_free(type);
-qobject_unref(pdict);
-return obj;
-}
-
-
-int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
-{
-bool (*type_opt_predicate)(const char *, QemuOpts *) = opaque;
-Object *obj
[PATCH v3 29/30] vl: QAPIfy -object
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.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
softmmu/vl.c | 109 +++
1 file changed, 84 insertions(+), 25 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 10bd8a10a3..deb061cc78 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -113,6 +113,7 @@
#include "sysemu/replay.h"
#include "qapi/qapi-events-run-state.h"
#include "qapi/qapi-visit-block-core.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qapi-visit-ui.h"
#include "qapi/qapi-commands-block-core.h"
#include "qapi/qapi-commands-migration.h"
@@ -132,6 +133,14 @@ typedef struct BlockdevOptionsQueueEntry {
typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
+typedef struct ObjectOptionsQueueEntry {
+ObjectOptions *options;
+Location loc;
+QTAILQ_ENTRY(ObjectOptionsQueueEntry) next;
+} ObjectOptionsQueueEntry;
+
+typedef QTAILQ_HEAD(, ObjectOptionsQueueEntry) ObjectOptionsQueue;
+
static const char *cpu_option;
static const char *mem_path;
static const char *incoming;
@@ -143,6 +152,7 @@ static int snapshot;
static bool preconfig_requested;
static QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list);
static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
+static ObjectOptionsQueue obj_queue = QTAILQ_HEAD_INITIALIZER(obj_queue);
static bool nographic = false;
static int mem_prealloc; /* force preallocation of physical target memory */
static ram_addr_t ram_size;
@@ -1691,12 +1701,9 @@ static int machine_set_property(void *opaque,
* cannot be created here, as it depends on the chardev
* already existing.
*/
-static bool object_create_early(const char *type, QemuOpts *opts)
+static bool object_create_early(ObjectOptions *options)
{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-
+const char *type = ObjectType_str(options->qom_type);
/*
* Objects should not be made "delayed" without a reason. If you
* add one, state the reason in a comment!
@@ -1744,6 +1751,56 @@ static bool object_create_early(const char *type,
QemuOpts *opts)
return true;
}
+static void object_queue_create(bool early)
+{
+ObjectOptionsQueueEntry *entry, *next;
+
+QTAILQ_FOREACH_SAFE(entry, _queue, next, next) {
+if (early != object_create_early(entry->options)) {
+continue;
+}
+QTAILQ_REMOVE(_queue, entry, next);
+loc_push_restore(>loc);
+user_creatable_add_qapi(entry->options, _fatal);
+loc_pop(>loc);
+qapi_free_ObjectOptions(entry->options);
+g_free(entry);
+}
+}
+
+/*
+ * -readconfig still parses things into QemuOpts. Convert any such
+ * configurations to an ObjectOptionsQueueEntry.
+ *
+ * This is more restricted than the normal -object parser because QemuOpts
+ * parsed things, so no support for non-scalar properties. Help is also not
+ * supported (but this shouldn't be requested in a config file anyway).
+ */
+static int object_readconfig_to_qapi(void *opaque, QemuOpts *opts, Error
**errp)
+{
+ERRP_GUARD();
+ObjectOptionsQueueEntry *entry;
+ObjectOptions *options;
+QDict *args = qemu_opts_to_qdict(opts, NULL);
+Visitor *v;
+
+v = qobject_input_visitor_new_keyval(QOBJECT(args));
+visit_type_ObjectOptions(v, NULL, , errp);
+visit_free(v);
+qobject_unref(args);
+
+if (*errp) {
+return -1;
+}
+
+entry = g_new0(ObjectOptionsQueueEntry, 1);
+entry->options = options;
+loc_save(>loc);
+QTAILQ_INSERT_TAIL(_queue, entry, next);
+
+return 0;
+}
+
static void qemu_apply_machine_options(void)
{
MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
@@ -1816,8 +1873,8 @@ static void qemu_create_early_backends(void)
}
[PATCH v3 28/30] qom: Add user_creatable_parse_str()
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
include/qom/object_interfaces.h | 15 +++
qom/object_interfaces.c | 20 ++--
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 07511e6cff..fb32330901 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -144,6 +144,21 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * user_creatable_parse_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
+ *
+ * Parses the option for the user creatable object with a keyval parser and
+ * implicit key 'qom-type', converting the result to ObjectOptions.
+ *
+ * If a help option is given, print help instead.
+ *
+ * Returns: ObjectOptions on success, NULL when an error occurred (*errp is set
+ * then) or help was printed (*errp is not set).
+ */
+ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp);
+
/**
* user_creatable_add_from_str:
* @optarg: the object definition string as passed on the command line
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 6dcab60f09..62d7db7629 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -292,7 +292,7 @@ static void user_creatable_print_help_from_qdict(QDict
*args)
}
}
-bool user_creatable_add_from_str(const char *optarg, Error **errp)
+ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
{
ERRP_GUARD();
QDict *args;
@@ -302,12 +302,12 @@ bool user_creatable_add_from_str(const char *optarg,
Error **errp)
args = keyval_parse(optarg, "qom-type", , errp);
if (*errp) {
-return false;
+return NULL;
}
if (help) {
user_creatable_print_help_from_qdict(args);
qobject_unref(args);
-return false;
+return NULL;
}
v = qobject_input_visitor_new_keyval(QOBJECT(args));
@@ -315,12 +315,20 @@ bool user_creatable_add_from_str(const char *optarg,
Error **errp)
visit_free(v);
qobject_unref(args);
-if (*errp) {
-goto out;
+return options;
+}
+
+bool user_creatable_add_from_str(const char *optarg, Error **errp)
+{
+ERRP_GUARD();
+ObjectOptions *options;
+
+options = user_creatable_parse_str(optarg, errp);
+if (!options) {
+return false;
}
user_creatable_add_qapi(options, errp);
-out:
qapi_free_ObjectOptions(options);
return !*errp;
}
--
2.29.2
[PATCH v3 16/30] qapi/qom: Add ObjectOptions for input-*
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/common.json | 12 ++
qapi/qom.json| 59
qapi/ui.json | 13 +--
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index b87e7f9039..7c976296f0 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -185,3 +185,15 @@
##
{ 'enum': 'NetFilterDirection',
'data': [ 'all', 'rx', 'tx' ] }
+
+##
+# @GrabToggleKeys:
+#
+# Keys to toggle input-linux between host and guest.
+#
+# Since: 4.0
+#
+##
+{ 'enum': 'GrabToggleKeys',
+ 'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
+'ctrl-scrolllock' ] }
diff --git a/qapi/qom.json b/qapi/qom.json
index ad72dbdec2..6b96e9b0b3 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -444,6 +444,61 @@
'base': 'NetfilterProperties',
'data': { '*vnet_hdr_support': 'bool' } }
+##
+# @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")
+#
+# @x-origin: x coordinate of the leftmost pixel on the guest screen
+#(default: "0")
+#
+# @y-origin: y coordinate of the topmost pixel on the guest screen
+#(default: "0")
+#
+# @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' } }
+
+##
+# @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)
+#
+# @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' } }
+
##
# @IothreadProperties:
#
@@ -691,6 +746,8 @@
'filter-redirector',
'filter-replay',
'filter-rewriter',
+'input-barrier',
+'input-linux',
'iothread',
'memory-backend-file',
'memory-backend-memfd',
@@ -744,6 +801,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': { 'type': 'MemoryBackendMemfdProperties',
diff --git a/qapi/ui.json b/qapi/ui.json
index d08d72b439..cc1882108b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -6,6 +6,7 @@
# = Remote desktop
##
+{ 'include': 'common.json' }
{ 'include': 'sockets.json' }
##
@@ -1021,18 +1022,6 @@
'*head' : 'int',
'events' : [ 'InputEvent' ] } }
-##
-# @GrabToggleKeys:
-#
-# Keys to toggle input-linux between host and guest.
-#
-# Since: 4.0
-#
-##
-{ 'enum': 'GrabToggleKeys',
- 'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
-'ctrl-scrolllock' ] }
-
##
# @DisplayGTK:
#
--
2.29.2
[PATCH v3 27/30] hmp: QAPIfy object_add
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
Reviewed-by: Dr. David Alan Gilbert
---
monitor/hmp-cmds.c | 17 ++---
qom/object_interfaces.c | 11 ++-
hmp-commands.hx | 2 +-
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
+const char *options = qdict_get_str(qdict, "object");
Error *err = NULL;
-QemuOpts *opts;
-Object *obj = NULL;
-
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, );
-if (err) {
-goto end;
-}
-obj = user_creatable_add_opts(opts, );
-qemu_opts_del(opts);
-
-end:
+user_creatable_add_from_str(options, );
hmp_handle_error(mon, err);
-
-if (obj) {
-object_unref(obj);
-}
}
void hmp_getfd(Monitor *mon, const QDict *qdict)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index bf9f8cd2c6..6dcab60f09 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -14,6 +14,7 @@
#include "qemu/id.h"
#include "qemu/module.h"
#include "qemu/option.h"
+#include "qemu/qemu-print.h"
#include "qapi/opts-visitor.h"
#include "qemu/config-file.h"
@@ -221,11 +222,11 @@ static void user_creatable_print_types(void)
{
GSList *l, *list;
-printf("List of user creatable objects:\n");
+qemu_printf("List of user creatable objects:\n");
list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
for (l = list; l != NULL; l = l->next) {
ObjectClass *oc = OBJECT_CLASS(l->data);
-printf(" %s\n", object_class_get_name(oc));
+qemu_printf(" %s\n", object_class_get_name(oc));
}
g_slist_free(list);
}
@@ -256,12 +257,12 @@ static bool user_creatable_print_type_properites(const
char *type)
}
g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
if (array->len > 0) {
-printf("%s options:\n", type);
+qemu_printf("%s options:\n", type);
} else {
-printf("There are no options for %s.\n", type);
+qemu_printf("There are no options for %s.\n", type);
}
for (i = 0; i < array->len; i++) {
-printf("%s\n", (char *)array->pdata[i]);
+qemu_printf("%s\n", (char *)array->pdata[i]);
}
g_ptr_array_set_free_func(array, g_free);
g_ptr_array_free(array, true);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5d..6f5d9ce2fb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1337,7 +1337,7 @@ ERST
{
.name = "object_add",
-.args_type = "object:O",
+.args_type = "object:S",
.params = "[qom-type=]type,id=str[,prop=value][,...]",
.help = "create QOM object",
.cmd= hmp_object_add,
--
2.29.2
[PATCH v3 24/30] qemu-nbd: Use user_creatable_process_cmdline() for --object
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qemu-nbd.c | 34 +++---
1 file changed, 3 insertions(+), 31 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index b1b9430a8f..93ef4e288f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -401,24 +401,6 @@ static QemuOptsList file_opts = {
},
};
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_nbd_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
-
static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, bool list,
Error **errp)
{
@@ -594,7 +576,6 @@ int main(int argc, char **argv)
qcrypto_init(_fatal);
module_call_init(MODULE_INIT_QOM);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
qemu_init_exec_dir(argv[0]);
@@ -747,14 +728,9 @@ int main(int argc, char **argv)
case '?':
error_report("Try `%s --help' for more information.", argv[0]);
exit(EXIT_FAILURE);
-case QEMU_NBD_OPT_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-exit(EXIT_FAILURE);
-}
-} break;
+case QEMU_NBD_OPT_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case QEMU_NBD_OPT_TLSCREDS:
tlscredsid = optarg;
break;
@@ -802,10 +778,6 @@ int main(int argc, char **argv)
export_name = "";
}
-qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_nbd_object_print_help, _fatal);
-
if (!trace_init_backends()) {
exit(1);
}
--
2.29.2
[PATCH v3 25/30] qom: Add user_creatable_add_from_str()
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
include/qom/object_interfaces.h | 16
qom/object_interfaces.c | 29 -
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 1e6c51b541..07511e6cff 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -144,6 +144,22 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * 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);
+
/**
* user_creatable_process_cmdline:
* @optarg: the object definition string as passed on the command line
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 2eaf9971f5..bf9f8cd2c6 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -291,26 +291,45 @@ static void user_creatable_print_help_from_qdict(QDict
*args)
}
}
-void user_creatable_process_cmdline(const char *optarg)
+bool user_creatable_add_from_str(const char *optarg, Error **errp)
{
+ERRP_GUARD();
QDict *args;
bool help;
Visitor *v;
ObjectOptions *options;
-args = keyval_parse(optarg, "qom-type", , _fatal);
+args = keyval_parse(optarg, "qom-type", , errp);
+if (*errp) {
+return false;
+}
if (help) {
user_creatable_print_help_from_qdict(args);
-exit(EXIT_SUCCESS);
+qobject_unref(args);
+return false;
}
v = qobject_input_visitor_new_keyval(QOBJECT(args));
-visit_type_ObjectOptions(v, NULL, , _fatal);
+visit_type_ObjectOptions(v, NULL, , errp);
visit_free(v);
qobject_unref(args);
-user_creatable_add_qapi(options, _fatal);
+if (*errp) {
+goto out;
+}
+
+user_creatable_add_qapi(options, errp);
+out:
qapi_free_ObjectOptions(options);
+return !*errp;
+}
+
+void user_creatable_process_cmdline(const char *optarg)
+{
+if (!user_creatable_add_from_str(optarg, _fatal)) {
+/* Help was printed */
+exit(EXIT_SUCCESS);
+}
}
bool user_creatable_del(const char *id, Error **errp)
--
2.29.2
[PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object
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
Acked-by: Peter Krempa
---
qemu-img.c | 251 ++---
1 file changed, 45 insertions(+), 206 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index e2952fe955..babb5573ab 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -226,23 +226,6 @@ static void QEMU_NORETURN help(void)
exit(EXIT_SUCCESS);
}
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_img_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
/*
* Is @optarg safe for accumulate_options()?
* It is when multiple of them can be joined together separated by ','.
@@ -566,14 +549,9 @@ static int img_create(int argc, char **argv)
case 'u':
flags |= BDRV_O_NO_BACKING;
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-goto fail;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
}
}
@@ -589,12 +567,6 @@ static int img_create(int argc, char **argv)
}
optind++;
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-goto fail;
-}
-
/* Get image size, if specified */
if (optind < argc) {
int64_t sval;
@@ -804,14 +776,9 @@ static int img_check(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) {
-return 1;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -831,12 +798,6 @@ static int img_check(int argc, char **argv)
return 1;
}
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-return 1;
-}
-
ret = bdrv_parse_cache_mode(cache, , );
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
@@ -1034,14 +995,9 @@ static int img_commit(int argc, char **argv)
return 1;
}
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-return 1;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -1058,12 +1014,6 @@ static int img_commit(int argc, char **argv)
}
filename = argv[optind++];
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-return 1;
-}
-
flags = BDRV_O_RDWR | BDRV_O_UNMAP;
ret = bdrv_parse_cache_mode(cache, , );
if (ret < 0) {
@@ -1353,7 +1303,7 @@ static int check_empty_sectors(BlockBackend *blk, int64_t
offset,
/*
* Compares two images. Exit codes:
*
- * 0 - Images are identical
+ * 0 - Images are identical or the requested help was printed
* 1 - Images differ
* >1 - Error occurred
*/
@@ -1423,15 +1373,21 @@ 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;
+case OPTION_OBJECT:
+{
+Error *local_err = NULL;
+
+if (!user_creatable_add_from_str(optarg, _err)) {
+if (local_err) {
+error_report_err(local_err);
+exit(2);
+}
[PATCH v3 23/30] qemu-io: Use user_creatable_process_cmdline() for --object
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qemu-io.c | 33 +++--
1 file changed, 3 insertions(+), 30 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index ac88d8bd40..bf902302e9 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -477,23 +477,6 @@ enum {
OPTION_IMAGE_OPTS = 257,
};
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_io_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
static QemuOptsList file_opts = {
.name = "file",
.implied_opt_name = "file",
@@ -550,7 +533,6 @@ int main(int argc, char **argv)
qcrypto_init(_fatal);
module_call_init(MODULE_INIT_QOM);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
bdrv_init();
@@ -612,14 +594,9 @@ int main(int argc, char **argv)
case 'U':
force_share = true;
break;
-case OPTION_OBJECT: {
-QemuOpts *qopts;
-qopts = qemu_opts_parse_noisily(_object_opts,
-optarg, true);
-if (!qopts) {
-exit(1);
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
imageOpts = true;
break;
@@ -644,10 +621,6 @@ int main(int argc, char **argv)
exit(1);
}
-qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_io_object_print_help, _fatal);
-
if (!trace_init_backends()) {
exit(1);
}
--
2.29.2
[PATCH v3 08/30] qapi/qom: Add ObjectOptions for throttle-group
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
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.
+#
+# 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
+#
+# 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 6d3b8c4fe0..0721a636f9 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' }
##
@@ -449,7 +450,8 @@
'memory-backend-ram',
'rng-builtin',
'rng-egd',
-'rng-random'
+'rng-random',
+'throttle-group'
] }
##
@@ -484,7 +486,8 @@
'memory-backend-ram': 'MemoryBackendProperties',
'rng-builtin':'RngProperties',
'rng-egd':'RngEgdProperties',
- 'rng-random': 'RngRandomProperties'
+ 'rng-random': 'RngRandomProperties',
+ 'throttle-group': 'ThrottleGroupProperties'
} }
##
--
2.29.2
[PATCH v3 21/30] qom: Remove user_creatable_add_dict()
This function is now unused and can be removed.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
include/qom/object_interfaces.h | 18 --
qom/object_interfaces.c | 32
2 files changed, 50 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 9b9938b8c0..5299603f50 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -86,24 +86,6 @@ Object *user_creatable_add_type(const char *type, const char
*id,
const QDict *qdict,
Visitor *v, Error **errp);
-/**
- * user_creatable_add_dict:
- * @qdict: the object definition
- * @keyval: if true, use a keyval visitor for processing @qdict (i.e.
- * assume that all @qdict values are strings); otherwise, use
- * the normal QObject visitor (i.e. assume all @qdict values
- * have the QType expected by the QOM object type)
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object that is defined by
- * @qdict. The object type is taken from the QDict key 'qom-type', its
- * ID from the key 'id'. The remaining entries in @qdict are used to
- * initialize the object properties.
- *
- * Returns: %true on success, %false on failure.
- */
-bool user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp);
-
/**
* user_creatable_add_opts:
* @opts: the object definition
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index d4df2334b7..02c3934329 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -113,38 +113,6 @@ out:
return obj;
}
-bool user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp)
-{
-Visitor *v;
-Object *obj;
-g_autofree char *type = NULL;
-g_autofree char *id = NULL;
-
-type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
-if (!type) {
-error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-return false;
-}
-qdict_del(qdict, "qom-type");
-
-id = g_strdup(qdict_get_try_str(qdict, "id"));
-if (!id) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
-return false;
-}
-qdict_del(qdict, "id");
-
-if (keyval) {
-v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-} else {
-v = qobject_input_visitor_new(QOBJECT(qdict));
-}
-obj = user_creatable_add_type(type, id, qdict, v, errp);
-visit_free(v);
-object_unref(obj);
-return !!obj;
-}
-
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
Visitor *v;
--
2.29.2
[PATCH v3 20/30] qemu-storage-daemon: Implement --object with qmp_object_add()
This QAPIfies --object and ensures that QMP and the command line option
behave the same.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
storage-daemon/qemu-storage-daemon.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/storage-daemon/qemu-storage-daemon.c
b/storage-daemon/qemu-storage-daemon.c
index a1bcbacf05..4ab7e73053 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -38,6 +38,7 @@
#include "qapi/qapi-visit-block-core.h"
#include "qapi/qapi-visit-block-export.h"
#include "qapi/qapi-visit-control.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
#include "qapi/qobject-input-visitor.h"
@@ -134,15 +135,6 @@ enum {
extern QemuOptsList qemu_chardev_opts;
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
static void init_qmp_commands(void)
{
qmp_init_marshal(_commands);
@@ -282,14 +274,22 @@ static void process_options(int argc, char *argv[])
{
QDict *args;
bool help;
+Visitor *v;
+ObjectOptions *options;
args = keyval_parse(optarg, "qom-type", , _fatal);
if (help) {
user_creatable_print_help_from_qdict(args);
exit(EXIT_SUCCESS);
}
-user_creatable_add_dict(args, true, _fatal);
+
+v = qobject_input_visitor_new_keyval(QOBJECT(args));
+visit_type_ObjectOptions(v, NULL, , _fatal);
+visit_free(v);
qobject_unref(args);
+
+qmp_object_add(options, _fatal);
+qapi_free_ObjectOptions(options);
break;
}
case OPTION_PIDFILE:
@@ -338,7 +338,6 @@ int main(int argc, char *argv[])
module_call_init(MODULE_INIT_QOM);
module_call_init(MODULE_INIT_TRACE);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
qcrypto_init(_fatal);
bdrv_init();
--
2.29.2
[PATCH v3 19/30] qom: Make "object" QemuOptsList optional
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qom/object_interfaces.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 7661270b98..d4df2334b7 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -299,6 +299,7 @@ void user_creatable_print_help_from_qdict(QDict *args)
bool user_creatable_del(const char *id, Error **errp)
{
+QemuOptsList *opts_list;
Object *container;
Object *obj;
@@ -318,8 +319,10 @@ bool user_creatable_del(const char *id, Error **errp)
* if object was defined on the command-line, remove its corresponding
* option group entry
*/
-qemu_opts_del(qemu_opts_find(qemu_find_opts_err("object", _abort),
- id));
+opts_list = qemu_find_opts_err("object", NULL);
+if (opts_list) {
+qemu_opts_del(qemu_opts_find(opts_list, id));
+}
object_unparent(obj);
return true;
--
2.29.2
[PATCH v3 18/30] qapi/qom: QAPIfy object-add
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
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(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 0fd8563693..5b8a5da16f 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -844,13 +844,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
#
@@ -864,9 +857,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 }
##
# @object-del:
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 07d5cc8832..9b9938b8c0 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -196,11 +196,4 @@ bool user_creatable_del(const char *id, Error **errp);
*/
void user_creatable_cleanup(void);
-/**
- * qmp_object_add:
- *
- * QMP command handler for object-add. See the QAPI schema for documentation.
- */
-void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp);
-
#endif
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index a3b69e2709..ac82d54063 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -836,17 +836,17 @@ static XenBlockIOThread *xen_block_iothread_create(const
char *id,
{
ERRP_GUARD();
XenBlockIOThread *iothread = g_new(XenBlockIOThread, 1);
-QDict *opts;
-QObject *ret_data = NULL;
+ObjectOptions *opts;
iothread->id = g_strdup(id);
-opts = qdict_new();
-qdict_put_str(opts, "qom-type", TYPE_IOTHREAD);
-qdict_put_str(opts, "id", id);
-qmp_object_add(opts, _data, errp);
-qobject_unref(opts);
-qobject_unref(ret_data);
+opts = g_new(ObjectOptions, 1);
+*opts = (ObjectOptions) {
+.qom_type = OBJECT_TYPE_IOTHREAD,
+.id = g_strdup(id),
+};
+qmp_object_add(opts, errp);
+qapi_free_ObjectOptions(opts);
if (*errp) {
g_free(iothread->id);
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed747..42efd9e2ab 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -235,8 +235,6 @@ static void monitor_init_qmp_commands(void)
qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
qmp_register_command(_commands, "device_add", qmp_device_add,
QCO_NO_OPTIONS);
-qmp_register_command(_commands, "object-add", qmp_object_add,
- QCO_NO_OPTIONS);
QTAILQ_INIT(_cap_negotiation_commands);
qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 19fd5e117f..e577a96adf 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -19,8 +19,11 @@
#include "qapi/error.h"
#include "qapi/qapi-commands-qdev.h"
#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
#include "qemu/cutils.h"
#include "qom/object_interfaces.h"
#include "qom/qom-qobject.h"
@@ -223,9 +226,27 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char
*typename,
return prop_list;
}
-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);
+
+props = qobject_to(QDict, qobj);
+qdict_del(props, "qom-type");
+qdict_del(props, "id");
+
+v =
[PATCH v3 17/30] qapi/qom: Add ObjectOptions for x-remote-object
This adds a QAPI schema for the properties of the x-remote-object
object.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 6b96e9b0b3..0fd8563693 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -644,6 +644,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' } }
+
##
# @RngProperties:
#
@@ -765,7 +779,8 @@
'tls-creds-anon',
'tls-creds-psk',
'tls-creds-x509',
-'tls-cipher-suites'
+'tls-cipher-suites',
+'x-remote-object'
] }
##
@@ -820,7 +835,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'
} }
##
--
2.29.2
[PATCH v3 15/30] qapi/qom: Add ObjectOptions for confidential-guest-support
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 37 +
1 file changed, 37 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 6afac9169f..ad72dbdec2 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -635,6 +635,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)
+#
+# @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
+#
+# 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:
#
@@ -663,12 +695,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',
@@ -720,6 +755,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',
--
2.29.2
[PATCH v3 14/30] qapi/qom: Add ObjectOptions for pr-manager-helper
This adds a QAPI schema for the properties of the pr-manager-helper
object.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 14 ++
1 file changed, 14 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 6fe775bd83..6afac9169f 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -577,6 +577,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' } }
+
##
# @RngProperties:
#
@@ -651,6 +663,7 @@
'memory-backend-file',
'memory-backend-memfd',
'memory-backend-ram',
+'pr-manager-helper',
'rng-builtin',
'rng-egd',
'rng-random',
@@ -701,6 +714,7 @@
'memory-backend-memfd': { 'type': 'MemoryBackendMemfdProperties',
'if': 'defined(CONFIG_LINUX)' },
'memory-backend-ram': 'MemoryBackendProperties',
+ 'pr-manager-helper': 'PrManagerHelperProperties',
'rng-builtin':'RngProperties',
'rng-egd':'RngEgdProperties',
'rng-random': 'RngRandomProperties',
--
2.29.2
[PATCH v3 13/30] qapi/qom: Add ObjectOptions for filter-*
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/common.json | 20 +++
qapi/net.json| 20 ---
qapi/qom.json| 143 +++
3 files changed, 163 insertions(+), 20 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 2dad4fadc3..b87e7f9039 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -165,3 +165,23 @@
##
{ 'enum': 'HostMemPolicy',
'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
+
+##
+# @NetFilterDirection:
+#
+# Indicates whether a netfilter is attached to a netdev's transmit queue or
+# receive queue or both.
+#
+# @all: the filter is attached both to the receive and the transmit
+# queue of the netdev (default).
+#
+# @rx: the filter is attached to the receive queue of the netdev,
+# where it will receive packets sent to the netdev.
+#
+# @tx: the filter is attached to the transmit queue of the netdev,
+# where it will receive packets sent by the netdev.
+#
+# Since: 2.5
+##
+{ 'enum': 'NetFilterDirection',
+ 'data': [ 'all', 'rx', 'tx' ] }
diff --git a/qapi/net.json b/qapi/net.json
index c31748c87f..af3f5b0fda 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -492,26 +492,6 @@
'vhost-user': 'NetdevVhostUserOptions',
'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
-##
-# @NetFilterDirection:
-#
-# Indicates whether a netfilter is attached to a netdev's transmit queue or
-# receive queue or both.
-#
-# @all: the filter is attached both to the receive and the transmit
-# queue of the netdev (default).
-#
-# @rx: the filter is attached to the receive queue of the netdev,
-# where it will receive packets sent to the netdev.
-#
-# @tx: the filter is attached to the transmit queue of the netdev,
-# where it will receive packets sent by the netdev.
-#
-# Since: 2.5
-##
-{ 'enum': 'NetFilterDirection',
- 'data': [ 'all', 'rx', 'tx' ] }
-
##
# @RxState:
#
diff --git a/qapi/qom.json b/qapi/qom.json
index a34ae43cb9..6fe775bd83 100644
--- a/qapi/qom.json
+++ 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")
+#
+# @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")
+#
+# @insert: where to insert the filter relative to the filter given in
@position.
+# Ignored if @position is "head" or "tail". (default: behind)
+#
+# 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' } }
+
+##
+# @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' } }
+
+##
+# @FilterMirrorProperties:
+#
+# Properties for filter-mirror objects.
+#
+# @outdev: the name of a character
[PATCH v3 12/30] qapi/qom: Add ObjectOptions for colo-compare
This adds a QAPI schema for the properties of the colo-compare object.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 49 +
1 file changed, 49 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index fd87896bca..a34ae43cb9 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)
+#
+# @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' } }
+
##
# @CryptodevBackendProperties:
#
@@ -458,6 +505,7 @@
'authz-simple',
'can-bus',
'can-host-socketcan',
+'colo-compare',
'cryptodev-backend',
'cryptodev-backend-builtin',
'cryptodev-vhost-user',
@@ -499,6 +547,7 @@
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
'can-host-socketcan': 'CanHostSocketcanProperties',
+ 'colo-compare': 'ColoCompareProperties',
'cryptodev-backend': 'CryptodevBackendProperties',
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
--
2.29.2
[PATCH v3 11/30] qapi/qom: Add ObjectOptions for can-*
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.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 18 ++
1 file changed, 18 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 512d8fce12..fd87896bca 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' } }
+
##
# @CryptodevBackendProperties:
#
@@ -441,6 +456,8 @@
'authz-listfile',
'authz-pam',
'authz-simple',
+'can-bus',
+'can-host-socketcan',
'cryptodev-backend',
'cryptodev-backend-builtin',
'cryptodev-vhost-user',
@@ -481,6 +498,7 @@
'authz-listfile': 'AuthZListFileProperties',
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
+ 'can-host-socketcan': 'CanHostSocketcanProperties',
'cryptodev-backend': 'CryptodevBackendProperties',
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
--
2.29.2
[PATCH v3 10/30] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded'
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
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' } }
+
+##
+# @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'] } } }
+
+##
+# @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' } }
+
+##
+# @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 created secret object containing the
+# password for decryption.
+#
+# Features:
+# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 2.5
+##
+{ 'struct': 'TlsCredsX509Properties',
+ 'base': 'TlsCredsProperties',
+ 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
+'*sanity-check': 'bool',
+'*passwordid': 'str' } }
diff --git a/qapi/qom.json b/qapi/qom.json
index e4bbddd986..512d8fce12 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -454,7 +454,11 @@
'rng-random',
'secret',
'secret_keyring',
-'throttle-group'
+'throttle-group',
+'tls-creds-anon',
+'tls-creds-psk',
+'tls-creds-x509',
+'tls-cipher-suites'
] }
##
@@ -492,7 +496,11 @@
'rng-random': 'RngRandomProperties',
'secret': 'SecretProperties',
'secret_keyring': 'SecretKeyringProperties',
- 'throttle-group': 'ThrottleGroupProperties'
+
[PATCH v3 09/30] qapi/qom: Add ObjectOptions for secret*, deprecate 'loaded'
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
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' } }
+
+##
+# @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' } }
+
+##
+# @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' } }
diff --git a/qapi/qom.json b/qapi/qom.json
index 0721a636f9..e4bbddd986 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)
@@ -451,6 +452,8 @@
'rng-builtin',
'rng-egd',
'rng-random',
+'secret',
+'secret_keyring',
'throttle-group'
] }
@@ -487,6 +490,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 3dac79f600..f4e8226963 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -162,6 +162,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 effect (if
+``loaded`` was the last option) or cause options to be effectively ignored as
+if they were not given. The property is therefore useless and should not be
+specified.
+
+
QEMU Machine Protocol (QMP) commands
--
2.29.2
[PATCH v3 07/30] qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened'
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
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
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 8c0e06c198..6d3b8c4fe0 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -382,6 +382,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'] } } }
+
+##
+# @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' } }
+
+##
+# @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' } }
+
##
# @ObjectType:
#
@@ -400,7 +446,10 @@
'iothread',
'memory-backend-file',
'memory-backend-memfd',
-'memory-backend-ram'
+'memory-backend-ram',
+'rng-builtin',
+'rng-egd',
+'rng-random'
] }
##
@@ -432,7 +481,10 @@
'memory-backend-file':'MemoryBackendFileProperties',
'memory-backend-memfd': { 'type': 'MemoryBackendMemfdProperties',
'if': 'defined(CONFIG_LINUX)' },
- '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 893f3e8579..3dac79f600 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -153,6 +153,15 @@ The ``-writeconfig`` option is not able to serialize the
entire contents
of the QEMU command line. It is thus considered a failed experiment
and deprecated, with no current replacement.
+``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.
+
QEMU Machine Protocol (QMP) commands
--
2.29.2
[PATCH v3 06/30] qapi/qom: Add ObjectOptions for memory-backend-*
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
Acked-by: Peter Krempa
---
qapi/common.json | 20
qapi/machine.json | 22 +
qapi/qom.json | 121 +-
3 files changed, 141 insertions(+), 22 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 716712d4b3..2dad4fadc3 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -145,3 +145,23 @@
##
{ 'enum': 'PCIELinkWidth',
'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
+
+##
+# @HostMemPolicy:
+#
+# Host memory policy types
+#
+# @default: restore default policy, remove any nondefault policy
+#
+# @preferred: set the preferred host nodes for allocation
+#
+# @bind: a strict policy that restricts memory allocation to the
+#host nodes specified
+#
+# @interleave: memory allocations are interleaved across the set
+# of host nodes specified
+#
+# Since: 2.1
+##
+{ 'enum': 'HostMemPolicy',
+ 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
diff --git a/qapi/machine.json b/qapi/machine.json
index 330189efe3..4322aee782 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -8,6 +8,8 @@
# = Machines
##
+{ 'include': 'common.json' }
+
##
# @SysEmuTarget:
#
@@ -897,26 +899,6 @@
'policy': 'HmatCacheWritePolicy',
'line': 'uint16' }}
-##
-# @HostMemPolicy:
-#
-# Host memory policy types
-#
-# @default: restore default policy, remove any nondefault policy
-#
-# @preferred: set the preferred host nodes for allocation
-#
-# @bind: a strict policy that restricts memory allocation to the
-#host nodes specified
-#
-# @interleave: memory allocations are interleaved across the set
-# of host nodes specified
-#
-# Since: 2.1
-##
-{ 'enum': 'HostMemPolicy',
- 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
-
##
# @memsave:
#
diff --git a/qapi/qom.json b/qapi/qom.json
index 942654e05c..8c0e06c198 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': 'common.json' }
##
# = QEMU Object Model (QOM)
@@ -272,6 +273,115 @@
'*poll-grow': 'int',
'*poll-shrink': 'int' } }
+##
+# @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)
+#
+# @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)
+#
+# @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.
+#This option is considered stable
+#despite the x- prefix. (default:
+#false generally, but true for machine
+#types <= 4.0)
+#
+# 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)s @mem-path. Some
+# backend stores specified by @mem-path require an alignment different
+# 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)
+#
+# @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
[PATCH v3 05/30] qapi/qom: Add ObjectOptions for dbus-vmstate
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.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 18 ++
1 file changed, 18 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 46c2cdc6cf..942654e05c 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' } }
+
##
# @IothreadProperties:
#
@@ -270,6 +286,7 @@
'cryptodev-backend',
'cryptodev-backend-builtin',
'cryptodev-vhost-user',
+'dbus-vmstate',
'iothread'
] }
@@ -297,6 +314,7 @@
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
'if': 'defined(CONFIG_VIRTIO_CRYPTO) &&
defined(CONFIG_VHOST_CRYPTO)' },
+ 'dbus-vmstate': 'DBusVMStateProperties',
'iothread': 'IothreadProperties'
} }
--
2.29.2
[PATCH v3 04/30] qapi/qom: Add ObjectOptions for cryptodev-*
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
Acked-by: Peter Krempa
---
qapi/qom.json | 35 +++
1 file changed, 35 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 30ed179bc1..46c2cdc6cf 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' } }
+
+##
+# @CryptodevVhostUserProperties:
+#
+# Properties for cryptodev-vhost-user objects.
+#
+# @chardev: the name of a Unix domain socket character device that connects to
+# the vhost-user server
+#
+# Since: 2.12
+##
+{ 'struct': 'CryptodevVhostUserProperties',
+ 'base': 'CryptodevBackendProperties',
+ 'data': { 'chardev': 'str' } }
+
##
# @IothreadProperties:
#
@@ -239,6 +267,9 @@
'authz-listfile',
'authz-pam',
'authz-simple',
+'cryptodev-backend',
+'cryptodev-backend-builtin',
+'cryptodev-vhost-user',
'iothread'
] }
@@ -262,6 +293,10 @@
'authz-listfile': 'AuthZListFileProperties',
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
+ 'cryptodev-backend': 'CryptodevBackendProperties',
+ 'cryptodev-backend-builtin': 'CryptodevBackendProperties',
+ 'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
+ 'if': 'defined(CONFIG_VIRTIO_CRYPTO) &&
defined(CONFIG_VHOST_CRYPTO)' },
'iothread': 'IothreadProperties'
} }
--
2.29.2
[PATCH v3 03/30] qapi/qom: Add ObjectOptions for authz-*
This adds a QAPI schema for the properties of the authz-* objects.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/authz.json | 61 +---
qapi/qom.json| 10 +
storage-daemon/qapi/qapi-schema.json | 1 +
3 files changed, 67 insertions(+), 5 deletions(-)
diff --git a/qapi/authz.json b/qapi/authz.json
index 42afe752d1..51845e37cc 100644
--- a/qapi/authz.json
+++ b/qapi/authz.json
@@ -50,12 +50,63 @@
'*format': 'QAuthZListFormat'}}
##
-# @QAuthZListRuleListHack:
+# @AuthZListProperties:
#
-# Not exposed via QMP; hack to generate QAuthZListRuleList
-# for use internally by the code.
+# 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'] } }
+
+##
+# @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' } }
+
+##
+# @AuthZPAMProperties:
+#
+# Properties for authz-pam objects.
+#
+# @service: PAM service name to use for authorization
+#
+# Since: 4.0
+##
+{ 'struct': 'AuthZPAMProperties',
+ 'data': { 'service': 'str' } }
+
+##
+# @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': 'QAuthZListRuleListHack',
- 'data': { 'unused': ['QAuthZListRule'] } }
+{ 'struct': 'AuthZSimpleProperties',
+ 'data': { 'identity': 'str' } }
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' }
--
2.29.2
[PATCH v3 02/30] qapi/qom: Add ObjectOptions for iothread
Add an ObjectOptions union that will eventually describe the options of
all user creatable object types. As unions can't exist without any
branches, also add the first object type.
This adds a QAPI schema for the properties of the iothread object.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json | 53 +++
1 file changed, 53 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 96c91c1faf..bf2ecb34be 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -202,6 +202,59 @@
'returns': [ 'ObjectPropertyInfo' ],
'allow-preconfig': true }
+##
+# @IothreadProperties:
+#
+# Properties for iothread objects.
+#
+# @poll-max-ns: the maximum number of nanoseconds to busy wait for events.
+# 0 means polling is disabled (default: 32768 on POSIX hosts,
+# 0 otherwise)
+#
+# @poll-grow: the multiplier used to increase the polling time when the
+# algorithm detects it is missing events due to not polling long
+# enough. 0 selects a default behaviour (default: 0)
+#
+# @poll-shrink: the divisor used to decrease the polling time when the
+# algorithm detects it is spending too long polling without
+# encountering events. 0 selects a default behaviour (default: 0)
+#
+# Since: 2.0
+##
+{ 'struct': 'IothreadProperties',
+ 'data': { '*poll-max-ns': 'int',
+'*poll-grow': 'int',
+'*poll-shrink': 'int' } }
+
+##
+# @ObjectType:
+#
+# Since: 6.0
+##
+{ 'enum': 'ObjectType',
+ 'data': [
+'iothread'
+ ] }
+
+##
+# @ObjectOptions:
+#
+# Describes the options of a user creatable QOM object.
+#
+# @qom-type: the class name for the object to be created
+#
+# @id: the name of the new object
+#
+# Since: 6.0
+##
+{ 'union': 'ObjectOptions',
+ 'base': { 'qom-type': 'ObjectType',
+'id': 'str' },
+ 'discriminator': 'qom-type',
+ 'data': {
+ 'iothread': 'IothreadProperties'
+ } }
+
##
# @object-add:
#
--
2.29.2
[PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
This series adds a QAPI type for the properties of all user creatable QOM types and finally makes the --object command line option (in all binaries) and the object-add monitor commands (in QMP and HMP) use the new ObjectOptions union. This change improves things in more than just one way: 1. Documentation for QOM object types has always been lacking. Adding the schema, we get documentation for every property. 2. It prevents bugs by performing parts of the input validation (e.g. checking presence of mandatory properties) already in QAPI instead of relying on separate manual implementations in each class. 3. It provides QAPI introspection for user creatable objects. 4. Non-scalar properties are now supported everywhere because the command line parsers (including HMP) use the keyval parser now. If you are in the CC list and didn't expect this series, it's probably because you're the maintainer of one of the objects for which I'm adding a QAPI schema description. Please just have a look at the specific patch for your object and check whether the schema and its documentation make sense to you. You can ignore all other patches. In a next step after this series, we can add make use of the QAPI structs in the implementation of the object and separate their configuration from the runtime state. Specifically, the plan is to add a .configure() callback to ObjectClass that allows configuring the object in one place at creation time and keeping QOM property setters only for properties that can actually be changed at runtime. Paolo made an example of what the state could look like after this: https://wiki.qemu.org/Features/QOM-QAPI_integration Finally, the intention is to extend the QAPI schema to have separate 'object' entities and generate some of the code that was written manually in the intermediate state before. This series is available as a git tag at: https://repo.or.cz/qemu/kevin.git qapi-object-v3 v3: - Removed now useless QAuthZListRuleListHack - Made some more ObjectOptions branches conditional - Improved documentation for some properties - Fixed 'qemu-img compare' exit code for option parsing failure v2: - Convert not only object-add, but all external interfaces so that the schema will always be enforced and mismatch between implementation and schema can't go unnoticed. - Rebased, covering properties and object types added since v1 (yes, things do become outdated rather quickly when you touch all user creatable objects) - Changed the "Since:" version number in the schema documentation to refer to the version when the object was introduced rather than 6.0 where the schema will (hopefully) be added - Probably some other minor changes Kevin Wolf (30): qapi/qom: Drop deprecated 'props' from object-add qapi/qom: Add ObjectOptions for iothread qapi/qom: Add ObjectOptions for authz-* qapi/qom: Add ObjectOptions for cryptodev-* qapi/qom: Add ObjectOptions for dbus-vmstate qapi/qom: Add ObjectOptions for memory-backend-* qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened' qapi/qom: Add ObjectOptions for throttle-group qapi/qom: Add ObjectOptions for secret*, deprecate 'loaded' qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded' qapi/qom: Add ObjectOptions for can-* qapi/qom: Add ObjectOptions for colo-compare qapi/qom: Add ObjectOptions for filter-* qapi/qom: Add ObjectOptions for pr-manager-helper qapi/qom: Add ObjectOptions for confidential-guest-support qapi/qom: Add ObjectOptions for input-* qapi/qom: Add ObjectOptions for x-remote-object qapi/qom: QAPIfy object-add qom: Make "object" QemuOptsList optional qemu-storage-daemon: Implement --object with qmp_object_add() qom: Remove user_creatable_add_dict() qom: Factor out user_creatable_process_cmdline() qemu-io: Use user_creatable_process_cmdline() for --object qemu-nbd: Use user_creatable_process_cmdline() for --object qom: Add user_creatable_add_from_str() qemu-img: Use user_creatable_process_cmdline() for --object hmp: QAPIfy object_add qom: Add user_creatable_parse_str() vl: QAPIfy -object qom: Drop QemuOpts based interfaces qapi/authz.json | 61 ++- qapi/block-core.json | 27 ++ qapi/common.json | 52 +++ qapi/crypto.json | 159 +++ qapi/machine.json| 22 +- qapi/net.json| 20 - qapi/qom.json| 644 ++- qapi/ui.json | 13 +- docs/system/deprecated.rst | 25 +- docs/system/removed-features.rst | 5 + include/qom/object_interfaces.h | 106 ++--- hw/block/xen-block.c | 16 +- monitor/hmp-cmds.c | 17 +- monitor/misc.c | 2 - qemu-img.c | 251 ++- qemu-io.c| 33 +- qemu-nbd.c
[PATCH v3 01/30] qapi/qom: Drop deprecated 'props' from object-add
The option has been deprecated in QEMU 5.0, remove it.
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
---
qapi/qom.json| 6 +-
docs/system/deprecated.rst | 5 -
docs/system/removed-features.rst | 5 +
qom/qom-qmp-cmds.c | 21 -
4 files changed, 6 insertions(+), 31 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 0b0b92944b..96c91c1faf 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -211,10 +211,6 @@
#
# @id: the name of the new object
#
-# @props: a dictionary of properties to be passed to the backend. Deprecated
-# since 5.0, specify the properties on the top level instead. It is an
-# error to specify the same option both on the top level and in @props.
-#
# Additional arguments depend on qom-type and are passed to the backend
# unchanged.
#
@@ -232,7 +228,7 @@
#
##
{ 'command': 'object-add',
- 'data': {'qom-type': 'str', 'id': 'str', '*props': 'any'},
+ 'data': {'qom-type': 'str', 'id': 'str'},
'gen': false } # so we can get the additional arguments
##
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 561c916da2..893f3e8579 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -206,11 +206,6 @@ Use ``migrate-set-parameters`` and
``query-migrate-parameters`` instead.
Use arguments ``base-node`` and ``top-node`` instead.
-``object-add`` option ``props`` (since 5.0)
-'''
-
-Specify the properties for the object as top-level arguments instead.
-
``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].status
(since 4.0)
''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index c8481cafbd..95f3fb2912 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -58,6 +58,11 @@ documentation of ``query-hotpluggable-cpus`` for additional
details.
Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.
+``object-add`` option ``props`` (removed in 6.0)
+
+
+Specify the properties for the object as top-level arguments instead.
+
Human Monitor Protocol (HMP) commands
-
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index b40ac39f30..19fd5e117f 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -225,27 +225,6 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char
*typename,
void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
{
-QObject *props;
-QDict *pdict;
-
-props = qdict_get(qdict, "props");
-if (props) {
-pdict = qobject_to(QDict, props);
-if (!pdict) {
-error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
-return;
-}
-qobject_ref(pdict);
-qdict_del(qdict, "props");
-qdict_join(qdict, pdict, false);
-if (qdict_size(pdict) != 0) {
-error_setg(errp, "Option in 'props' conflicts with top level");
-qobject_unref(pdict);
-return;
-}
-qobject_unref(pdict);
-}
-
user_creatable_add_dict(qdict, false, errp);
}
--
2.29.2
Re: [PATCH 00/14] deprecations: remove many old deprecations
On Wed, Feb 24, 2021 at 04:21:13PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/24/21 3:38 PM, Peter Maydell wrote:
> > On Wed, 24 Feb 2021 at 13:21, Daniel P. Berrangé
> > wrote:
> >>
> >> The following features have been deprecated for well over the 2
> >> release cycle we promise
> >>
> >> ``-usbdevice`` (since 2.10.0)
> >> ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
> >> ``-vnc acl`` (since 4.0.0)
> >> ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
> >
> > Are the literal '=3D' here intended ?
>
> No, this is a git-publish bug:
> https://github.com/stefanha/git-publish/issues/88
>
> Apparently the fix is not yet backported to Fedora.
Thanks for reminding me. I'll roll a new git-publish release and package
it in Fedora.
Stefan
signature.asc
Description: PGP signature
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Mon, Mar 08, 2021 at 04:32:26PM +0100, Andrea Bolognani wrote: > On Mon, 2021-03-08 at 13:17 +, Daniel P. Berrangé wrote: > > On Mon, Mar 08, 2021 at 02:11:56PM +0100, Andrea Bolognani wrote: > > > The reason why VFIO device assignment is currently not completely > > > broken in KubeVirt is that, when the QEMU process is initially > > > started, we set the memory locking limit after fork(), so we can do > > > that using setrlimit() which doesn't require additional capabilities, > > > but in the hotplug scenario libvirtd needs to change the limits of a > > > different process: in that case we are forced to use prlimit(), which > > > fails due to the lack of CAP_SYS_RESOURCE. > > > > Since you added code to parse existing limits from /proc, I'm wondering > > if we can just do without the config option. Simply try to use prlimit > > and if it fails, query existing limits to determine if we sould treat > > the prlimit as fatal or ignore it. Overall I'd prefer libvirt to > > "just work" out of the box rather than requiring people to know about > > setting a "make-vfio-hotplug-work=yes" flag in the config file. > > The problem with that approach is what to do when *lowering* the > limit, for example as a consequence of hot-unplugging the last VFIO > device from the VM. > > If we're controlling the memory locking limit ourselves, then failure > to lower it should be an error, because leaving the limit much higher > than necessary creates potential for DoS by a compromised QEMU; on > the other hand, if the limit is controlled by an external process, > all we can really do is assume they will do the right thing after > hot-unplugging has happened. IMHO once QEMU vCPUs start running, immediately assume QEMU is compromised / hostile. IOW, the DoS risk arrived the moment it was given the higher limit. We're just failing to close off the existing risk we've already accepted, which doesn't worry me much. On unplug the only thing we actually do when memory lock reduce fails is to log a warning message, it is never treated as a fatal error. So the only difference is whether we skip the warning message when we get EPERM from prlimit(), or always emit the warning. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Mon, 2021-03-08 at 13:17 +, Daniel P. Berrangé wrote: > On Mon, Mar 08, 2021 at 02:11:56PM +0100, Andrea Bolognani wrote: > > The reason why VFIO device assignment is currently not completely > > broken in KubeVirt is that, when the QEMU process is initially > > started, we set the memory locking limit after fork(), so we can do > > that using setrlimit() which doesn't require additional capabilities, > > but in the hotplug scenario libvirtd needs to change the limits of a > > different process: in that case we are forced to use prlimit(), which > > fails due to the lack of CAP_SYS_RESOURCE. > > Since you added code to parse existing limits from /proc, I'm wondering > if we can just do without the config option. Simply try to use prlimit > and if it fails, query existing limits to determine if we sould treat > the prlimit as fatal or ignore it. Overall I'd prefer libvirt to > "just work" out of the box rather than requiring people to know about > setting a "make-vfio-hotplug-work=yes" flag in the config file. The problem with that approach is what to do when *lowering* the limit, for example as a consequence of hot-unplugging the last VFIO device from the VM. If we're controlling the memory locking limit ourselves, then failure to lower it should be an error, because leaving the limit much higher than necessary creates potential for DoS by a compromised QEMU; on the other hand, if the limit is controlled by an external process, all we can really do is assume they will do the right thing after hot-unplugging has happened. I don't think discoverability is too much of an issue, as anyone who needs to use this option will already have needed to figure out a lot more in order to effectively take over memory locking limit management responsibilities from libvirt... -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 12/17] util: Try to get limits from /proc
On Mon, Mar 08, 2021 at 04:21:16PM +0100, Andrea Bolognani wrote:
> On Mon, 2021-03-08 at 10:50 +, Daniel P. Berrangé wrote:
> > On Fri, Mar 05, 2021 at 08:13:59PM +0100, Andrea Bolognani wrote:
> > > +if (!(label = virProcessLimitResourceToLabel(resource))) {
> > > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > > + _("Unknown resource %d requested for process
> > > %lld"),
> > > + resource, (long long)pid);
> > > +return -1;
> >
> > Setting errors on -1
>
> This is only hit if virProcessGetLimitFromProc() has been asked to
> obtain limits for a resource it doesn't know how to fetch, which
> indicates a bug in libvirt and is thus reported as internal error.
The our coding standard is that we only call virReportError if the
caller is actually going to honour the errors.
>
> > > +procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
> > > +
> > > +if (!g_file_get_contents(procfile, , , NULL))
> > > +return -1;
> >
> > Not setting errors on -1
>
> This is simply "file couldn't be read", which would be the case on
> FreeBSD for example.
Right, but we *must* always be consistent in whether a method uses
virReportError or not. We have two code paths here returning -1,
and only one of which reports errors. Either all must report
errors, or none. Since the only caller is ignoring errors, then
it looks like none should report errors.
>
> > > +/* For whatever reason, using prlimit() on another process - even
> > > + * when it's just to obtain the current limit rather than changing
> > > + * it - requires CAP_SYS_RESOURCE, which we might not have in a
> > > + * containerized environment; on the other hand, no particular
> > > + * permission is needed to poke around /proc, so try that if going
> > > + * through the syscall didn't work */
> > > +if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
> > > +return 0;
> >
> > This ought to be conditional for Linux only and error reporting needs
> > to be made consistent.
>
> The intent above was to have this fail quietly on non-Linux without
> adding checks for it, but sure I can have an actual stub on other
> platforms instead.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 12/17] util: Try to get limits from /proc
On Mon, 2021-03-08 at 10:50 +, Daniel P. Berrangé wrote:
> On Fri, Mar 05, 2021 at 08:13:59PM +0100, Andrea Bolognani wrote:
> > +if (!(label = virProcessLimitResourceToLabel(resource))) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unknown resource %d requested for process %lld"),
> > + resource, (long long)pid);
> > +return -1;
>
> Setting errors on -1
This is only hit if virProcessGetLimitFromProc() has been asked to
obtain limits for a resource it doesn't know how to fetch, which
indicates a bug in libvirt and is thus reported as internal error.
> > +procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
> > +
> > +if (!g_file_get_contents(procfile, , , NULL))
> > +return -1;
>
> Not setting errors on -1
This is simply "file couldn't be read", which would be the case on
FreeBSD for example.
> > +/* For whatever reason, using prlimit() on another process - even
> > + * when it's just to obtain the current limit rather than changing
> > + * it - requires CAP_SYS_RESOURCE, which we might not have in a
> > + * containerized environment; on the other hand, no particular
> > + * permission is needed to poke around /proc, so try that if going
> > + * through the syscall didn't work */
> > +if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
> > +return 0;
>
> This ought to be conditional for Linux only and error reporting needs
> to be made consistent.
The intent above was to have this fail quietly on non-Linux without
adding checks for it, but sure I can have an actual stub on other
platforms instead.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob
On 3/8/21 11:52 AM, Daniel P. Berrangé wrote: On Fri, Mar 05, 2021 at 08:14:02PM +0100, Andrea Bolognani wrote: This will be useful when libvirtd is running in a containerized environment with limited capabilities, and in order to make things like VFIO device assignment still work an external privileged process changes the limits from outside of the container. KubeVirt is an example of this setup. Signed-off-by: Andrea Bolognani --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 12 src/qemu/qemu_conf.c | 4 src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 5 files changed, 19 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 3c1045858b..f1b024a37f 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -104,6 +104,7 @@ module Libvirtd_qemu = | str_entry "slirp_helper" | str_entry "dbus_daemon" | bool_entry "set_process_name" + | bool_entry "external_limit_manager" | int_entry "max_processes" | int_entry "max_files" | limits_entry "max_core" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 0c1054f198..15cbc3ba38 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -662,6 +662,18 @@ # #set_process_name = 1 +# If enabled, libvirt will not attempt to change process limits (as +# configured with the max_processes, max_files and max_core settings +# below) itself but will instead expect an external entity to perform +# this task. Can't users simply not set max_core, max_files, etc already ? These two yes, mem lock no. Michal
Re: [PATCH v2 00/12] qemu: Prepare for QAPIfied object-add
On Wed, Feb 24, 2021 at 16:57:54 +0100, Peter Krempa wrote: > QEMU plans to QAPIfy object add. This series prepares for the API change > (drop of 'props' wrapper for the object) and adds testing based on our > qemuxml2argv test data which forces the output to JSON and validates it > agains the schema. > > Based on Kevin's qemu patches: > https://listman.redhat.com/archives/libvir-list/2021-February/msg01168.html Ping? Kevin already submitted a pull-request with the patches: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg01773.html Note that the qemu series removes deprecated 'props' wrapper object of object-add, and this series adapts to that!
Re: [libvirt PATCH 12/17] util: Try to get limits from /proc
On Mon, 2021-03-08 at 11:30 +0100, Michal Privoznik wrote: > On 3/5/21 8:13 PM, Andrea Bolognani wrote: > > +if (!STRPREFIX(line, label)) > > +continue; > > + > > +line += strlen(label); > > Or if (!(line = STRSKIP(line, label)) continue; Oh, I didn't know that existed! Nice suggestion :) -- Andrea Bolognani / Red Hat / Virtualization
[PATCH v1] tests: Adjust libxlxml2domconfigtest to work with Xen < 4.8
Commit fcdc387410fadfb066b95395c5b5d2a6a16f7066 used a libxl API which
is only available since Xen 4.8.
Due to lack of a specific guard for this API change, reuse another
guard from libxl.h.
Signed-off-by: Olaf Hering
---
tests/libxlxml2domconfigtest.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index d58be1211b..c13a562a3c 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -105,7 +105,13 @@ testCompareXMLToDomConfig(const char *xmlfile,
*/
# ifndef LIBXL_HAVE_BUILDINFO_APIC
if (expectconfig.c_info.type == LIBXL_DOMAIN_TYPE_HVM) {
+# ifdef LIBXL_HAVE_MEMKB_64BITS
+/*
+ * This part of the libxl API was changed without a guard in Xen 4.8.
+ * Reuse another Xen 4.8 specific conditional.
+ */
libxl_defbool_unset(_info.acpi);
+# endif
libxl_defbool_set(_info.u.hvm.apic, true);
libxl_defbool_set(_info.u.hvm.acpi, true);
}
Re: [libvirt PATCH 11/17] tests: Mock virProcessGetMaxMemLock()
On Mon, 2021-03-08 at 11:31 +0100, Michal Privoznik wrote:
> On 3/5/21 8:13 PM, Andrea Bolognani wrote:
> > +int
> > +virProcessGetMaxMemLock(pid_t pid G_GNUC_UNUSED, unsigned long long *bytes
> > G_GNUC_UNUSED)
>
> Ehm, probably coffee hadn't kicked in? Because I can see @bytes used ..
>
> > +{
> > +*bytes = 0;
>
> .. right here :-D
More like it had been too long since the last coffee, but yeah, of
course you're absolutely right :)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 02/17] util: Simplify stubs
On Mon, 2021-03-08 at 11:31 +0100, Michal Privoznik wrote: > On 3/5/21 8:13 PM, Andrea Bolognani wrote: > > Calling a stub should always result in ENOSYS being raised, > > regardless of what arguments are passed to it. > > > > Signed-off-by: Andrea Bolognani > > --- > > src/util/virprocess.c | 22 ++ > > 1 file changed, 6 insertions(+), 16 deletions(-) > > Missed virProcessSetMaxMemLock(). Good catch - I must have misplaced it during a rebase O:-) -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Mon, Mar 08, 2021 at 02:11:56PM +0100, Andrea Bolognani wrote: > On Mon, 2021-03-08 at 10:54 +, Daniel P. Berrangé wrote: > > On Fri, Mar 05, 2021 at 08:13:47PM +0100, Andrea Bolognani wrote: > > > This feature has been requested by KubeVirt developers and will make > > > it possible for them to make some VFIO-related features, such as > > > migration and hotplug, work correctly. > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1916346 > > > > > > The first part of the series, especially the first 9 patches, is > > > preparation work: it addresses a few annoying issues with our APIs > > > that deal with process limits, and makes them all nice, consistent > > > and easy to reason about while moving policy code from the generic > > > code to the QEMU driver where it belongs. > > > > IIUC, the code was already supposed to attempt to set the limits > > and gracefully carry on if it was unable todo so. Can you outline > > the actual problem being solved, as wading through the bug comments > > to understand the precise detail of the problem is not very clear. > > Not being able to set the memory locking limit shouldn't be a soft > failure, because that might cause QEMU to start up apparently fine > and then error out later on during the lifetime of the VM, which is > much worse than not starting up at all. We're actually doing this > correctly for the most part, which is why hotplug requests for VFIO > devices in KubeVirt result in errors under certain conditions. Yep, I agree we shoudln't have it be a mere warning. IIUC, we would try to set the limit, and if we fail, then we query the current limit to see if its satisfactory and report a fatal error if not. > The reason why VFIO device assignment is currently not completely > broken in KubeVirt is that, when the QEMU process is initially > started, we set the memory locking limit after fork(), so we can do > that using setrlimit() which doesn't require additional capabilities, > but in the hotplug scenario libvirtd needs to change the limits of a > different process: in that case we are forced to use prlimit(), which > fails due to the lack of CAP_SYS_RESOURCE. Since you added code to parse existing limits from /proc, I'm wondering if we can just do without the config option. Simply try to use prlimit and if it fails, query existing limits to determine if we sould treat the prlimit as fatal or ignore it. Overall I'd prefer libvirt to "just work" out of the box rather than requiring people to know about setting a "make-vfio-hotplug-work=yes" flag in the config file. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Mon, 2021-03-08 at 10:54 +, Daniel P. Berrangé wrote: > On Fri, Mar 05, 2021 at 08:13:47PM +0100, Andrea Bolognani wrote: > > This feature has been requested by KubeVirt developers and will make > > it possible for them to make some VFIO-related features, such as > > migration and hotplug, work correctly. > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1916346 > > > > The first part of the series, especially the first 9 patches, is > > preparation work: it addresses a few annoying issues with our APIs > > that deal with process limits, and makes them all nice, consistent > > and easy to reason about while moving policy code from the generic > > code to the QEMU driver where it belongs. > > IIUC, the code was already supposed to attempt to set the limits > and gracefully carry on if it was unable todo so. Can you outline > the actual problem being solved, as wading through the bug comments > to understand the precise detail of the problem is not very clear. Not being able to set the memory locking limit shouldn't be a soft failure, because that might cause QEMU to start up apparently fine and then error out later on during the lifetime of the VM, which is much worse than not starting up at all. We're actually doing this correctly for the most part, which is why hotplug requests for VFIO devices in KubeVirt result in errors under certain conditions. The reason why VFIO device assignment is currently not completely broken in KubeVirt is that, when the QEMU process is initially started, we set the memory locking limit after fork(), so we can do that using setrlimit() which doesn't require additional capabilities, but in the hotplug scenario libvirtd needs to change the limits of a different process: in that case we are forced to use prlimit(), which fails due to the lack of CAP_SYS_RESOURCE. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] docs: stop mentioning insecure / broken SASL mechanisms
... > > +**Note:** the SASL ``passwd.db`` file stores passwords in clear text, so > +care should be taken not to let its contents be disclosed to unauthorized > +users. Can we make ^hits all bold to make it more visible? Reviewed-by: Erik Skultety
Re: [libvirt PATCH 1/2] docs: convert auth page into RST format
On Thu, Mar 04, 2021 at 06:10:12PM +, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > docs/auth.html.in | 368 -- > docs/auth.rst | 350 +++ > docs/meson.build | 2 +- > 3 files changed, 351 insertions(+), 369 deletions(-) > delete mode 100644 docs/auth.html.in > create mode 100644 docs/auth.rst > > diff --git a/docs/auth.html.in b/docs/auth.html.in > deleted file mode 100644 > index 9b940a8598..00 > --- a/docs/auth.html.in > +++ /dev/null > @@ -1,368 +0,0 @@ > - > - > -http://www.w3.org/1999/xhtml;> > - > -Connection authentication > - > - When connecting to libvirt, some connections may require client > - authentication before allowing use of the APIs. The set of possible > - authentication mechanisms is administrator controlled, independent > - of applications using libvirt. Once authenticated, libvirt can apply > - fine grained access control to the operations > - performed by a client. > - > - > - > - > -Client configuration > - > - > - When connecting to a remote hypervisor which requires authentication, > -most libvirt applications will prompt the user for the credentials. It is > -also possible to provide a client configuration file containing all the > -authentication credentials, avoiding any interaction. Libvirt will look > -for the authentication file using the following sequence: > - > - > - The file path specified by the $LIBVIRT_AUTH_FILE environment > -variable. > - The file path specified by the "authfile=/some/file" URI > -query parameter > - The file $XDG_CONFIG_HOME/libvirt/auth.conf > - The file /etc/libvirt/auth.conf > - > - > - > - The auth configuration file uses the traditional ".ini" > - style syntax. There are two types of groups that can be present in > - the config. First there are one or more credential > - sets, which provide the actual authentication credentials. The keys > - within the group may be: > - > - > - > - username: the user login name to act as. This > -is relevant for ESX, Xen, HyperV and SSH, but probably not > -the one you want to libvirtd with SASL. > - authname: the name to authorize as. This is > -what is commonly required for libvirtd with SASL. > - password: the secret password > - realm: the domain realm for SASL, mostly > -unused > - > - > - > - Each set of credentials has a name, which is part of the group > - entry name. Overall the syntax is > - > - > - > -[credentials-$NAME] > -credname1=value1 > -credname2=value2 > - > - > - For example, to define two sets of credentials used for production > - and test machines, using libvirtd, and a further ESX server for dev: > - > - > -[credentials-test] > -authname=fred > -password=123456 > - > -[credentials-prod] > -authname=bar > -password=letmein > - > -[credentials-dev] > -username=joe > -password=hello > - > -[credentials-defgrp] > -username=defuser > -password=defpw > - > - > - The second set of groups provide mappings of credentials to > - specific machine services. The config file group names compromise > - the service type and host: > - > - > - > -[auth-$SERVICE-$HOSTNAME] > -credentials=$CREDENTIALS > - > - > - For example, following the previous example, here is how to > - map some machines. For convenience libvirt supports a default > - mapping of credentials to machines: > - > - > - > -[auth-libvirt-test1.example.com] > -credentials=test > - > -[auth-libvirt-test2.example.com] > -credentials=test > - > -[auth-libvirt-demo3.example.com] > -credentials=test > - > -[auth-libvirt-prod1.example.com] > -credentials=prod > - > -[auth-libvirt-default] > -credentials=defgrp > - > -[auth-esx-dev1.example.com] > -credentials=dev > - > -[auth-esx-default] > -credentials=defgrp > - > - > - > - The following service types are known to libvirt: > - > - > - > - esx - used for connections to an ESX or > -VirtualCenter server > - hyperv - used for connections to an HyperV > -server > - libvirt - used for connections to a libvirtd > -server, which is configured with SASL auth > - ssh - used for connections to a remote QEMU driver > -over SSH > - > - > - > - Applications using libvirt are free to use this same configuration > - file for storing other credentials. For example, it can be used > - to storage VNC or SPICE login credentials > - > - > -Server configuration > - > -The libvirt daemon allows the administrator to choose the authentication > -mechanisms used for client connections on each network socket independently. > -This is primarily controlled via the libvirt daemon master config file
Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob
On Mon, Mar 08, 2021 at 01:56:15PM +0100, Andrea Bolognani wrote: > On Mon, 2021-03-08 at 10:52 +, Daniel P. Berrangé wrote: > > On Fri, Mar 05, 2021 at 08:14:02PM +0100, Andrea Bolognani wrote: > > > +# If enabled, libvirt will not attempt to change process limits (as > > > +# configured with the max_processes, max_files and max_core settings > > > +# below) itself but will instead expect an external entity to perform > > > +# this task. > > > > Can't users simply not set max_core, max_files, etc already ? > > That works for things that are static and have a corresponding > configuration option in qemu.conf, but the memory locking limit is > dynamic, per-VM and needs to change as devices are added and removed > from the guest. > > > I think it is preferrable to have flags tailored specifically to > > the individual limits, not a global flag. Otherwise you can end > > up in a case where you want to disable the memory limits, but > > keep the other limits set which is impossible with this global > > flag. > > Since what I'm interested in is the memory locking limit, I guess I > could turn this into > > max_memlock_external = 1 > > or even > > max_memlock = "external" > > with "dynamic" being the other accepted value, which would be the > default and would behave as libvirt does today. > > Do you think that would work better? I think that would be better, as it has clearly defined scope which we can maintain more accurately long term. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob
On Mon, 2021-03-08 at 10:52 +, Daniel P. Berrangé wrote: > On Fri, Mar 05, 2021 at 08:14:02PM +0100, Andrea Bolognani wrote: > > +# If enabled, libvirt will not attempt to change process limits (as > > +# configured with the max_processes, max_files and max_core settings > > +# below) itself but will instead expect an external entity to perform > > +# this task. > > Can't users simply not set max_core, max_files, etc already ? That works for things that are static and have a corresponding configuration option in qemu.conf, but the memory locking limit is dynamic, per-VM and needs to change as devices are added and removed from the guest. > I think it is preferrable to have flags tailored specifically to > the individual limits, not a global flag. Otherwise you can end > up in a case where you want to disable the memory limits, but > keep the other limits set which is impossible with this global > flag. Since what I'm interested in is the memory locking limit, I guess I could turn this into max_memlock_external = 1 or even max_memlock = "external" with "dynamic" being the other accepted value, which would be the default and would behave as libvirt does today. Do you think that would work better? -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration
On Fri, Mar 05, 2021 at 09:06:45AM +0100, Markus Armbruster wrote: > Markus Armbruster writes: > > > Daniel P. Berrangé writes: > > > >> On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote: > >>> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate > >>> configuring floppies with -global isa-fdc" (v5.1.0). > >>> > >>> Signed-off-by: Markus Armbruster > [...] > > Sadly, the commit's update of docs/system/deprecated.rst neglects to > > cover this use. Looks the series overtaxed my capacity to juggle > > details; my apologies. > [...] > > I'm talking about commit 4a27a638e7 here. > > The deprecated.rst text only covers setting floppy controller properties > with -global. It neglects to cover setting them with -device. For > onboard controllers, -global is the only way to set them. > > I append a fixup. > > We can put it before this patch. This patch then moves the fixed up > text to removed-features.rst. > > Or we squash it into this patch, i.e. this patch deletes the flawed text > from deprecated.rst and adds the fixed up text to removed-features.rst. > > Got a preference? I'm fine with either option. It isn't unusual to tweak the text when moving it to the removed-features.rst file, as we'll be talking about the past rather than future. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 0/2] docs: less docs for insecure SASL mechanisms
On Mon, Mar 08, 2021 at 12:00:03PM +0100, Erik Skultety wrote: > On Mon, Mar 08, 2021 at 10:41:55AM +, Daniel P. Berrangé wrote: > > On Fri, Mar 05, 2021 at 08:02:49AM +0100, Erik Skultety wrote: > > > On Thu, Mar 04, 2021 at 06:10:11PM +, Daniel P. Berrangé wrote: > > > > GSSAPI and SCRAM-SHA-256 are the only two SASL mechanisms we > > > > especially want people to be using. Even the latter is a little > > > > questionable due to storing passwords in cleartext on the server. > > > > > > At what point of the SCRAM-SHA-256 auth process is password handled as > > > clear > > > text? I mean I tried to look up the issue you mention and couldn't find > > > anything, quite the contrary, e.g. Postgres says SCRAM-SHA-256 is the only > > > recommended scheme for password-based auth and storing passwords in clear > > > text > > > is not possible. Isn't it kind of the point that passwords are never > > > stored in > > > clear text with this scheme? > > > > You can clearly see the passwd in clear text in the file > > > > Add a new user > > > > $ echo "fish food" | saslpasswd2 -a libvirt demo > > > > Look for the password: > > > > $ strings /etc/libvirt/passwd.db | grep fish > > fish food > > > > The actual mechanism protocol does send in clear text over the wire. > > The storage in clear text on the server side is simply a choice of the > > cyrus-sasl impl of this mechanism documented here: > > > > https://www.cyrusimap.org/sasl/sasl/faqs/plaintextpasswords.html > > So if this is the case, why are we even bothering promoting an insecure > solution, why not promote only GSSAPI for the reasons given? Backcompat? Calling it insecure is a rather absolutist view of security IMHO. It *is* secure against network attackers, which is the primary risk factor we're protecting against with our use of SASL. To compromise the passwords requires the attacker to first compromise the root account on the virt host. Anything is possible, but IMHO that danger doesn't justify eliminating the SCRAM auth mechanism. If we only promote GSSAPI, then the result is likely to be that people run with no authentication at all, or make their own ill informed worse choices. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 0/2] docs: less docs for insecure SASL mechanisms
On Mon, Mar 08, 2021 at 10:41:55AM +, Daniel P. Berrangé wrote: > On Fri, Mar 05, 2021 at 08:02:49AM +0100, Erik Skultety wrote: > > On Thu, Mar 04, 2021 at 06:10:11PM +, Daniel P. Berrangé wrote: > > > GSSAPI and SCRAM-SHA-256 are the only two SASL mechanisms we > > > especially want people to be using. Even the latter is a little > > > questionable due to storing passwords in cleartext on the server. > > > > At what point of the SCRAM-SHA-256 auth process is password handled as clear > > text? I mean I tried to look up the issue you mention and couldn't find > > anything, quite the contrary, e.g. Postgres says SCRAM-SHA-256 is the only > > recommended scheme for password-based auth and storing passwords in clear > > text > > is not possible. Isn't it kind of the point that passwords are never stored > > in > > clear text with this scheme? > > You can clearly see the passwd in clear text in the file > > Add a new user > > $ echo "fish food" | saslpasswd2 -a libvirt demo > > Look for the password: > > $ strings /etc/libvirt/passwd.db | grep fish > fish food > > The actual mechanism protocol does send in clear text over the wire. > The storage in clear text on the server side is simply a choice of the > cyrus-sasl impl of this mechanism documented here: > > https://www.cyrusimap.org/sasl/sasl/faqs/plaintextpasswords.html So if this is the case, why are we even bothering promoting an insecure solution, why not promote only GSSAPI for the reasons given? Backcompat? Erik
Re: [PATCH] virDevMapperGetTargetsImpl: Use correct length when copying into dm.name
On Mon, Mar 08, 2021 at 09:14:18AM +0100, Michal Privoznik wrote:
> For reasons unknown, when rewriting this code and dropping
> libdevmapper I've mistakenly used incorrect length of dm.name. In
> linux/dm-ioctl.h the dm_ioctl struct is defined as follows:
>
> #define DM_NAME_LEN 128
>
> struct dm_ioctl {
> ...
> char name[DM_NAME_LEN]; /* device name */
> ...
> };
>
> However, when copying string into this member, DM_TABLE_DEPS was
> used, which is defined as follows:
>
> #define DM_TABLE_DEPS_IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)
>
> After decryption, this results in the following size: 3241737483.
>
> Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e
> Signed-off-by: Michal Privoznik
> ---
> src/util/virdevmapper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
So we were not correctly capping the input path length. Bad
but IIUC not a security bug because the input is controlled by
a client who already has privileges equivalent to root.
>
> diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
> index fcb11e954f..2c4c2df999 100644
> --- a/src/util/virdevmapper.c
> +++ b/src/util/virdevmapper.c
> @@ -240,7 +240,7 @@ virDevMapperGetTargetsImpl(int controlFD,
> if (!(sanitizedPath = virDMSanitizepath(path)))
> return 0;
>
> -if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) < 0) {
> +if (virStrcpy(dm.name, sanitizedPath, DM_NAME_LEN) < 0) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> _("Resolved device mapper name too long"));
> return -1;
Reviewed-by: Daniel P. Berrangé
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On Fri, Mar 05, 2021 at 08:13:47PM +0100, Andrea Bolognani wrote: > This feature has been requested by KubeVirt developers and will make > it possible for them to make some VFIO-related features, such as > migration and hotplug, work correctly. > > https://bugzilla.redhat.com/show_bug.cgi?id=1916346 > > The first part of the series, especially the first 9 patches, is > preparation work: it addresses a few annoying issues with our APIs > that deal with process limits, and makes them all nice, consistent > and easy to reason about while moving policy code from the generic > code to the QEMU driver where it belongs. IIUC, the code was already supposed to attempt to set the limits and gracefully carry on if it was unable todo so. Can you outline the actual problem being solved, as wading through the bug comments to understand the precise detail of the problem is not very clear. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob
On Fri, Mar 05, 2021 at 08:14:02PM +0100, Andrea Bolognani wrote:
> This will be useful when libvirtd is running in a containerized
> environment with limited capabilities, and in order to make
> things like VFIO device assignment still work an external
> privileged process changes the limits from outside of the
> container. KubeVirt is an example of this setup.
>
> Signed-off-by: Andrea Bolognani
> ---
> src/qemu/libvirtd_qemu.aug | 1 +
> src/qemu/qemu.conf | 12
> src/qemu/qemu_conf.c | 4
> src/qemu/qemu_conf.h | 1 +
> src/qemu/test_libvirtd_qemu.aug.in | 1 +
> 5 files changed, 19 insertions(+)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 3c1045858b..f1b024a37f 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -104,6 +104,7 @@ module Libvirtd_qemu =
> | str_entry "slirp_helper"
> | str_entry "dbus_daemon"
> | bool_entry "set_process_name"
> + | bool_entry "external_limit_manager"
> | int_entry "max_processes"
> | int_entry "max_files"
> | limits_entry "max_core"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 0c1054f198..15cbc3ba38 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -662,6 +662,18 @@
> #
> #set_process_name = 1
>
> +# If enabled, libvirt will not attempt to change process limits (as
> +# configured with the max_processes, max_files and max_core settings
> +# below) itself but will instead expect an external entity to perform
> +# this task.
Can't users simply not set max_core, max_files, etc already ?
I think it is preferrable to have flags tailored specifically to
the individual limits, not a global flag. Otherwise you can end
up in a case where you want to disable the memory limits, but
keep the other limits set which is impossible with this global
flag.
> +#
> +# This also applies to the memory locking limit, which cannot be
> +# configured here and is instead calculated dynamically based on the
> +# exact guest configuration: if an external limit manager is in use,
> +# then libvirt will merely check that the limit has been set
> +# appropriately.
> +#
> +#external_limit_manager = 1
>
> # If max_processes is set to a positive integer, libvirt will use
> # it to set the maximum number of processes that can be run by qemu
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 2bbc75024c..ee95c124dd 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -673,6 +673,10 @@
> virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg,
>
> if (virConfGetValueBool(conf, "set_process_name", >setProcessName)
> < 0)
> return -1;
> +
> +if (virConfGetValueBool(conf, "external_limit_manager",
> >externalLimitManager) < 0)
> +return -1;
> +
> if (virConfGetValueUInt(conf, "max_processes", >maxProcesses) < 0)
> return -1;
> if (virConfGetValueUInt(conf, "max_files", >maxFiles) < 0)
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 7025b5222e..15e0353253 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -176,6 +176,7 @@ struct _virQEMUDriverConfig {
> bool nogfxAllowHostAudio;
> bool setProcessName;
>
> +bool externalLimitManager;
> unsigned int maxProcesses;
> unsigned int maxFiles;
> unsigned int maxThreadsPerProc;
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in
> b/src/qemu/test_libvirtd_qemu.aug.in
> index 9310dcec1c..73be55febe 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -77,6 +77,7 @@ module Test_libvirtd_qemu =
> { "hugetlbfs_mount" = "/dev/hugepages" }
> { "bridge_helper" = "/usr/libexec/qemu-bridge-helper" }
> { "set_process_name" = "1" }
> +{ "external_limit_manager" = "1" }
> { "max_processes" = "0" }
> { "max_files" = "0" }
> { "max_threads_per_process" = "0" }
> --
> 2.26.2
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 12/17] util: Try to get limits from /proc
On Fri, Mar 05, 2021 at 08:13:59PM +0100, Andrea Bolognani wrote:
> Calling prlimit() requires elevated privileges, specifically
> CAP_SYS_RESOURCE, and getrlimit() only works for the current
> process which is too limiting for our needs; /proc/$pid/limits,
> on the other hand, can be read by any process, so implement
> parsing that file as a fallback for when prlimit() fails.
>
> This is useful in containerized environments.
>
> Signed-off-by: Andrea Bolognani
> ---
> src/util/virprocess.c | 98 +++
> 1 file changed, 98 insertions(+)
>
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index abce522bf3..e62ec379a6 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -757,6 +757,95 @@ virProcessSetRLimit(int resource,
> #endif /* WITH_SETRLIMIT */
>
> #if WITH_GETRLIMIT
> +static const char*
> +virProcessLimitResourceToLabel(int resource)
> +{
> +switch (resource) {
> +# if defined(RLIMIT_MEMLOCK)
> +case RLIMIT_MEMLOCK:
> +return "Max locked memory";
> +# endif /* defined(RLIMIT_MEMLOCK) */
> +
> +# if defined(RLIMIT_NPROC)
> +case RLIMIT_NPROC:
> +return "Max processes";
> +# endif /* defined(RLIMIT_NPROC) */
> +
> +# if defined(RLIMIT_NOFILE)
> +case RLIMIT_NOFILE:
> +return "Max open files";
> +# endif /* defined(RLIMIT_NOFILE) */
> +
> +# if defined(RLIMIT_CORE)
> +case RLIMIT_CORE:
> +return "Max core file size";
> +# endif /* defined(RLIMIT_CORE) */
> +
> +default:
> +return NULL;
> +}
> +}
> +
> +static int
> +virProcessGetLimitFromProc(pid_t pid,
> + int resource,
> + struct rlimit *limit)
> +{
> +g_autofree char *procfile = NULL;
> +g_autofree char *buf = NULL;
> +g_auto(GStrv) lines = NULL;
> +const char *label;
> +size_t len;
> +size_t i;
> +
> +if (!(label = virProcessLimitResourceToLabel(resource))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unknown resource %d requested for process %lld"),
> + resource, (long long)pid);
> +return -1;
Setting errors on -1
> +}
> +
> +procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
> +
> +if (!g_file_get_contents(procfile, , , NULL))
> +return -1;
Not setting errors on -1
> +
> +if (!(lines = g_strsplit(buf, "\n", 0)))
> +return -1;
> +
> +for (i = 0; lines[i]; i++) {
> +g_autofree char *softLimit = NULL;
> +g_autofree char *hardLimit = NULL;
> +char *line = lines[i];
> +unsigned long long tmp;
> +
> +if (!STRPREFIX(line, label))
> +continue;
> +
> +line += strlen(label);
> +
> +if (sscanf(line, "%ms %ms %*s", , ) < 2)
> +return -1;
> +
> +if (STREQ(softLimit, "unlimited")) {
> +limit->rlim_cur = RLIM_INFINITY;
> +} else {
> +if (virStrToLong_ull(softLimit, NULL, 10, ) < 0)
> +return -1;
> +limit->rlim_cur = tmp;
> +}
> +if (STREQ(hardLimit, "unlimited")) {
> +limit->rlim_max = RLIM_INFINITY;
> +} else {
> +if (virStrToLong_ull(hardLimit, NULL, 10, ) < 0)
> +return -1;
> +limit->rlim_max = tmp;
> +}
> +}
> +
> +return 0;
> +}
> +
> static int
> virProcessGetLimit(pid_t pid,
> int resource,
> @@ -768,6 +857,15 @@ virProcessGetLimit(pid_t pid,
> if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
> return 0;
>
> +/* For whatever reason, using prlimit() on another process - even
> + * when it's just to obtain the current limit rather than changing
> + * it - requires CAP_SYS_RESOURCE, which we might not have in a
> + * containerized environment; on the other hand, no particular
> + * permission is needed to poke around /proc, so try that if going
> + * through the syscall didn't work */
> +if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
> +return 0;
This ought to be conditional for Linux only and error reporting needs
to be made consistent.
> +
> if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
> return 0;
>
> --
> 2.26.2
>
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 0/2] docs: less docs for insecure SASL mechanisms
On Fri, Mar 05, 2021 at 08:02:49AM +0100, Erik Skultety wrote: > On Thu, Mar 04, 2021 at 06:10:11PM +, Daniel P. Berrangé wrote: > > GSSAPI and SCRAM-SHA-256 are the only two SASL mechanisms we > > especially want people to be using. Even the latter is a little > > questionable due to storing passwords in cleartext on the server. > > At what point of the SCRAM-SHA-256 auth process is password handled as clear > text? I mean I tried to look up the issue you mention and couldn't find > anything, quite the contrary, e.g. Postgres says SCRAM-SHA-256 is the only > recommended scheme for password-based auth and storing passwords in clear text > is not possible. Isn't it kind of the point that passwords are never stored in > clear text with this scheme? You can clearly see the passwd in clear text in the file Add a new user $ echo "fish food" | saslpasswd2 -a libvirt demo Look for the password: $ strings /etc/libvirt/passwd.db | grep fish fish food The actual mechanism protocol does send in clear text over the wire. The storage in clear text on the server side is simply a choice of the cyrus-sasl impl of this mechanism documented here: https://www.cyrusimap.org/sasl/sasl/faqs/plaintextpasswords.html Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 02/17] util: Simplify stubs
On 3/5/21 8:13 PM, Andrea Bolognani wrote: Calling a stub should always result in ENOSYS being raised, regardless of what arguments are passed to it. Signed-off-by: Andrea Bolognani --- src/util/virprocess.c | 22 ++ 1 file changed, 6 insertions(+), 16 deletions(-) Missed virProcessSetMaxMemLock(). Michal
Re: [libvirt PATCH 11/17] tests: Mock virProcessGetMaxMemLock()
On 3/5/21 8:13 PM, Andrea Bolognani wrote:
Up until now we've implicitly relied on the fact that failures
reported from this function were simply ignored, but that's
about to change and so we need a proper mock.
Signed-off-by: Andrea Bolognani
---
src/util/virprocess.h | 2 +-
tests/virprocessmock.c | 7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 34210d6c9d..dbf4148e90 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -79,7 +79,7 @@ int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
int virProcessSetMaxFiles(pid_t pid, unsigned int files);
int virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes);
-int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes);
+int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes)
G_GNUC_NO_INLINE;
/* Callback to run code within the mount namespace tied to the given
* pid. This function must use only async-signal-safe functions, as
diff --git a/tests/virprocessmock.c b/tests/virprocessmock.c
index c9386d757a..0356ff2f70 100644
--- a/tests/virprocessmock.c
+++ b/tests/virprocessmock.c
@@ -21,6 +21,13 @@
#include
#include "virprocess.h"
+int
+virProcessGetMaxMemLock(pid_t pid G_GNUC_UNUSED, unsigned long long *bytes
G_GNUC_UNUSED)
Ehm, probably coffee hadn't kicked in? Because I can see @bytes used ..
+{
+*bytes = 0;
.. right here :-D
+return 0;
+}
+
int
virProcessSetMaxMemLock(pid_t pid G_GNUC_UNUSED, unsigned long long bytes
G_GNUC_UNUSED)
{
Michal
Re: [libvirt PATCH 12/17] util: Try to get limits from /proc
On 3/5/21 8:13 PM, Andrea Bolognani wrote:
Calling prlimit() requires elevated privileges, specifically
CAP_SYS_RESOURCE, and getrlimit() only works for the current
process which is too limiting for our needs; /proc/$pid/limits,
on the other hand, can be read by any process, so implement
parsing that file as a fallback for when prlimit() fails.
This is useful in containerized environments.
Signed-off-by: Andrea Bolognani
---
src/util/virprocess.c | 98 +++
1 file changed, 98 insertions(+)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index abce522bf3..e62ec379a6 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -757,6 +757,95 @@ virProcessSetRLimit(int resource,
#endif /* WITH_SETRLIMIT */
#if WITH_GETRLIMIT
+static const char*
+virProcessLimitResourceToLabel(int resource)
+{
+switch (resource) {
+# if defined(RLIMIT_MEMLOCK)
+case RLIMIT_MEMLOCK:
+return "Max locked memory";
+# endif /* defined(RLIMIT_MEMLOCK) */
+
+# if defined(RLIMIT_NPROC)
+case RLIMIT_NPROC:
+return "Max processes";
+# endif /* defined(RLIMIT_NPROC) */
+
+# if defined(RLIMIT_NOFILE)
+case RLIMIT_NOFILE:
+return "Max open files";
+# endif /* defined(RLIMIT_NOFILE) */
+
+# if defined(RLIMIT_CORE)
+case RLIMIT_CORE:
+return "Max core file size";
+# endif /* defined(RLIMIT_CORE) */
+
+default:
+return NULL;
+}
+}
+
+static int
+virProcessGetLimitFromProc(pid_t pid,
+ int resource,
+ struct rlimit *limit)
+{
+g_autofree char *procfile = NULL;
+g_autofree char *buf = NULL;
+g_auto(GStrv) lines = NULL;
+const char *label;
+size_t len;
+size_t i;
+
+if (!(label = virProcessLimitResourceToLabel(resource))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown resource %d requested for process %lld"),
+ resource, (long long)pid);
+return -1;
+}
+
+procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
+
+if (!g_file_get_contents(procfile, , , NULL))
+return -1;
+
+if (!(lines = g_strsplit(buf, "\n", 0)))
+return -1;
+
+for (i = 0; lines[i]; i++) {
+g_autofree char *softLimit = NULL;
+g_autofree char *hardLimit = NULL;
+char *line = lines[i];
+unsigned long long tmp;
+
+if (!STRPREFIX(line, label))
+continue;
+
+line += strlen(label);
Or if (!(line = STRSKIP(line, label)) continue;
+
+if (sscanf(line, "%ms %ms %*s", , ) < 2)
+return -1;
+
+if (STREQ(softLimit, "unlimited")) {
+limit->rlim_cur = RLIM_INFINITY;
+} else {
+if (virStrToLong_ull(softLimit, NULL, 10, ) < 0)
+return -1;
+limit->rlim_cur = tmp;
+}
+if (STREQ(hardLimit, "unlimited")) {
+limit->rlim_max = RLIM_INFINITY;
+} else {
+if (virStrToLong_ull(hardLimit, NULL, 10, ) < 0)
+return -1;
+limit->rlim_max = tmp;
+}
+}
+
+return 0;
+}
+
static int
virProcessGetLimit(pid_t pid,
int resource,
@@ -768,6 +857,15 @@ virProcessGetLimit(pid_t pid,
if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
return 0;
+/* For whatever reason, using prlimit() on another process - even
+ * when it's just to obtain the current limit rather than changing
+ * it - requires CAP_SYS_RESOURCE, which we might not have in a
+ * containerized environment; on the other hand, no particular
+ * permission is needed to poke around /proc, so try that if going
+ * through the syscall didn't work */
+if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
+return 0;
+
if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
return 0;
Michal
Re: [libvirt PATCH 04/17] util: Introduce virProcess{Get,Set}Limit()
On 3/5/21 8:13 PM, Andrea Bolognani wrote:
These functions abstract part of the existing logic, which is
the same in all virProcessSetMax*() functions, and changes it
so that which underlying syscall is used depends on their
availability rather than on the context in which they are
called: since prlimit() and {g,s}etrlimit() have slightly
different requirements, using the same one every single time
should make for a more consistent experience.
As part of the change, we also remove the special case for
passing zero to virProcessSetMax*() functions: we have removed
all callers that depended on that functionality in the previous
commit, so this is now safe to do and makes the semantics
simpler.
This commit is better viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani
---
src/util/virprocess.c | 175 +++---
1 file changed, 95 insertions(+), 80 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index e01ff25540..38b248217e 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -738,10 +738,66 @@ virProcessPrLimit(pid_t pid G_GNUC_UNUSED,
}
#endif
+#if WITH_GETRLIMIT
+static int
+virProcessGetRLimit(int resource,
+struct rlimit *old_limit)
+{
+return getrlimit(resource, old_limit);
+}
+#endif /* WITH_GETRLIMIT */
+
+#if WITH_SETRLIMIT
+static int
+virProcessSetRLimit(int resource,
+const struct rlimit *new_limit)
+{
+return setrlimit(resource, new_limit);
+}
+#endif /* WITH_SETRLIMIT */
+
+#if WITH_GETRLIMIT
Question of preference. I'd rather have these two WITH_GETRLIMIT
sections joined together [1]
+static int
+virProcessGetLimit(pid_t pid,
+ int resource,
+ struct rlimit *old_limit)
+{
+pid_t current_pid = getpid();
+bool same_process = (pid == current_pid);
+
+if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
+return 0;
+
+if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
+return 0;
+
+return -1;
+}
+#endif /* WITH_GETRLIMIT */
+
+#if WITH_SETRLIMIT
1: ... just like these two WITH_SETRLIMIT. But it's matter of taste, I
agree.
+static int
+virProcessSetLimit(pid_t pid,
+ int resource,
+ const struct rlimit *new_limit)
+{
+pid_t current_pid = getpid();
+bool same_process = (pid == current_pid);
+
+if (virProcessPrLimit(pid, resource, new_limit, NULL) == 0)
+return 0;
+
+if (same_process && virProcessSetRLimit(resource, new_limit) == 0)
+return 0;
+
+return -1;
+}
+#endif /* WITH_SETRLIMIT */
+
Michal
Re: [libvirt PATCH 16/17] qemu: Wire up external limit manager
On 3/5/21 8:14 PM, Andrea Bolognani wrote:
When the config knob is enabled, we simply skip the part where
limits are set; for the memory locking limit, which can change
dynamically over the lifetime of the guest, we still make sure
that the external process has set it correctly and error out
if that turns out not to be the case
This commit is better viewed with 'git show -w'.
https://bugzilla.redhat.com/show_bug.cgi?id=1916346
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.h | 1 +
src/qemu/qemu_domain.c | 39 ++--
src/qemu/qemu_process.c | 44 +++--
3 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7d208d881c..d1333020e1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2807,6 +2807,7 @@ struct _virDomainObj {
size_t ndeprecations;
char **deprecations;
+bool externalLimitManager; /* Whether process limits are handled outside of libvirt */
unsigned long long originalMemlock; /* Original RLIMIT_MEMLOCK, zero if no
* restore will be required later */
};
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f8b0e1a62a..0d9adb2f9c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9246,23 +9246,32 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm,
if (virProcessGetMaxMemLock(vm->pid, ) < 0)
return -1;
-if (desiredMemLock > 0) {
-/* If this is the first time adjusting the limit, save the current
- * value so that we can restore it once memory locking is no longer
- * required */
-if (vm->originalMemlock == 0) {
-vm->originalMemlock = currentMemLock;
+if (!vm->externalLimitManager) {
+if (desiredMemLock > 0) {
+/* If this is the first time adjusting the limit, save the current
+ * value so that we can restore it once memory locking is no longer
+ * required */
+if (vm->originalMemlock == 0) {
+vm->originalMemlock = currentMemLock;
+}
+} else {
+/* Once memory locking is no longer required, we can restore the
+ * original, usually very low, limit */
+desiredMemLock = vm->originalMemlock;
+vm->originalMemlock = 0;
}
-} else {
-/* Once memory locking is no longer required, we can restore the
- * original, usually very low, limit */
-desiredMemLock = vm->originalMemlock;
-vm->originalMemlock = 0;
-}
-if (desiredMemLock > 0 &&
-virProcessSetMaxMemLock(vm->pid, desiredMemLock) < 0) {
-return -1;
+if (desiredMemLock > 0 &&
+virProcessSetMaxMemLock(vm->pid, desiredMemLock) < 0) {
+return -1;
+}
+} else {
+if (currentMemLock < desiredMemLock) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("insufficient memlock limit (%llu < %llu)"),
+ currentMemLock, desiredMemLock);
Should we go this way? Our limit computation is nothing but a guess
(even though it might be more educated than others). I think we should
reduce this to a warning. User had chosen not to set any limits and
might have tracked down (e.g. via strace) the actual value needed which
may be smaller than our guess.
Another reason is that if domain is started with a VFIO/mdev device then
no memlock is set at all but here, during hotplug it would be.
+return -1;
+}
}
return 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c05cbe3570..2eac3934c7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7016,25 +7016,31 @@ qemuProcessLaunch(virConnectPtr conn,
virCommandSetPreExecHook(cmd, qemuProcessHook, );
virCommandSetUmask(cmd, 0x002);
-VIR_DEBUG("Setting up process limits");
-
-/* In some situations, eg. VFIO passthrough, QEMU might need to lock a
- * significant amount of memory, so we need to set the limit accordingly */
-maxMemLock = qemuDomainGetMemLockLimitBytes(vm->def, false);
-
-/* For all these settings, zero indicates that the limit should
- * not be set explicitly and the default/inherited limit should
- * be applied instead */
-if (maxMemLock > 0)
-virCommandSetMaxMemLock(cmd, maxMemLock);
-if (cfg->maxProcesses > 0)
-virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
-if (cfg->maxFiles > 0)
-virCommandSetMaxFiles(cmd, cfg->maxFiles);
-
-/* In this case, however, zero means that core dumps should be
- * disabled, and so we always need to set the limit explicitly */
-virCommandSetMaxCoreSize(cmd, cfg->maxCore);
+if (cfg->externalLimitManager) {
+VIR_DEBUG("Not setting up process limits (handled
Re: [libvirt PATCH 00/17] qemu: Implement external limit manager feature
On 3/5/21 8:13 PM, Andrea Bolognani wrote:
This feature has been requested by KubeVirt developers and will make
it possible for them to make some VFIO-related features, such as
migration and hotplug, work correctly.
https://bugzilla.redhat.com/show_bug.cgi?id=1916346
The first part of the series, especially the first 9 patches, is
preparation work: it addresses a few annoying issues with our APIs
that deal with process limits, and makes them all nice, consistent
and easy to reason about while moving policy code from the generic
code to the QEMU driver where it belongs.
Andrea Bolognani (17):
util: Document limit-related functions
util: Simplify stubs
util: Always pass a pid to virProcessSetMax*()
util: Introduce virProcess{Get,Set}Limit()
qemu: Make some minor tweaks
qemu: Set all limits at the same time
util: Have virCommand remember whether limits are set
qemu: Set limits only when explicitly asked to do so
util: Don't special-case setting a limit to zero
conf: Rename original_memlock -> originalMemlock
tests: Mock virProcessGetMaxMemLock()
util: Try to get limits from /proc
qemu: Don't ignore virProcessGetMaxMemLock() errors
qemu: Refactor qemuDomainAdjustMaxMemLock()
qemu: Add external_limit_manager config knob
qemu: Wire up external limit manager
news: Document external limit manager feature
NEWS.rst | 10 +
src/conf/domain_conf.h | 5 +-
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 12 +
src/qemu/qemu_command.c| 4 -
src/qemu/qemu_conf.c | 4 +
src/qemu/qemu_conf.h | 1 +
src/qemu/qemu_domain.c | 47 ++--
src/qemu/qemu_migration.c | 2 +
src/qemu/qemu_process.c| 30 ++-
src/qemu/test_libvirtd_qemu.aug.in | 1 +
src/util/vircommand.c | 21 +-
src/util/virprocess.c | 340 -
src/util/virprocess.h | 2 +-
tests/virprocessmock.c | 7 +
15 files changed, 354 insertions(+), 133 deletions(-)
Reviewed-by: Michal Privoznik
Michal
Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob
On 3/5/21 8:14 PM, Andrea Bolognani wrote:
This will be useful when libvirtd is running in a containerized
environment with limited capabilities, and in order to make
things like VFIO device assignment still work an external
privileged process changes the limits from outside of the
container. KubeVirt is an example of this setup.
Signed-off-by: Andrea Bolognani
---
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 12
src/qemu/qemu_conf.c | 4
src/qemu/qemu_conf.h | 1 +
src/qemu/test_libvirtd_qemu.aug.in | 1 +
5 files changed, 19 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 3c1045858b..f1b024a37f 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -104,6 +104,7 @@ module Libvirtd_qemu =
| str_entry "slirp_helper"
| str_entry "dbus_daemon"
| bool_entry "set_process_name"
+ | bool_entry "external_limit_manager"
| int_entry "max_processes"
| int_entry "max_files"
| limits_entry "max_core"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 0c1054f198..15cbc3ba38 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -662,6 +662,18 @@
#
#set_process_name = 1
+# If enabled, libvirt will not attempt to change process limits (as
+# configured with the max_processes, max_files and max_core settings
+# below) itself but will instead expect an external entity to perform
+# this task.
+#
+# This also applies to the memory locking limit, which cannot be
+# configured here and is instead calculated dynamically based on the
+# exact guest configuration: if an external limit manager is in use,
+# then libvirt will merely check that the limit has been set
+# appropriately.
+#
+#external_limit_manager = 1
# If max_processes is set to a positive integer, libvirt will use
# it to set the maximum number of processes that can be run by qemu
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2bbc75024c..ee95c124dd 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -673,6 +673,10 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr
cfg,
if (virConfGetValueBool(conf, "set_process_name", >setProcessName) < 0)
return -1;
+
+if (virConfGetValueBool(conf, "external_limit_manager",
>externalLimitManager) < 0)
+return -1;
+
if (virConfGetValueUInt(conf, "max_processes", >maxProcesses) < 0)
return -1;
if (virConfGetValueUInt(conf, "max_files", >maxFiles) < 0)
I think we could error out if external_limit_manager is set and one of
these max_processes, max_files, max_core is set also. It's not really a
combination we can make sense of, is it? It's perfectly okay to write
that in a separate patch though.
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 7025b5222e..15e0353253 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -176,6 +176,7 @@ struct _virQEMUDriverConfig {
bool nogfxAllowHostAudio;
bool setProcessName;
+bool externalLimitManager;
unsigned int maxProcesses;
unsigned int maxFiles;
unsigned int maxThreadsPerProc;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in
b/src/qemu/test_libvirtd_qemu.aug.in
index 9310dcec1c..73be55febe 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -77,6 +77,7 @@ module Test_libvirtd_qemu =
{ "hugetlbfs_mount" = "/dev/hugepages" }
{ "bridge_helper" = "/usr/libexec/qemu-bridge-helper" }
{ "set_process_name" = "1" }
+{ "external_limit_manager" = "1" }
{ "max_processes" = "0" }
{ "max_files" = "0" }
{ "max_threads_per_process" = "0" }
Michal
[PATCH] virDevMapperGetTargetsImpl: Use correct length when copying into dm.name
For reasons unknown, when rewriting this code and dropping
libdevmapper I've mistakenly used incorrect length of dm.name. In
linux/dm-ioctl.h the dm_ioctl struct is defined as follows:
#define DM_NAME_LEN 128
struct dm_ioctl {
...
char name[DM_NAME_LEN]; /* device name */
...
};
However, when copying string into this member, DM_TABLE_DEPS was
used, which is defined as follows:
#define DM_TABLE_DEPS_IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)
After decryption, this results in the following size: 3241737483.
Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e
Signed-off-by: Michal Privoznik
---
src/util/virdevmapper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index fcb11e954f..2c4c2df999 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -240,7 +240,7 @@ virDevMapperGetTargetsImpl(int controlFD,
if (!(sanitizedPath = virDMSanitizepath(path)))
return 0;
-if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) < 0) {
+if (virStrcpy(dm.name, sanitizedPath, DM_NAME_LEN) < 0) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("Resolved device mapper name too long"));
return -1;
--
2.26.2
