Re: [libvirt PATCH 02/10] Revert "qemuxml2xmltest: Convert all acpi-hotplug control related tests to DO_TEST_CAPS_LATEST"

2021-10-24 Thread Ani Sinha


On Sat, 23 Oct 2021, Laine Stump wrote:

> On 10/21/21 1:06 PM, Ján Tomko wrote:
> > On a Thursday in 2021, Laine Stump wrote:
> > > This reverts commit da896d440c7267e0b4575e4a3f2780bebf3fbfca.
> > >
> > > Signed-off-by: Laine Stump 
> > > ---
> > > ...i-hotplug-bridge-disable.x86_64-latest.xml | 36 -
> > > .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
> > > ...pi-hotplug-bridge-enable.x86_64-latest.xml | 36 -
> > > .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
> > > ...cpi-root-hotplug-disable.x86_64-latest.xml | 33 
> > > .../pc-i440fx-acpi-root-hotplug-disable.xml   |  1 +
> > > ...acpi-root-hotplug-enable.x86_64-latest.xml | 33 
> > > .../pc-i440fx-acpi-root-hotplug-enable.xml    |  1 +
> > > ...i-hotplug-bridge-disable.x86_64-latest.xml | 53 ---
> > > .../q35-acpi-hotplug-bridge-disable.xml   |  1 +
> > > ...pi-hotplug-bridge-enable.x86_64-latest.xml | 53 ---
> > > .../q35-acpi-hotplug-bridge-enable.xml    |  1 +
> > > tests/qemuxml2xmltest.c   | 24 ++---
> > > 13 files changed, 24 insertions(+), 250 deletions(-)
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-enable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-enable.xml
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
> > > delete mode 100644
> > > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml
> > > create mode 12
> > > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml
> > >
> > > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> > > index b0a1212a54..d6effbdac6 100644
> > > --- a/tests/qemuxml2xmltest.c
> > > +++ b/tests/qemuxml2xmltest.c
> > > @@ -424,12 +424,24 @@ mymain(void)
> > >     DO_TEST_NOCAPS("input-usbtablet");
> > >     DO_TEST_NOCAPS("misc-acpi");
> > >     DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3);
> > > -    DO_TEST_CAPS_LATEST("pc-i440fx-acpi-root-hotplug-disable");
> > > -    DO_TEST_CAPS_LATEST("pc-i440fx-acpi-root-hotplug-enable");
> > > -    DO_TEST_CAPS_LATEST("pc-i440fx-acpi-hotplug-bridge-disable");
> > > -    DO_TEST_CAPS_LATEST("pc-i440fx-acpi-hotplug-bridge-enable");
> > > -    DO_TEST_CAPS_LATEST("q35-acpi-hotplug-bridge-disable");
> > > -    DO_TEST_CAPS_LATEST("q35-acpi-hotplug-bridge-enable");
> > > +    DO_TEST("pc-i440fx-acpi-root-hotplug-disable",
> > > +    QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG);
> > > +    DO_TEST("pc-i440fx-acpi-root-hotplug-enable",
> > > +    QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG);
> >
> > Please only do a partial revert here and leave the i440fx root hotplug
> > tests using the _LATEST form, since you're only reverting bridge hotplug
> > tests by the end of the series.
>
> Huh, I thought I caught all of those (maybe I "re-un-re-reverted" it in
> another patch?). Thanks for catching it!
>
> I fixed it, but I'm waiting until Monday to push these.

Please also review and push two simple patches related to pci-root hotplug
that I sent. Hopefully they can make it in before the release.

After you revert, I will also need to make another pass through my
changes in case something was missed.

>
> >
> > (The only conflict will be in context in the second-to-last patch.
> >   Well, and the NEWS addition, because I pushed a patch there in the
> >   meantime)
> >
> > Jano
> >
> > > +    DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable",
> > > +    QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
> > > +    DO_TEST("pc-i440fx-acpi-hotplug-bridge-enable",
> > > +    QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
>
>

[PATCH 9/9] qapi: Extend -compat to set policy for unstable interfaces

2021-10-24 Thread Markus Armbruster
New option parameters unstable-input and unstable-output set policy
for unstable interfaces just like deprecated-input and
deprecated-output set policy for deprecated interfaces (see commit
6dd75472d5 "qemu-options: New -compat to set policy for deprecated
interfaces").  This is intended for testing users of the management
interfaces.  It is experimental.

For now, this covers only syntactic aspects of QMP, i.e. stuff tagged
with feature 'unstable'.  We may want to extend it to cover semantic
aspects, or the command line.

Note that there is no good way for management application to detect
presence of these new option parameters: they are not visible output
of query-qmp-schema or query-command-line-options.  Tolerable, because
it's meant for testing.  If running with -compat fails, skip the test.

Signed-off-by: Markus Armbruster 
---
 qapi/compat.json  |  6 +-
 include/qapi/util.h   |  1 +
 qapi/qmp-dispatch.c   |  6 ++
 qapi/qobject-output-visitor.c |  8 ++--
 qemu-options.hx   | 20 +++-
 scripts/qapi/events.py| 10 ++
 scripts/qapi/schema.py| 10 ++
 7 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/qapi/compat.json b/qapi/compat.json
index 74a8493d3d..9bc9804abb 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -47,9 +47,13 @@
 #
 # @deprecated-input: how to handle deprecated input (default 'accept')
 # @deprecated-output: how to handle deprecated output (default 'accept')
+# @unstable-input: how to handle unstable input (default 'accept')
+# @unstable-output: how to handle unstable output (default 'accept')
 #
 # Since: 6.0
 ##
 { 'struct': 'CompatPolicy',
   'data': { '*deprecated-input': 'CompatPolicyInput',
-'*deprecated-output': 'CompatPolicyOutput' } }
+'*deprecated-output': 'CompatPolicyOutput',
+'*unstable-input': 'CompatPolicyInput',
+'*unstable-output': 'CompatPolicyOutput' } }
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 0cc98db9f9..81a2b13a33 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -13,6 +13,7 @@
 
 typedef enum {
 QAPI_DEPRECATED,
+QAPI_UNSTABLE,
 } QapiSpecialFeature;
 
 typedef struct QEnumLookup {
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index e29ade134c..c5c6e521a2 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -59,6 +59,12 @@ bool compat_policy_input_ok(unsigned special_features,
 error_class, kind, name, errp)) {
 return false;
 }
+if ((special_features & (1u << QAPI_UNSTABLE))
+&& !compat_policy_input_ok1("Unstable",
+policy->unstable_input,
+error_class, kind, name, errp)) {
+return false;
+}
 return true;
 }
 
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index b5c6564cbb..74770edd73 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -212,8 +212,12 @@ static bool qobject_output_type_null(Visitor *v, const 
char *name,
 static bool qobject_output_policy_skip(Visitor *v, const char *name,
unsigned special_features)
 {
-return !(special_features && 1u << QAPI_DEPRECATED)
-|| v->compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE;
+CompatPolicy *pol = >compat_policy;
+
+return ((special_features & 1u << QAPI_DEPRECATED)
+&& pol->deprecated_output == COMPAT_POLICY_OUTPUT_HIDE)
+|| ((special_features & 1u << QAPI_UNSTABLE)
+&& pol->unstable_output == COMPAT_POLICY_OUTPUT_HIDE);
 }
 
 /* Finish building, and return the root object.
diff --git a/qemu-options.hx b/qemu-options.hx
index 5f375bbfa6..f051536b63 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3641,7 +3641,9 @@ DEFHEADING(Debug/Expert options:)
 
 DEF("compat", HAS_ARG, QEMU_OPTION_compat,
 "-compat 
[deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
-"Policy for handling deprecated management interfaces\n",
+"Policy for handling deprecated management interfaces\n"
+"-compat 
[unstable-input=accept|reject|crash][,unstable-output=accept|hide]\n"
+"Policy for handling unstable management interfaces\n",
 QEMU_ARCH_ALL)
 SRST
 ``-compat 
[deprecated-input=@var{input-policy}][,deprecated-output=@var{output-policy}]``
@@ -3659,6 +3661,22 @@ SRST
 Suppress deprecated command results and events
 
 Limitation: covers only syntactic aspects of QMP.
+
+``-compat 
[unstable-input=@var{input-policy}][,unstable-output=@var{output-policy}]``
+Set policy for handling unstable management interfaces (experimental):
+
+``unstable-input=accept`` (default)
+Accept unstable commands and arguments
+``unstable-input=reject``
+Reject unstable commands and arguments
+

[PATCH 7/9] qapi: Generalize enum member policy checking

2021-10-24 Thread Markus Armbruster
The code to check enumeration value policy can see special feature
flag 'deprecated' in QEnumLookup member flags[value].  I want to make
feature flag 'unstable' visible there as well, so I can add policy for
it.

Instead of extending flags[], replace it by @special_features (a
bitset of QapiSpecialFeature), because that's how special features get
passed around elsewhere.

Signed-off-by: Markus Armbruster 
---
 include/qapi/util.h|  5 +
 qapi/qapi-visit-core.c |  3 ++-
 scripts/qapi/types.py  | 22 --
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 7a8d5c7d72..0cc98db9f9 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -15,12 +15,9 @@ typedef enum {
 QAPI_DEPRECATED,
 } QapiSpecialFeature;
 
-/* QEnumLookup flags */
-#define QAPI_ENUM_DEPRECATED 1
-
 typedef struct QEnumLookup {
 const char *const *array;
-const unsigned char *const flags;
+const unsigned char *const special_features;
 const int size;
 } QEnumLookup;
 
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index b4a81f1757..5572d90efb 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -407,7 +407,8 @@ static bool input_type_enum(Visitor *v, const char *name, 
int *obj,
 return false;
 }
 
-if (lookup->flags && (lookup->flags[value] & QAPI_ENUM_DEPRECATED)) {
+if (lookup->special_features
+&& (lookup->special_features[value] & QAPI_DEPRECATED)) {
 switch (v->compat_policy.deprecated_input) {
 case COMPAT_POLICY_INPUT_ACCEPT:
 break;
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index ab2441adc9..3013329c24 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -16,7 +16,7 @@
 from typing import List, Optional
 
 from .common import c_enum_const, c_name, mcgen
-from .gen import QAPISchemaModularCVisitor, ifcontext
+from .gen import QAPISchemaModularCVisitor, gen_special_features, ifcontext
 from .schema import (
 QAPISchema,
 QAPISchemaEnumMember,
@@ -39,7 +39,7 @@ def gen_enum_lookup(name: str,
 members: List[QAPISchemaEnumMember],
 prefix: Optional[str] = None) -> str:
 max_index = c_enum_const(name, '_MAX', prefix)
-flags = ''
+feats = ''
 ret = mcgen('''
 
 const QEnumLookup %(c_name)s_lookup = {
@@ -54,19 +54,21 @@ def gen_enum_lookup(name: str,
 ''',
  index=index, name=memb.name)
 ret += memb.ifcond.gen_endif()
-if 'deprecated' in (f.name for f in memb.features):
-flags += mcgen('''
-[%(index)s] = QAPI_ENUM_DEPRECATED,
-''',
-   index=index)
 
-if flags:
+special_features = gen_special_features(memb.features)
+if special_features != '0':
+feats += mcgen('''
+[%(index)s] = %(special_features)s,
+''',
+   index=index, special_features=special_features)
+
+if feats:
 ret += mcgen('''
 },
-.flags = (const unsigned char[%(max_index)s]) {
+.special_features = (const unsigned char[%(max_index)s]) {
 ''',
  max_index=max_index)
-ret += flags
+ret += feats
 
 ret += mcgen('''
 },
-- 
2.31.1



[PATCH 8/9] qapi: Factor out compat_policy_input_ok()

2021-10-24 Thread Markus Armbruster
The code to check policy for handling deprecated input is triplicated.
Factor it out into compat_policy_input_ok() before I mess with it in
the next commit.

Signed-off-by: Markus Armbruster 
---
 include/qapi/compat-policy.h |  7 +
 qapi/qapi-visit-core.c   | 18 +
 qapi/qmp-dispatch.c  | 51 +++-
 qapi/qobject-input-visitor.c | 19 +++---
 4 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
index 1083f95122..8b7b25c0b5 100644
--- a/include/qapi/compat-policy.h
+++ b/include/qapi/compat-policy.h
@@ -13,10 +13,17 @@
 #ifndef QAPI_COMPAT_POLICY_H
 #define QAPI_COMPAT_POLICY_H
 
+#include "qapi/error.h"
 #include "qapi/qapi-types-compat.h"
 
 extern CompatPolicy compat_policy;
 
+bool compat_policy_input_ok(unsigned special_features,
+const CompatPolicy *policy,
+ErrorClass error_class,
+const char *kind, const char *name,
+Error **errp);
+
 /*
  * Create a QObject input visitor for @obj for use with QMP
  *
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 5572d90efb..a1ddfe8831 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/visitor.h"
@@ -408,18 +409,11 @@ static bool input_type_enum(Visitor *v, const char *name, 
int *obj,
 }
 
 if (lookup->special_features
-&& (lookup->special_features[value] & QAPI_DEPRECATED)) {
-switch (v->compat_policy.deprecated_input) {
-case COMPAT_POLICY_INPUT_ACCEPT:
-break;
-case COMPAT_POLICY_INPUT_REJECT:
-error_setg(errp, "Deprecated value '%s' disabled by policy",
-   enum_str);
-return false;
-case COMPAT_POLICY_INPUT_CRASH:
-default:
-abort();
-}
+&& !compat_policy_input_ok(lookup->special_features[value],
+   >compat_policy,
+   ERROR_CLASS_GENERIC_ERROR,
+   "value", enum_str, errp)) {
+return false;
 }
 
 *obj = value;
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 8cca18c891..e29ade134c 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -28,6 +28,40 @@
 
 CompatPolicy compat_policy;
 
+static bool compat_policy_input_ok1(const char *adjective,
+CompatPolicyInput policy,
+ErrorClass error_class,
+const char *kind, const char *name,
+Error **errp)
+{
+switch (policy) {
+case COMPAT_POLICY_INPUT_ACCEPT:
+return true;
+case COMPAT_POLICY_INPUT_REJECT:
+error_set(errp, error_class, "%s %s %s disabled by policy",
+  adjective, kind, name);
+return false;
+case COMPAT_POLICY_INPUT_CRASH:
+default:
+abort();
+}
+}
+
+bool compat_policy_input_ok(unsigned special_features,
+const CompatPolicy *policy,
+ErrorClass error_class,
+const char *kind, const char *name,
+Error **errp)
+{
+if ((special_features & 1u << QAPI_DEPRECATED)
+&& !compat_policy_input_ok1("Deprecated",
+policy->deprecated_input,
+error_class, kind, name, errp)) {
+return false;
+}
+return true;
+}
+
 Visitor *qobject_input_visitor_new_qmp(QObject *obj)
 {
 Visitor *v = qobject_input_visitor_new(obj);
@@ -176,19 +210,10 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject 
*request,
   "The command %s has not been found", command);
 goto out;
 }
-if (cmd->special_features & 1u << QAPI_DEPRECATED) {
-switch (compat_policy.deprecated_input) {
-case COMPAT_POLICY_INPUT_ACCEPT:
-break;
-case COMPAT_POLICY_INPUT_REJECT:
-error_set(, ERROR_CLASS_COMMAND_NOT_FOUND,
-  "Deprecated command %s disabled by policy",
-  command);
-goto out;
-case COMPAT_POLICY_INPUT_CRASH:
-default:
-abort();
-}
+if (!compat_policy_input_ok(cmd->special_features, _policy,
+ERROR_CLASS_COMMAND_NOT_FOUND,
+"command", command, )) {
+goto out;
 }
 if (!cmd->enabled) {
 error_set(, ERROR_CLASS_COMMAND_NOT_FOUND,
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index fda485614b..f0b4c7ca9d 100644
--- a/qapi/qobject-input-visitor.c
+++ 

[PATCH 4/9] qapi: Tools for sets of special feature flags in generated code

2021-10-24 Thread Markus Armbruster
New enum QapiSpecialFeature enumerates the special feature flags.

New helper gen_special_features() returns code to represent a
collection of special feature flags as a bitset.

The next few commits will put them to use.

Signed-off-by: Markus Armbruster 
---
 include/qapi/util.h|  4 
 scripts/qapi/gen.py| 13 +
 scripts/qapi/schema.py |  3 +++
 3 files changed, 20 insertions(+)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 257c600f99..7a8d5c7d72 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -11,6 +11,10 @@
 #ifndef QAPI_UTIL_H
 #define QAPI_UTIL_H
 
+typedef enum {
+QAPI_DEPRECATED,
+} QapiSpecialFeature;
+
 /* QEnumLookup flags */
 #define QAPI_ENUM_DEPRECATED 1
 
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 2ec1e7b3b6..9d07b88cf6 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -29,6 +29,7 @@
 mcgen,
 )
 from .schema import (
+QAPISchemaFeature,
 QAPISchemaIfCond,
 QAPISchemaModule,
 QAPISchemaObjectType,
@@ -37,6 +38,18 @@
 from .source import QAPISourceInfo
 
 
+def gen_special_features(features: QAPISchemaFeature):
+ret = ''
+sep = ''
+
+for feat in features:
+if feat.is_special():
+ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
+sep = ' | '
+
+return ret or '0'
+
+
 class QAPIGen:
 def __init__(self, fname: str):
 self.fname = fname
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 6d5f46509a..55f82d7389 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -725,6 +725,9 @@ def connect_doc(self, doc):
 class QAPISchemaFeature(QAPISchemaMember):
 role = 'feature'
 
+def is_special(self):
+return self.name in ('deprecated')
+
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
 def __init__(self, name, info, typ, optional, ifcond=None, features=None):
-- 
2.31.1



[PATCH 2/9] qapi: Mark unstable QMP parts with feature 'unstable'

2021-10-24 Thread Markus Armbruster
Add special feature 'unstable' everywhere the name starts with 'x-',
except for InputBarrierProperties member x-origin and
MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
because these two are actually stable.

Signed-off-by: Markus Armbruster 
---
 qapi/block-core.json | 123 +++
 qapi/migration.json  |  35 +---
 qapi/misc.json   |   6 ++-
 qapi/qom.json|  11 ++--
 4 files changed, 130 insertions(+), 45 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6d3217abb6..ce2c1352cb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1438,6 +1438,9 @@
 #
 # @x-perf: Performance options. (Since 6.0)
 #
+# Features:
+# @unstable: Member @x-perf is experimental.
+#
 # Note: @on-source-error and @on-target-error only affect background
 #   I/O.  If an error occurs during a guest write request, the device's
 #   rerror/werror actions will be used.
@@ -1452,7 +1455,9 @@
 '*on-source-error': 'BlockdevOnError',
 '*on-target-error': 'BlockdevOnError',
 '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
-'*filter-node-name': 'str', '*x-perf': 'BackupPerf'  } }
+'*filter-node-name': 'str',
+'*x-perf': { 'type': 'BackupPerf',
+ 'features': [ 'unstable' ] } } }
 
 ##
 # @DriveBackup:
@@ -1916,9 +1921,13 @@
 #
 # Get the block graph.
 #
+# Features:
+# @unstable: This command is meant for debugging.
+#
 # Since: 4.0
 ##
-{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph' }
+{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph',
+  'features': [ 'unstable' ] }
 
 ##
 # @drive-mirror:
@@ -2257,6 +2266,9 @@
 #
 # Get bitmap SHA256.
 #
+# Features:
+# @unstable: This command is meant for debugging.
+#
 # Returns: - BlockDirtyBitmapSha256 on success
 #  - If @node is not a valid block device, DeviceNotFound
 #  - If @name is not found or if hashing has failed, GenericError with 
an
@@ -2265,7 +2277,8 @@
 # Since: 2.10
 ##
 { 'command': 'x-debug-block-dirty-bitmap-sha256',
-  'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
+  'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256',
+  'features': [ 'unstable' ] }
 
 ##
 # @blockdev-mirror:
@@ -2495,27 +2508,57 @@
 #
 # 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
 #
+# Features:
+# @unstable: All members starting with x- are aliases for the same key
+#without x- in the @limits object.  This is not a stable
+#interface and may be removed or changed incompatibly in
+#the future.  Use @limits for a supported stable
+#interface.
+#
 # 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' } }
+'*x-iops-total': { 'type': 'int',
+   'features': [ 'unstable' ] },
+'*x-iops-total-max': { 'type': 'int',
+   'features': [ 'unstable' ] },
+'*x-iops-total-max-length': { 'type': 'int',
+  'features': [ 'unstable' ] },
+'*x-iops-read': { 'type': 'int',
+  'features': [ 'unstable' ] },
+'*x-iops-read-max': { 'type': 'int',
+  'features': [ 'unstable' ] },
+'*x-iops-read-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+'*x-iops-write': { 'type': 'int',
+   'features': [ 'unstable' ] },
+'*x-iops-write-max': { 'type': 'int',
+   'features': [ 'unstable' ] },
+'*x-iops-write-max-length': { 'type': 'int',
+  'features': [ 'unstable' ] },
+'*x-bps-total': { 'type': 'int',
+  

[PATCH 1/9] qapi: New special feature flag "unstable"

2021-10-24 Thread Markus Armbruster
By convention, names starting with "x-" are experimental.  The parts
of external interfaces so named may be withdrawn or changed
incompatibly in future releases.

Drawback: promoting something from experimental to stable involves a
name change.  Client code needs to be updated.

Moreover, the convention is not universally observed:

* QOM type "input-barrier" has properties "x-origin", "y-origin".
  Looks accidental, but it's ABI since 4.2.

* QOM types "memory-backend-file", "memory-backend-memfd",
  "memory-backend-ram", and "memory-backend-epc" have a property
  "x-use-canonical-path-for-ramblock-id" that is documented to be
  stable despite its name.

We could document these exceptions, but documentation helps only
humans.  We want to recognize "unstable" in code, like "deprecated".

Replace the convention by a new special feature flag "unstable".  It
will be recognized by the QAPI generator, like the existing feature
flag "deprecated", and unlike regular feature flags.

This commit updates documentation and prepares tests.  The next commit
updates the QAPI schema.  The remaining patches update the QAPI
generator and wire up -compat policy checking.

Signed-off-by: Markus Armbruster 
---
 docs/devel/qapi-code-gen.rst| 9 ++---
 tests/qapi-schema/qapi-schema-test.json | 7 +--
 tests/qapi-schema/qapi-schema-test.out  | 5 +
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 4071c9074a..38f2d7aad3 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -713,6 +713,10 @@ member as deprecated.  It is not supported elsewhere so 
far.
 Interfaces so marked may be withdrawn in future releases in accordance
 with QEMU's deprecation policy.
 
+Feature "unstable" marks a command, event, enum value, or struct
+member as unstable.  It is not supported elsewhere so far.  Interfaces
+so marked may be withdrawn or changed incompatibly in future releases.
+
 
 Naming rules and reserved names
 ---
@@ -746,9 +750,8 @@ Member name ``u`` and names starting with ``has-`` or 
``has_`` are reserved
 for the generator, which uses them for unions and for tracking
 optional members.
 
-Any name (command, event, type, member, or enum value) beginning with
-``x-`` is marked experimental, and may be withdrawn or changed
-incompatibly in a future release.
+Names beginning with ``x-`` used to signify "experimental".  This
+convention has been replaced by special feature "unstable".
 
 Pragmas ``command-name-exceptions`` and ``member-name-exceptions`` let
 you violate naming rules.  Use for new code is strongly discouraged. See
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index b677ab861d..43b8697002 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -273,7 +273,7 @@
   'data': { 'foo': { 'type': 'int', 'features': [ 'deprecated' ] } },
   'features': [ 'feature1' ] }
 { 'struct': 'FeatureStruct2',
-  'data': { 'foo': 'int' },
+  'data': { 'foo': { 'type': 'int', 'features': [ 'unstable' ] } },
   'features': [ { 'name': 'feature1' } ] }
 { 'struct': 'FeatureStruct3',
   'data': { 'foo': 'int' },
@@ -331,7 +331,7 @@
 { 'command': 'test-command-features1',
   'features': [ 'deprecated' ] }
 { 'command': 'test-command-features3',
-  'features': [ 'feature1', 'feature2' ] }
+  'features': [ 'unstable', 'feature1', 'feature2' ] }
 
 { 'command': 'test-command-cond-features1',
   'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] }
@@ -348,3 +348,6 @@
 
 { 'event': 'TEST_EVENT_FEATURES1',
   'features': [ 'deprecated' ] }
+
+{ 'event': 'TEST_EVENT_FEATURES2',
+  'features': [ 'unstable' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 16846dbeb8..1f9585fa9b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -308,6 +308,7 @@ object FeatureStruct1
 feature feature1
 object FeatureStruct2
 member foo: int optional=False
+feature unstable
 feature feature1
 object FeatureStruct3
 member foo: int optional=False
@@ -373,6 +374,7 @@ command test-command-features1 None -> None
 feature deprecated
 command test-command-features3 None -> None
 gen=True success_response=True boxed=False oob=False preconfig=False
+feature unstable
 feature feature1
 feature feature2
 command test-command-cond-features1 None -> None
@@ -394,6 +396,9 @@ event TEST_EVENT_FEATURES0 FeatureStruct1
 event TEST_EVENT_FEATURES1 None
 boxed=False
 feature deprecated
+event TEST_EVENT_FEATURES2 None
+boxed=False
+feature unstable
 module include/sub-module.json
 include sub-sub-module.json
 object SecondArrayRef
-- 
2.31.1



[PATCH 5/9] qapi: Generalize struct member policy checking

2021-10-24 Thread Markus Armbruster
The generated visitor functions call visit_deprecated_accept() and
visit_deprecated() when visiting a struct member with special feature
flag 'deprecated'.  This makes the feature flag visible to the actual
visitors.  I want to make feature flag 'unstable' visible there as
well, so I can add policy for it.

To let me make it visible, replace these functions by
visit_policy_reject() and visit_policy_skip(), which take the member's
special features as an argument.  Note that the new functions have the
opposite sense, i.e. the return value flips.

Signed-off-by: Markus Armbruster 
---
 include/qapi/visitor-impl.h   |  6 --
 include/qapi/visitor.h| 17 +
 qapi/qapi-forward-visitor.c   | 16 +---
 qapi/qapi-visit-core.c| 22 --
 qapi/qobject-input-visitor.c  | 15 ++-
 qapi/qobject-output-visitor.c |  9 ++---
 qapi/trace-events |  4 ++--
 scripts/qapi/visit.py | 14 +++---
 8 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 72b6537bef..2badec5ba4 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -114,10 +114,12 @@ struct Visitor
 void (*optional)(Visitor *v, const char *name, bool *present);
 
 /* Optional */
-bool (*deprecated_accept)(Visitor *v, const char *name, Error **errp);
+bool (*policy_reject)(Visitor *v, const char *name,
+  unsigned special_features, Error **errp);
 
 /* Optional */
-bool (*deprecated)(Visitor *v, const char *name);
+bool (*policy_skip)(Visitor *v, const char *name,
+unsigned special_features);
 
 /* Must be set */
 VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index dcb96018a9..d53a84c9ba 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -461,22 +461,31 @@ void visit_end_alternate(Visitor *v, void **obj);
 bool visit_optional(Visitor *v, const char *name, bool *present);
 
 /*
- * Should we reject deprecated member @name?
+ * Should we reject member @name due to policy?
+ *
+ * @special_features is the member's special features encoded as a
+ * bitset of QapiSpecialFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
-bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
+bool visit_policy_reject(Visitor *v, const char *name,
+ unsigned special_features, Error **errp);
 
 /*
- * Should we visit deprecated member @name?
+ *
+ * Should we skip member @name due to policy?
+ *
+ * @special_features is the member's special features encoded as a
+ * bitset of QapiSpecialFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
-bool visit_deprecated(Visitor *v, const char *name);
+bool visit_policy_skip(Visitor *v, const char *name,
+   unsigned special_features);
 
 /*
  * Set policy for handling deprecated management interfaces.
diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
index a4b111e22a..25d098aa8a 100644
--- a/qapi/qapi-forward-visitor.c
+++ b/qapi/qapi-forward-visitor.c
@@ -246,25 +246,27 @@ static void forward_field_optional(Visitor *v, const char 
*name, bool *present)
 visit_optional(ffv->target, name, present);
 }
 
-static bool forward_field_deprecated_accept(Visitor *v, const char *name,
-Error **errp)
+static bool forward_field_policy_reject(Visitor *v, const char *name,
+unsigned special_features,
+Error **errp)
 {
 ForwardFieldVisitor *ffv = to_ffv(v);
 
 if (!forward_field_translate_name(ffv, , errp)) {
 return false;
 }
-return visit_deprecated_accept(ffv->target, name, errp);
+return visit_policy_reject(ffv->target, name, special_features, errp);
 }
 
-static bool forward_field_deprecated(Visitor *v, const char *name)
+static bool forward_field_policy_skip(Visitor *v, const char *name,
+  unsigned special_features)
 {
 ForwardFieldVisitor *ffv = to_ffv(v);
 
 if (!forward_field_translate_name(ffv, , NULL)) {
 return false;
 }
-return visit_deprecated(ffv->target, name);
+return visit_policy_skip(ffv->target, name, special_features);
 }
 
 static void forward_field_complete(Visitor *v, void *opaque)
@@ -313,8 +315,8 @@ Visitor *visitor_forward_field(Visitor *target, const char 
*from, const char *to
 v->visitor.type_any = forward_field_type_any;
 v->visitor.type_null = forward_field_type_null;
 v->visitor.optional = forward_field_optional;
-

[PATCH 0/9] Configurable policy for handling unstable interfaces

2021-10-24 Thread Markus Armbruster
Option -compat lets you configure what to do when deprecated
interfaces get used.  This series extends this to unstable interfaces.
Works the same way.  Intended for testing users of the management
interfaces.  It is experimental.

To make it possible, I replace the "x-" naming convention by special
feature flag "unstable".  See PATCH 1 for rationale.

Based on my "[PATCH v4 0/5] qapi: Add feature flags to enum members"

Based-on: Message-Id: <20211025042405.3762351-1-arm...@redhat.com>

Markus Armbruster (9):
  qapi: New special feature flag "unstable"
  qapi: Mark unstable QMP parts with feature 'unstable'
  qapi: Eliminate QCO_NO_OPTIONS for a slight simplification
  qapi: Tools for sets of special feature flags in generated code
  qapi: Generalize struct member policy checking
  qapi: Generalize command policy checking
  qapi: Generalize enum member policy checking
  qapi: Factor out compat_policy_input_ok()
  qapi: Extend -compat to set policy for unstable interfaces

 docs/devel/qapi-code-gen.rst|   9 +-
 qapi/block-core.json| 123 +---
 qapi/compat.json|   6 +-
 qapi/migration.json |  35 +--
 qapi/misc.json  |   6 +-
 qapi/qom.json   |  11 ++-
 include/qapi/compat-policy.h|   7 ++
 include/qapi/qmp/dispatch.h |   6 +-
 include/qapi/util.h |   8 +-
 include/qapi/visitor-impl.h |   6 +-
 include/qapi/visitor.h  |  17 +++-
 monitor/misc.c  |   7 +-
 qapi/qapi-forward-visitor.c |  16 +--
 qapi/qapi-visit-core.c  |  41 
 qapi/qmp-dispatch.c |  57 ---
 qapi/qmp-registry.c |   4 +-
 qapi/qobject-input-visitor.c|  22 ++---
 qapi/qobject-output-visitor.c   |  13 ++-
 storage-daemon/qemu-storage-daemon.c|   3 +-
 qapi/trace-events   |   4 +-
 qemu-options.hx |  20 +++-
 scripts/qapi/commands.py|  12 +--
 scripts/qapi/events.py  |  10 +-
 scripts/qapi/gen.py |  13 +++
 scripts/qapi/schema.py  |  11 ++-
 scripts/qapi/types.py   |  22 +++--
 scripts/qapi/visit.py   |  14 +--
 tests/qapi-schema/qapi-schema-test.json |   7 +-
 tests/qapi-schema/qapi-schema-test.out  |   5 +
 29 files changed, 353 insertions(+), 162 deletions(-)

-- 
2.31.1



[PATCH 6/9] qapi: Generalize command policy checking

2021-10-24 Thread Markus Armbruster
The code to check command policy can see special feature flag
'deprecated' as command flag QCO_DEPRECATED.  I want to make feature
flag 'unstable' visible there as well, so I can add policy for it.

To let me make it visible, add member @special_features (a bitset of
QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
through qmp_register_command().  Then replace "QCO_DEPRECATED in
@flags" by QAPI_DEPRECATED in @special_features", and drop
QCO_DEPRECATED.

Signed-off-by: Markus Armbruster 
---
 include/qapi/qmp/dispatch.h  | 5 +++--
 monitor/misc.c   | 6 --
 qapi/qmp-dispatch.c  | 2 +-
 qapi/qmp-registry.c  | 4 +++-
 storage-daemon/qemu-storage-daemon.c | 3 ++-
 scripts/qapi/commands.py | 9 -
 6 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 0ce88200b9..1e4240fd0d 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -25,7 +25,6 @@ typedef enum QmpCommandOptions
 QCO_ALLOW_OOB =  (1U << 1),
 QCO_ALLOW_PRECONFIG   =  (1U << 2),
 QCO_COROUTINE =  (1U << 3),
-QCO_DEPRECATED=  (1U << 4),
 } QmpCommandOptions;
 
 typedef struct QmpCommand
@@ -34,6 +33,7 @@ typedef struct QmpCommand
 /* Runs in coroutine context if QCO_COROUTINE is set */
 QmpCommandFunc *fn;
 QmpCommandOptions options;
+unsigned special_features;
 QTAILQ_ENTRY(QmpCommand) node;
 bool enabled;
 const char *disable_reason;
@@ -42,7 +42,8 @@ typedef struct QmpCommand
 typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
-  QmpCommandFunc *fn, QmpCommandOptions options);
+  QmpCommandFunc *fn, QmpCommandOptions options,
+  unsigned special_features);
 const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
const char *name);
 void qmp_disable_command(QmpCommandList *cmds, const char *name,
diff --git a/monitor/misc.c b/monitor/misc.c
index 3556b177f6..c2d227a07c 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -230,11 +230,13 @@ static void monitor_init_qmp_commands(void)
 
 qmp_init_marshal(_commands);
 
-qmp_register_command(_commands, "device_add", qmp_device_add, 0);
+qmp_register_command(_commands, "device_add",
+ qmp_device_add, 0, 0);
 
 QTAILQ_INIT(_cap_negotiation_commands);
 qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
 }
 
 /* Set the current CPU defined by the user. Callers must hold BQL. */
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 7e943a0af5..8cca18c891 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject 
*request,
   "The command %s has not been found", command);
 goto out;
 }
-if (cmd->options & QCO_DEPRECATED) {
+if (cmd->special_features & 1u << QAPI_DEPRECATED) {
 switch (compat_policy.deprecated_input) {
 case COMPAT_POLICY_INPUT_ACCEPT:
 break;
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index f78c064aae..485bc5e6fc 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -16,7 +16,8 @@
 #include "qapi/qmp/dispatch.h"
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
-  QmpCommandFunc *fn, QmpCommandOptions options)
+  QmpCommandFunc *fn, QmpCommandOptions options,
+  unsigned special_features)
 {
 QmpCommand *cmd = g_malloc0(sizeof(*cmd));
 
@@ -27,6 +28,7 @@ void qmp_register_command(QmpCommandList *cmds, const char 
*name,
 cmd->fn = fn;
 cmd->enabled = true;
 cmd->options = options;
+cmd->special_features = special_features;
 QTAILQ_INSERT_TAIL(cmds, cmd, node);
 }
 
diff --git a/storage-daemon/qemu-storage-daemon.c 
b/storage-daemon/qemu-storage-daemon.c
index 10a1a33761..52cf17e8ac 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -146,7 +146,8 @@ static void init_qmp_commands(void)
 
 QTAILQ_INIT(_cap_negotiation_commands);
 qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
 }
 
 static int getopt_set_loc(int argc, char **argv, const char *optstring,
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index c8a975528f..21001bbd6b 100644

[PATCH 3/9] qapi: Eliminate QCO_NO_OPTIONS for a slight simplification

2021-10-24 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
---
 include/qapi/qmp/dispatch.h | 1 -
 monitor/misc.c  | 3 +--
 scripts/qapi/commands.py| 5 +
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 075203dc67..0ce88200b9 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -21,7 +21,6 @@ typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
 
 typedef enum QmpCommandOptions
 {
-QCO_NO_OPTIONS=  0x0,
 QCO_NO_SUCCESS_RESP   =  (1U << 0),
 QCO_ALLOW_OOB =  (1U << 1),
 QCO_ALLOW_PRECONFIG   =  (1U << 2),
diff --git a/monitor/misc.c b/monitor/misc.c
index ffe7966870..3556b177f6 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -230,8 +230,7 @@ static void monitor_init_qmp_commands(void)
 
 qmp_init_marshal(_commands);
 
-qmp_register_command(_commands, "device_add", qmp_device_add,
- QCO_NO_OPTIONS);
+qmp_register_command(_commands, "device_add", qmp_device_add, 0);
 
 QTAILQ_INIT(_cap_negotiation_commands);
 qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 3654825968..c8a975528f 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -229,15 +229,12 @@ def gen_register_command(name: str,
 if coroutine:
 options += ['QCO_COROUTINE']
 
-if not options:
-options = ['QCO_NO_OPTIONS']
-
 ret = mcgen('''
 qmp_register_command(cmds, "%(name)s",
  qmp_marshal_%(c_name)s, %(opts)s);
 ''',
 name=name, c_name=c_name(name),
-opts=" | ".join(options))
+opts=' | '.join(options) or 0)
 return ret
 
 
-- 
2.31.1



Re: [PATCH v4 5/5] block: Deprecate transaction type drive-backup

2021-10-24 Thread Markus Armbruster
I neglected to put RFC in the subject.



Re: [PATCH v3 1/5] qapi: Enable enum member introspection to show more than name

2021-10-24 Thread Markus Armbruster
Markus Armbruster  writes:

> The next commit will add feature flags to enum members.  There's a
> problem, though: query-qmp-schema shows an enum type's members as an
> array of member names (SchemaInfoEnum member @values).  If it showed
> an array of objects with a name member, we could simply add more
> members to these objects.  Since it's just strings, we can't.
>
> I can see three ways to correct this design mistake:
>
> 1. Do it the way we should have done it, plus compatibility goo.
>
>We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
>changing @values would be a compatibility break, add a new member
>@members instead.
>
>@values is now redundant.  In my testing, output of
>qemu-system-x86_64's query-qmp-schema grows by 11% (18.5KiB).
>
>We can deprecate @values now and drop it later.  This will break
>outmoded clients.  Well-behaved clients such as libvirt are
>expected to break cleanly.
>
> 2. Like 1, but omit "boring" elements of @member, and empty @member.
>
>@values does not become redundant.  @members augments it.  Somewhat
>cumbersome, but output of query-qmp-schema grows only as we make
>enum members non-boring.
>
>There is nothing to deprecate here.
>
> 3. Versioned query-qmp-schema.
>
>query-qmp-schema provides either @values or @members.  The QMP
>client can select which version it wants.  There is no redundant
>output.
>
>We can deprecate old versions and eventually drop them.  This will
>break outmoded clients.  Breaking cleanly is easier than for 1.
>
>While 1 and 2 operate within the common rules for compatible
>evolution apply (section "Compatibility considerations" in
>docs/devel/qapi-code-gen.rst), 3 bypasses them.  Attractive when
>operating within the rules is just too awkward.  Not the case here.
>
> This commit implements 1.  Libvirt developers prefer it.
>
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> Tested-by: Peter Krempa 
> Acked-by: Peter Krempa 

I meant to deprecate @values, but forgot.  I should really do it right
away, because... 

> ---
>  docs/devel/qapi-code-gen.rst | 15 +++
>  qapi/introspect.json | 21 +++--
>  scripts/qapi/introspect.py   | 18 ++
>  3 files changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index b2569de486..d267889d2c 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -1231,14 +1231,21 @@ Example: the SchemaInfo for ['str'] ::
>"element-type": "str" }
>  
>  The SchemaInfo for an enumeration type has meta-type "enum" and
> -variant member "values".  The values are listed in no particular
> -order; clients must search the entire enum when learning whether a
> -particular value is supported.
> +variant member "members".
> +
> +"members" is a JSON array describing the enumeration values.  Each
> +element is a JSON object with member "name" (the member's name).  The
> +"members" array is in no particular order; clients must search the
> +entire array when learning whether a particular value is supported.
>  
>  Example: the SchemaInfo for MyEnum from section `Enumeration types`_ ::
>  
>  { "name": "MyEnum", "meta-type": "enum",
> -  "values": [ "value1", "value2", "value3" ] }
> +  "members": [
> +{ "name": "value1" },
> +{ "name": "value2" },
> +{ "name": "value3" }
> +  ] }
>  
>  The SchemaInfo for a built-in type has the same name as the type in
>  the QAPI schema (see section `Built-in Types`_), with one exception

... this doesn't document @values anymore, only @members.

Done in v5.

[...]



[PATCH v4 0/5] qapi: Add feature flags to enum members

2021-10-24 Thread Markus Armbruster
PATCH 1+2 add feature flags to enum members.  Awkward due to an
introspection design mistake; see PATCH 1 for details.

PATCH 3+4 implement policy deprecated-input={reject,crash} for enum
values.

Policy deprecated-output=hide is not implemented, because we can't
hide a value without hiding the entire member, which is almost
certainly more than the requester of this policy bargained for.
Perhaps we want a new policy deprecated-output=hide-or-else-crash to
help us catch unwanted use of deprecated enum values.  Perhaps we want
deprecated-output=hide to behave that way together with
deprecated-input=crash.  Or even always.  Thoughts?

PATCH 5 puts the new feature flags to use.  It's RFC because it makes
sense only on top of Vladimir's deprecation of drive-backup.  See its
commit message for a reference.

I prefer to commit new features together with a use outside tests/.
PATCH 5 adds such a use, but it's RFC, because it depends on
Vladimir's work.  Perhaps another use pops up.  I can delay this work
in the hope of a use becoming ready, but the feature flags work I have
in the pipeline will eventually force my hand.

v4:
* PATCH 1: Deprecate SchemaInfoEnum member @values.
* PATCH 2: Doc tweak

v3:
* PATCH 1+2: Update qapi-code-gen.rst [Kevin, Eric]
* PATCH 4: Commit message typo [Eric], doc update moved to PATCH 2
* PATCH 5: Doc comment FIXME resolved [Kevin]

v2:
* Rebased with straightforward conflicts.
* PATCH 1-4: No longer RFC.
* PATCH 1: "Since" information fixed [Eric].  Commit message updated
  to reflect feedback.
* PATCH 2: Commit message amended to point out special feature flag
 'deprecated' is ignored at this stage.
* PATCH 4: Documentation updated.  Commit message tweaked.

Markus Armbruster (5):
  qapi: Enable enum member introspection to show more than name
  qapi: Add feature flags to enum members
  qapi: Move compat policy from QObject to generic visitor
  qapi: Implement deprecated-input={reject,crash} for enum values
  block: Deprecate transaction type drive-backup

 docs/about/deprecated.rst |  6 
 docs/devel/qapi-code-gen.rst  | 29 ++-
 qapi/compat.json  |  3 ++
 qapi/introspect.json  | 28 --
 qapi/transaction.json |  6 +++-
 include/qapi/qobject-input-visitor.h  |  4 ---
 include/qapi/qobject-output-visitor.h |  4 ---
 include/qapi/util.h   |  6 +++-
 include/qapi/visitor-impl.h   |  3 ++
 include/qapi/visitor.h|  9 ++
 qapi/qapi-visit-core.c| 27 +++--
 qapi/qmp-dispatch.c   |  4 +--
 qapi/qobject-input-visitor.c  | 14 +
 qapi/qobject-output-visitor.c | 14 +
 scripts/qapi/expr.py  |  3 +-
 scripts/qapi/introspect.py| 19 +---
 scripts/qapi/schema.py| 22 --
 scripts/qapi/types.py | 17 ++-
 tests/qapi-schema/doc-good.json   |  5 +++-
 tests/qapi-schema/doc-good.out|  3 ++
 tests/qapi-schema/doc-good.txt|  3 ++
 .../qapi-schema/enum-dict-member-unknown.err  |  2 +-
 tests/qapi-schema/qapi-schema-test.json   |  3 +-
 tests/qapi-schema/qapi-schema-test.out|  1 +
 tests/qapi-schema/test-qapi.py|  1 +
 25 files changed, 174 insertions(+), 62 deletions(-)

-- 
2.31.1



[PATCH v4 4/5] qapi: Implement deprecated-input={reject, crash} for enum values

2021-10-24 Thread Markus Armbruster
This copies the code implementing the policy from qapi/qmp-dispatch.c
to qapi/qobject-input-visitor.c.  Tolerable, but if we acquire more
copies, we should look into factoring them out.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Tested-by: Peter Krempa 
Acked-by: Peter Krempa 
---
 qapi/compat.json   |  3 ++-
 include/qapi/util.h|  6 +-
 qapi/qapi-visit-core.c | 18 +++---
 scripts/qapi/types.py  | 17 -
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/qapi/compat.json b/qapi/compat.json
index 1d2b76f00c..74a8493d3d 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -42,7 +42,8 @@
 # with feature 'deprecated'.  We may want to extend it to cover
 # semantic aspects, CLI, and experimental features.
 #
-# Limitation: not implemented for deprecated enumeration values.
+# Limitation: deprecated-output policy @hide is not implemented for
+# enumeration values.  They behave the same as with policy @accept.
 #
 # @deprecated-input: how to handle deprecated input (default 'accept')
 # @deprecated-output: how to handle deprecated output (default 'accept')
diff --git a/include/qapi/util.h b/include/qapi/util.h
index d7bfb30e25..257c600f99 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -11,9 +11,13 @@
 #ifndef QAPI_UTIL_H
 #define QAPI_UTIL_H
 
+/* QEnumLookup flags */
+#define QAPI_ENUM_DEPRECATED 1
+
 typedef struct QEnumLookup {
 const char *const *array;
-int size;
+const unsigned char *const flags;
+const int size;
 } QEnumLookup;
 
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 066f77a26d..49136ae88e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -393,7 +393,7 @@ static bool input_type_enum(Visitor *v, const char *name, 
int *obj,
 const QEnumLookup *lookup, Error **errp)
 {
 int64_t value;
-char *enum_str;
+g_autofree char *enum_str = NULL;
 
 if (!visit_type_str(v, name, _str, errp)) {
 return false;
@@ -402,11 +402,23 @@ static bool input_type_enum(Visitor *v, const char *name, 
int *obj,
 value = qapi_enum_parse(lookup, enum_str, -1, NULL);
 if (value < 0) {
 error_setg(errp, QERR_INVALID_PARAMETER, enum_str);
-g_free(enum_str);
 return false;
 }
 
-g_free(enum_str);
+if (lookup->flags && (lookup->flags[value] & QAPI_ENUM_DEPRECATED)) {
+switch (v->compat_policy.deprecated_input) {
+case COMPAT_POLICY_INPUT_ACCEPT:
+break;
+case COMPAT_POLICY_INPUT_REJECT:
+error_setg(errp, "Deprecated value '%s' disabled by policy",
+   enum_str);
+return false;
+case COMPAT_POLICY_INPUT_CRASH:
+default:
+abort();
+}
+}
+
 *obj = value;
 return true;
 }
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 831294fe42..ab2441adc9 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -38,6 +38,8 @@
 def gen_enum_lookup(name: str,
 members: List[QAPISchemaEnumMember],
 prefix: Optional[str] = None) -> str:
+max_index = c_enum_const(name, '_MAX', prefix)
+flags = ''
 ret = mcgen('''
 
 const QEnumLookup %(c_name)s_lookup = {
@@ -52,13 +54,26 @@ def gen_enum_lookup(name: str,
 ''',
  index=index, name=memb.name)
 ret += memb.ifcond.gen_endif()
+if 'deprecated' in (f.name for f in memb.features):
+flags += mcgen('''
+[%(index)s] = QAPI_ENUM_DEPRECATED,
+''',
+   index=index)
+
+if flags:
+ret += mcgen('''
+},
+.flags = (const unsigned char[%(max_index)s]) {
+''',
+ max_index=max_index)
+ret += flags
 
 ret += mcgen('''
 },
 .size = %(max_index)s
 };
 ''',
- max_index=c_enum_const(name, '_MAX', prefix))
+ max_index=max_index)
 return ret
 
 
-- 
2.31.1



[PATCH v4 1/5] qapi: Enable enum member introspection to show more than name

2021-10-24 Thread Markus Armbruster
The next commit will add feature flags to enum members.  There's a
problem, though: query-qmp-schema shows an enum type's members as an
array of member names (SchemaInfoEnum member @values).  If it showed
an array of objects with a name member, we could simply add more
members to these objects.  Since it's just strings, we can't.

I can see three ways to correct this design mistake:

1. Do it the way we should have done it, plus compatibility goo.

   We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
   changing @values would be a compatibility break, add a new member
   @members instead.

   @values is now redundant.  In my testing, output of
   qemu-system-x86_64's query-qmp-schema grows by 11% (18.5KiB).

   We can deprecate @values now and drop it later.  This will break
   outmoded clients.  Well-behaved clients such as libvirt are
   expected to break cleanly.

2. Like 1, but omit "boring" elements of @member, and empty @member.

   @values does not become redundant.  @members augments it.  Somewhat
   cumbersome, but output of query-qmp-schema grows only as we make
   enum members non-boring.

   There is nothing to deprecate here.

3. Versioned query-qmp-schema.

   query-qmp-schema provides either @values or @members.  The QMP
   client can select which version it wants.  There is no redundant
   output.

   We can deprecate old versions and eventually drop them.  This will
   break outmoded clients.  Breaking cleanly is easier than for 1.

   While 1 and 2 operate within the common rules for compatible
   evolution apply (section "Compatibility considerations" in
   docs/devel/qapi-code-gen.rst), 3 bypasses them.  Attractive when
   operating within the rules is just too awkward.  Not the case here.

This commit implements 1.  Libvirt developers prefer it.

Deprecate @values in favour of @members.  Since query-qmp-schema
compatibility is pretty fundamental for management applications, an
extended grace period is advised.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Tested-by: Peter Krempa 
Acked-by: Peter Krempa 
---
 docs/about/deprecated.rst|  6 ++
 docs/devel/qapi-code-gen.rst | 15 +++
 qapi/introspect.json | 25 +++--
 scripts/qapi/introspect.py   | 18 ++
 4 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0bed6ecb1d..be19317470 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -228,6 +228,12 @@ Use the more generic commands ``block-export-add`` and 
``block-export-del``
 instead.  As part of this deprecation, where ``nbd-server-add`` used a
 single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
 
+``query-qmp-schema`` return value member ``values`` (since 6.2)
+'''
+
+Member ``values`` in return value elements with meta-type ``enum`` is
+deprecated.  Use ``members`` instead.
+
 System accelerators
 ---
 
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index b2569de486..d267889d2c 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -1231,14 +1231,21 @@ Example: the SchemaInfo for ['str'] ::
   "element-type": "str" }
 
 The SchemaInfo for an enumeration type has meta-type "enum" and
-variant member "values".  The values are listed in no particular
-order; clients must search the entire enum when learning whether a
-particular value is supported.
+variant member "members".
+
+"members" is a JSON array describing the enumeration values.  Each
+element is a JSON object with member "name" (the member's name).  The
+"members" array is in no particular order; clients must search the
+entire array when learning whether a particular value is supported.
 
 Example: the SchemaInfo for MyEnum from section `Enumeration types`_ ::
 
 { "name": "MyEnum", "meta-type": "enum",
-  "values": [ "value1", "value2", "value3" ] }
+  "members": [
+{ "name": "value1" },
+{ "name": "value2" },
+{ "name": "value3" }
+  ] }
 
 The SchemaInfo for a built-in type has the same name as the type in
 the QAPI schema (see section `Built-in Types`_), with one exception
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 39bd303778..9683e884f8 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -142,14 +142,35 @@
 #
 # Additional SchemaInfo members for meta-type 'enum'.
 #
-# @values: the enumeration type's values, in no particular order.
+# @members: the enum type's members, in no particular order
+#   (since 6.2).
+#
+# @values: the enumeration type's member names, in no particular order.
+#  Redundant with @members.  Just for backward compatibility.
+#
+# Features:
+# @deprecated: Member @values is deprecated.  Use @members instead.
 #
 # Values of this type are JSON string on the wire.
 #
 # Since: 2.5
 

[PATCH v4 3/5] qapi: Move compat policy from QObject to generic visitor

2021-10-24 Thread Markus Armbruster
The next commit needs to access compat policy from the generic visitor
core.  Move it there from qobject input and output visitor.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 include/qapi/qobject-input-visitor.h  |  4 
 include/qapi/qobject-output-visitor.h |  4 
 include/qapi/visitor-impl.h   |  3 +++
 include/qapi/visitor.h|  9 +
 qapi/qapi-visit-core.c|  9 +
 qapi/qmp-dispatch.c   |  4 ++--
 qapi/qobject-input-visitor.c  | 14 +-
 qapi/qobject-output-visitor.c | 14 +-
 8 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/include/qapi/qobject-input-visitor.h 
b/include/qapi/qobject-input-visitor.h
index 8d69388810..95985e25e5 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -15,7 +15,6 @@
 #ifndef QOBJECT_INPUT_VISITOR_H
 #define QOBJECT_INPUT_VISITOR_H
 
-#include "qapi/qapi-types-compat.h"
 #include "qapi/visitor.h"
 
 typedef struct QObjectInputVisitor QObjectInputVisitor;
@@ -59,9 +58,6 @@ typedef struct QObjectInputVisitor QObjectInputVisitor;
  */
 Visitor *qobject_input_visitor_new(QObject *obj);
 
-void qobject_input_visitor_set_policy(Visitor *v,
-  CompatPolicyInput deprecated);
-
 /*
  * Create a QObject input visitor for @obj for use with keyval_parse()
  *
diff --git a/include/qapi/qobject-output-visitor.h 
b/include/qapi/qobject-output-visitor.h
index f2a2f92a00..2b1726baf5 100644
--- a/include/qapi/qobject-output-visitor.h
+++ b/include/qapi/qobject-output-visitor.h
@@ -15,7 +15,6 @@
 #define QOBJECT_OUTPUT_VISITOR_H
 
 #include "qapi/visitor.h"
-#include "qapi/qapi-types-compat.h"
 
 typedef struct QObjectOutputVisitor QObjectOutputVisitor;
 
@@ -54,7 +53,4 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
  */
 Visitor *qobject_output_visitor_new(QObject **result);
 
-void qobject_output_visitor_set_policy(Visitor *v,
-   CompatPolicyOutput deprecated);
-
 #endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 3b950f6e3d..72b6537bef 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -122,6 +122,9 @@ struct Visitor
 /* Must be set */
 VisitorType type;
 
+/* Optional */
+struct CompatPolicy compat_policy;
+
 /* Must be set for output visitors, optional otherwise. */
 void (*complete)(Visitor *v, void *opaque);
 
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b3c9ef7a81..dcb96018a9 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -16,6 +16,7 @@
 #define QAPI_VISITOR_H
 
 #include "qapi/qapi-builtin-types.h"
+#include "qapi/qapi-types-compat.h"
 
 /*
  * The QAPI schema defines both a set of C data types, and a QMP wire
@@ -477,6 +478,14 @@ bool visit_deprecated_accept(Visitor *v, const char *name, 
Error **errp);
  */
 bool visit_deprecated(Visitor *v, const char *name);
 
+/*
+ * Set policy for handling deprecated management interfaces.
+ *
+ * Intended use: call visit_set_policy(v, _policy) when
+ * visiting management interface input or output.
+ */
+void visit_set_policy(Visitor *v, CompatPolicy *policy);
+
 /*
  * Visit an enum value.
  *
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a641adec51..066f77a26d 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -19,6 +19,10 @@
 #include "qapi/visitor-impl.h"
 #include "trace.h"
 
+/* Zero-initialization must result in default policy */
+QEMU_BUILD_BUG_ON(COMPAT_POLICY_INPUT_ACCEPT || COMPAT_POLICY_OUTPUT_ACCEPT);
+
+
 void visit_complete(Visitor *v, void *opaque)
 {
 assert(v->type != VISITOR_OUTPUT || v->complete);
@@ -153,6 +157,11 @@ bool visit_deprecated(Visitor *v, const char *name)
 return true;
 }
 
+void visit_set_policy(Visitor *v, CompatPolicy *policy)
+{
+v->compat_policy = *policy;
+}
+
 bool visit_is_input(Visitor *v)
 {
 return v->type == VISITOR_INPUT;
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 59600210ce..7e943a0af5 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -32,7 +32,7 @@ Visitor *qobject_input_visitor_new_qmp(QObject *obj)
 {
 Visitor *v = qobject_input_visitor_new(obj);
 
-qobject_input_visitor_set_policy(v, compat_policy.deprecated_input);
+visit_set_policy(v, _policy);
 return v;
 }
 
@@ -40,7 +40,7 @@ Visitor *qobject_output_visitor_new_qmp(QObject **result)
 {
 Visitor *v = qobject_output_visitor_new(result);
 
-qobject_output_visitor_set_policy(v, compat_policy.deprecated_output);
+visit_set_policy(v, _policy);
 return v;
 }
 
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 04b790412e..71b24a4429 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -14,7 +14,6 @@
 
 #include "qemu/osdep.h"
 #include 
-#include "qapi/compat-policy.h"
 #include 

[PATCH v4 5/5] block: Deprecate transaction type drive-backup

2021-10-24 Thread Markus Armbruster
Several moons ago, Vladimir posted

Subject: [PATCH v2 3/3] qapi: deprecate drive-backup
Date: Wed,  5 May 2021 16:58:03 +0300
Message-Id: <20210505135803.67896-4-vsement...@virtuozzo.com>
https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg01394.html

with this

TODO: We also need to deprecate drive-backup transaction action..
But union members in QAPI doesn't support 'deprecated' feature. I tried
to dig a bit, but failed :/ Markus, could you please help with it? At
least by advice?

This is one way to resolve it.  Sorry it took so long.

John explored another way, namely adding feature flags to union
branches.  Could also be useful, say to add different features to
branches in multiple unions sharing the same tag enum.

Signed-off-by: Markus Armbruster 
---
 qapi/transaction.json | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi/transaction.json b/qapi/transaction.json
index d175b5f863..381a2df782 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -54,6 +54,10 @@
 # @blockdev-snapshot-sync: since 1.1
 # @drive-backup: Since 1.6
 #
+# Features:
+# @deprecated: Member @drive-backup is deprecated.  Use member
+#  @blockdev-backup instead.
+#
 # Since: 1.1
 ##
 { 'enum': 'TransactionActionKind',
@@ -62,7 +66,7 @@
 'block-dirty-bitmap-disable', 'block-dirty-bitmap-merge',
 'blockdev-backup', 'blockdev-snapshot',
 'blockdev-snapshot-internal-sync', 'blockdev-snapshot-sync',
-'drive-backup' ] }
+{ 'name': 'drive-backup', 'features': [ 'deprecated' ] } ] }
 
 ##
 # @AbortWrapper:
-- 
2.31.1



[PATCH v4 2/5] qapi: Add feature flags to enum members

2021-10-24 Thread Markus Armbruster
This is quite similar to commit 84ab008687 "qapi: Add feature flags to
struct members", only for enums instead of structs.

Special feature flag 'deprecated' is silently ignored there.  This is
okay only because it will be implemented shortly.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/devel/qapi-code-gen.rst  | 16 +-
 qapi/compat.json  |  2 ++
 qapi/introspect.json  |  5 -
 scripts/qapi/expr.py  |  3 ++-
 scripts/qapi/introspect.py|  5 +++--
 scripts/qapi/schema.py| 22 +--
 tests/qapi-schema/doc-good.json   |  5 -
 tests/qapi-schema/doc-good.out|  3 +++
 tests/qapi-schema/doc-good.txt|  3 +++
 .../qapi-schema/enum-dict-member-unknown.err  |  2 +-
 tests/qapi-schema/qapi-schema-test.json   |  3 ++-
 tests/qapi-schema/qapi-schema-test.out|  1 +
 tests/qapi-schema/test-qapi.py|  1 +
 13 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index d267889d2c..4071c9074a 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -200,7 +200,9 @@ Syntax::
  '*if': COND,
  '*features': FEATURES }
 ENUM-VALUE = STRING
-   | { 'name': STRING, '*if': COND }
+   | { 'name': STRING,
+   '*if': COND,
+   '*features': FEATURES }
 
 Member 'enum' names the enum type.
 
@@ -706,8 +708,10 @@ QEMU shows a certain behaviour.
 Special features
 
 
-Feature "deprecated" marks a command, event, or struct member as
-deprecated.  It is not supported elsewhere so far.
+Feature "deprecated" marks a command, event, enum value, or struct
+member as deprecated.  It is not supported elsewhere so far.
+Interfaces so marked may be withdrawn in future releases in accordance
+with QEMU's deprecation policy.
 
 
 Naming rules and reserved names
@@ -1157,7 +1161,8 @@ and "variants".
 
 "members" is a JSON array describing the object's common members, if
 any.  Each element is a JSON object with members "name" (the member's
-name), "type" (the name of its type), and optionally "default".  The
+name), "type" (the name of its type), "features" (a JSON array of
+feature strings), and "default".  The latter two are optional.  The
 member is optional if "default" is present.  Currently, "default" can
 only have value null.  Other values are reserved for future
 extensions.  The "members" array is in no particular order; clients
@@ -1234,7 +1239,8 @@ The SchemaInfo for an enumeration type has meta-type 
"enum" and
 variant member "members".
 
 "members" is a JSON array describing the enumeration values.  Each
-element is a JSON object with member "name" (the member's name).  The
+element is a JSON object with member "name" (the member's name), and
+optionally "features" (a JSON array of feature strings).  The
 "members" array is in no particular order; clients must search the
 entire array when learning whether a particular value is supported.
 
diff --git a/qapi/compat.json b/qapi/compat.json
index ae3afc22df..1d2b76f00c 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -42,6 +42,8 @@
 # with feature 'deprecated'.  We may want to extend it to cover
 # semantic aspects, CLI, and experimental features.
 #
+# Limitation: not implemented for deprecated enumeration values.
+#
 # @deprecated-input: how to handle deprecated input (default 'accept')
 # @deprecated-output: how to handle deprecated output (default 'accept')
 #
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 9683e884f8..183148b2e9 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -167,10 +167,13 @@
 #
 # @name: the member's name, as defined in the QAPI schema.
 #
+# @features: names of features associated with the member, in no
+#particular order.
+#
 # Since: 6.2
 ##
 { 'struct': 'SchemaInfoEnumMember',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*features': [ 'str' ] } }
 
 ##
 # @SchemaInfoArray:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 819ea6ad97..3cb389e875 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -472,7 +472,7 @@ def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> 
None:
   for m in members]
 for member in members:
 source = "'data' member"
-check_keys(member, info, source, ['name'], ['if'])
+check_keys(member, info, source, ['name'], ['if', 'features'])
 member_name = member['name']
 check_name_is_str(member_name, info, source)
 source = "%s '%s'" % (source, member_name)
@@ -483,6 +483,7 @@ def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> 
None:
  permit_upper=permissive,
  

Re: [libvirt PATCH 00/13] cgroup and thread management in ch driver.

2021-10-24 Thread Yan Fu
Hi,

Could you give a reference link for ch driver please?

Thanks,
Yan Fu

On Fri, Oct 22, 2021 at 11:43 PM Praveen K Paladugu <
pra...@linux.microsoft.com> wrote:

> This patchset adds support for cgroup management of ch threads. This
> version
> correctly manages cgroups for vcpu and emulator threads created by ch.
> cgroup
> management for iothreads is not yet supported.
>
> Along with cgroup management, this patchset also enables support for
> pinning
> vcpu and emulator threads to selected host cpus.
>
> Praveen K Paladugu (2):
>   ch_process: Setup emulator and iothread settings
>   ch_driver: emulator threadinfo & pinning callbacks
>
> Vineeth Pillai (11):
>   util: Helper functions to get process info
>   ch: Explicitly link to virt_util_lib
>   ch_domain: add virCHDomainGetMonitor helper method
>   ch_domain: add methods to manage private vcpu data
>   ch_driver,ch_domain: vcpu info getter callbacks
>   ch_driver: domainGetVcpuPinInfo and nodeGetCPUMap
>   ch_monitor: Get nicindexes in prep for cgroup mgmt
>   ch_cgroup: methods for cgroup mgmt in ch driver
>   ch_driver,ch_domain: vcpupin callback in ch driver
>   ch_driver: enable typed param string for numatune
>   ch_driver: add numatune callbacks for CH driver
>
>  po/POTFILES.in|   1 +
>  src/ch/ch_cgroup.c| 457 
>  src/ch/ch_cgroup.h|  45 +++
>  src/ch/ch_conf.c  |   2 +
>  src/ch/ch_conf.h  |   9 +-
>  src/ch/ch_domain.c| 170 -
>  src/ch/ch_domain.h|  32 +-
>  src/ch/ch_driver.c| 810 +-
>  src/ch/ch_monitor.c   | 254 -
>  src/ch/ch_monitor.h   |  60 +++-
>  src/ch/ch_process.c   | 368 ++-
>  src/ch/ch_process.h   |   3 +
>  src/ch/meson.build|   6 +
>  src/util/virprocess.c | 136 +++
>  src/util/virprocess.h |   5 +
>  15 files changed, 2329 insertions(+), 29 deletions(-)
>  create mode 100644 src/ch/ch_cgroup.c
>  create mode 100644 src/ch/ch_cgroup.h
>
> --
> 2.27.0
>
>
>


[PATCH v5 3/5] conf: add encryption engine property

2021-10-24 Thread Or Ozeri
This commit extends libvirt XML configuration to support a custom encryption 
engine.
This means that   becomes valid.
The only engine for now is qemu. However, a new engine (librbd) will be added 
in an upcoming commit.
If no engine is specified, qemu will be used (assuming qemu driver is used).

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  |  6 +
 docs/schemas/domainbackup.rng |  7 +
 docs/schemas/storagecommon.rng|  7 +
 src/conf/storage_encryption_conf.c| 26 ++-
 src/conf/storage_encryption_conf.h|  9 +++
 src/qemu/qemu_block.c |  2 ++
 src/qemu/qemu_domain.c| 20 ++
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 ++---
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +--
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 .../disk-slices.x86_64-latest.xml |  4 +--
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 +-
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +++
 16 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..178fcd0d7c 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -23,6 +23,12 @@
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..60dcfac06c 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -15,6 +15,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..7fd601e4a2 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -47,6 +47,11 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
   "default", "qcow", "luks",
 );
 
+VIR_ENUM_IMPL(virStorageEncryptionEngine,
+  VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
+  "default", "qemu",
+);
+
 static void
 virStorageEncryptionInfoDefClear(virStorageEncryptionInfoDef *def)
 {
@@ -120,6 +125,7 @@ virStorageEncryptionCopy(const virStorageEncryption *src)
 ret->secrets = g_new0(virStorageEncryptionSecret *, src->nsecrets);
 ret->nsecrets = src->nsecrets;
 ret->format = src->format;
+ret->engine = src->engine;
 
 for (i = 0; i < src->nsecrets; i++) {
 if (!(ret->secrets[i] = 
virStorageEncryptionSecretCopy(src->secrets[i])))
@@ -239,6 +245,12 @@ virStorageEncryptionParseNode(xmlNodePtr node,
 goto cleanup;
 }
 
+if (virXMLPropEnum(node, "engine",
+   virStorageEncryptionEngineTypeFromString,
+   VIR_XML_PROP_NONZERO,
+   >engine) < 0)
+  goto cleanup;
+
 if ((n = virXPathNodeSet("./secret", ctxt, )) < 0)
 goto cleanup;
 
@@ -327,6 +339,7 @@ int
 virStorageEncryptionFormat(virBuffer *buf,
virStorageEncryption *enc)
 {
+const char *engine;
 const char *format;
 size_t i;
 
@@ -335,7 +348,18 @@ virStorageEncryptionFormat(virBuffer *buf,
"%s", _("unexpected encryption format"));
 return -1;
 }
-virBufferAsprintf(buf, "\n", format);
+if (enc->engine == VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT) {
+virBufferAsprintf(buf, "\n", format);
+} else {
+if (!(engine = virStorageEncryptionEngineTypeToString(enc->engine))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("unexpected encryption engine"));
+return -1;
+}
+virBufferAsprintf(buf, "\n",
+  format, engine);
+}
+
 virBufferAdjustIndent(buf, 2);
 
 for (i = 0; i < enc->nsecrets; i++) {
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h

[PATCH v5 2/5] qemu: capablities: Detect presence of 'rbd-encryption' as QEMU_CAPS_RBD_ENCRYPTION

2021-10-24 Thread Or Ozeri
rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
 tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 +
 4 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cddd39924d..6e72a18455 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -651,6 +651,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "chardev.json", /* QEMU_CAPS_CHARDEV_JSON */
   "device.json", /* QEMU_CAPS_DEVICE_JSON */
   "query-dirty-rate", /* QEMU_CAPS_QUERY_DIRTY_RATE */
+  "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
 );
 
 
@@ -1561,6 +1562,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
+{ "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/discard", QEMU_CAPS_DRIVE_DISCARD },
 { "blockdev-add/arg-type/detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
 { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bb53d9ae46..338470ac5d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -631,6 +631,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_CHARDEV_JSON, /* -chardev accepts JSON */
 QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON */
 QEMU_CAPS_QUERY_DIRTY_RATE, /* accepts query-dirty-rate */
+QEMU_CAPS_RBD_ENCRYPTION, /* Ceph RBD encryption support */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 98c2fcedce..e60ed4705b 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -240,6 +240,7 @@
   
   
   
+  
   6001000
   0
   43100243
diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
index 5a46da0a6a..5622745347 100644
--- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
@@ -241,6 +241,7 @@
   
   
   
+  
   6001050
   0
   43100244
-- 
2.25.1



[PATCH v5 1/5] qemu: add disk post parse to qemublocktest

2021-10-24 Thread Or Ozeri
The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.
Specifically, this will be needed for tests that use luks encryption,
so that the default encryption engine (which is added in an upcoming commit)
will be overridden by qemu.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c |  2 +-
 src/qemu/qemu_domain.h |  3 +++
 tests/qemublocktest.c  | 29 -
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1bd3730281..5ff602e3af 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5218,7 +5218,7 @@ 
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
  unsigned int parseFlags)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9cf5d5479e..6728ab047e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -857,6 +857,9 @@ int qemuDomainSecretPrepare(virQEMUDriver *driver,
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
 virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+ unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
  const char *domainChannelTargetDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e61e923a9..0176fbd3f4 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -276,6 +276,9 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
 return -1;
 
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return -1;
+
 if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
 return -1;
 
@@ -470,32 +473,24 @@ testQemuImageCreateLoadDiskXML(const char *name,
virDomainXMLOption *xmlopt)
 
 {
-virDomainSnapshotDiskDef *diskdef = NULL;
-g_autoptr(xmlDoc) doc = NULL;
-g_autoptr(xmlXPathContext) ctxt = NULL;
-xmlNodePtr node;
+virDomainDiskDef *disk = NULL;
 g_autofree char *xmlpath = NULL;
-virStorageSource *ret = NULL;
+g_autofree char *xmlstr = NULL;
 
 xmlpath = g_strdup_printf("%s%s.xml", testQemuImageCreatePath, name);
 
-if (!(doc = virXMLParseFileCtxt(xmlpath, )))
+if (virTestLoadFile(xmlpath, ) < 0)
 return NULL;
 
-if (!(node = virXPathNode("//disk", ctxt))) {
-VIR_TEST_VERBOSE("failed to find  element\n");
+/* qemu stores node names in the status XML portion */
+if (!(disk = virDomainDiskDefParse(xmlstr, xmlopt,
+   VIR_DOMAIN_DEF_PARSE_STATUS)))
 return NULL;
-}
 
-diskdef = g_new0(virDomainSnapshotDiskDef, 1);
-
-if (virDomainSnapshotDiskDefParseXML(node, ctxt, diskdef,
- VIR_DOMAIN_DEF_PARSE_STATUS,
- xmlopt) == 0)
-ret = g_steal_pointer(>src);
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return NULL;
 
-virDomainSnapshotDiskDefFree(diskdef);
-return ret;
+return disk->src;
 }
 
 
-- 
2.25.1



[PATCH v5 0/5] Add support for librbd encryption

2021-10-24 Thread Or Ozeri
v5: rebased + nit fixes suggested by Peter
v4:
 - added disk post parse to image creation flow in qemublocktest (since more 
tests failed after adding engine validation)
 - removed symlink changes
 - added luks2 and engine documentation
 - switched to using enum engine instead of int
 - added validation for encryption engine and formats
v3: rebased on master
v2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)

Feel free to make any other changes before pushing. Thanks!

Or Ozeri (5):
  qemu: add disk post parse to qemublocktest
  qemu: capablities: Detect presence of 'rbd-encryption' as
QEMU_CAPS_RBD_ENCRYPTION
  conf: add encryption engine property
  qemu: add librbd encryption engine
  conf: add luks2 encryption format

 docs/formatstorageencryption.html.in  | 29 ++-
 docs/schemas/domainbackup.rng |  7 ++
 docs/schemas/storagecommon.rng|  9 ++
 src/conf/storage_encryption_conf.c| 28 ++-
 src/conf/storage_encryption_conf.h| 11 +++
 src/qemu/qemu_block.c | 41 +
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_domain.c| 69 ++-
 src/qemu/qemu_domain.h|  3 +
 tests/qemublocktest.c | 29 +++
 .../caps_6.1.0.x86_64.xml |  1 +
 .../caps_6.2.0.x86_64.xml |  1 +
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +-
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 49 +++
 .../disk-network-rbd-encryption.xml   | 75 +
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +-
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++
 .../disk-slices.x86_64-latest.xml |  4 +-
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 ++--
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +--
 tests/qemuxml2xmltest.c   |  1 +
 28 files changed, 443 insertions(+), 45 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

-- 
2.25.1



[PATCH v5 5/5] conf: add luks2 encryption format

2021-10-24 Thread Or Ozeri
This commit extends libvirt XML configuration to support luks2 encryption 
format.
This means that  becomes valid.
Currently librbd is the only engine that supports this new format.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 14 +-
 docs/schemas/storagecommon.rng   |  1 +
 src/conf/storage_encryption_conf.c   |  2 +-
 src/conf/storage_encryption_conf.h   |  1 +
 src/qemu/qemu_block.c|  9 +
 src/qemu/qemu_domain.c   |  9 -
 ...isk-network-rbd-encryption.x86_64-latest.args | 16 ++--
 .../disk-network-rbd-encryption.xml  | 12 
 ...disk-network-rbd-encryption.x86_64-latest.xml | 13 +
 9 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index fb04a6a0ad..86d884f93d 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,7 +18,7 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
@@ -125,6 +125,18 @@
   
 
 
+"luks2" format
+
+  The luks2 format is currently supported only by the
+  librbd engine, and can only be applied to RBD network disks.
+  Since the librbd engine is currently not supported by the
+  storage driver, you cannot use it to control such disks. However,
+  pre-formatted RBD luks2 disks can be loaded to a qemu VM using the qemu
+  VM driver.
+  A single
+  secret type='passphrase'... element is expected.
+
+
 
 Examples
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 3ddff02e43..591a158209 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -13,6 +13,7 @@
   default
   qcow
   luks
+  luks2
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index d45ad717a0..a65ef1f8a2 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
 );
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 0931618608..312599ad44 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -65,6 +65,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 4af06aea1b..34fdec2c4b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -908,6 +908,10 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 encformat = "luks";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+encformat = "luks2";
+break;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("librbd encryption engine only supports 
luks/luks2 formats"));
@@ -1358,6 +1362,11 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 encformat = "luks";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("luks2 is currently not supported by the qemu 
encryption engine"));
+return -1;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
 default:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 71cebec4e8..4080671dd8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1188,7 +1188,8 @@ static bool
 qemuDomainDiskHasEncryptionSecret(virStorageSource *src)
 {
 if (!virStorageSourceIsEmpty(src) && src->encryption &&
-src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS ||
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2) &&
 src->encryption->nsecrets > 0)
 return true;
 
@@ -4778,6 +4779,11 @@ 

[PATCH v5 4/5] qemu: add librbd encryption engine

2021-10-24 Thread Or Ozeri
rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  | 11 ++-
 docs/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/qemu/qemu_block.c | 30 
 src/qemu/qemu_domain.c| 38 ++
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 45 
 .../disk-network-rbd-encryption.xml   | 63 +
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 70 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 263 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 178fcd0d7c..fb04a6a0ad 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -27,7 +27,16 @@
   The encryption tag supports an optional engine
   tag, which allows selecting which component actually handles
   the encryption. Currently defined values of engine are
-  qemu.
+  qemu and librbd.
+  Both qemu and librbd require using the qemu
+  driver.
+  The librbd engine requires qemu version >= 6.1.0,
+  and is only applicable for RBD network disks.
+  If the engine tag is not specified, the qemu engine will be
+  used by default (assuming the qemu driver is used).
+  Note that librbd engine is currently only supported by the
+  qemu VM driver, and is not supported by the storage driver. Furthermore,
+  the storage driver currently ignores the engine tag.
 
 
   The encryption tag can currently contain a sequence of
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 60dcfac06c..3ddff02e43 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -19,6 +19,7 @@
 
   
 qemu
+librbd
   
 
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 7fd601e4a2..d45ad717a0 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
   VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
-  "default", "qemu",
+  "default", "qemu", "librbd",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index e0ac0fe4bf..0931618608 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -54,6 +54,7 @@ struct _virStorageEncryptionInfoDef {
 typedef enum {
 VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_ENGINE_QEMU,
+VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD,
 
 VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
 } virStorageEncryptionEngine;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0e2395278a..4af06aea1b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -875,6 +875,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) encrypt = NULL;
+const char *encformat;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
 g_autoptr(virJSONValue) mode = NULL;
@@ -899,12 +901,40 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 return NULL;
 }
 
+if (src->encryption &&
+src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+switch ((virStorageEncryptionFormatType) src->encryption->format) {
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
+encformat = "luks";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("librbd encryption engine only supports 
luks/luks2 formats"));
+return NULL;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
+default:
+