[PATCH 0/7] secret: Relax usage 'name' regex
Patches 1-6 are mostly cleanups noticed while looking at the schema. See patch 7/7 for explanation. Peter Krempa (7): virSecretDefParseUsage: Use g_autofree for type_str secretXMLParseNode: Clean up freeing of memory virSecretLookupParseSecret: Use g_steal_pointer schema: domaincommon: Remove pointless 'choice' from 'inituser'/'initgroup' schema: Remove workaround for bug in libxml2 2.7.6 schema: Add define for object names schema: secret: Relax requirements for usage name docs/schemas/basictypes.rng | 13 ++++++++ docs/schemas/domaincommon.rng | 33 +++++-------------- docs/schemas/secret.rng | 8 ++--- docs/schemas/storagecommon.rng | 8 ----- docs/schemas/storagepool.rng | 4 +-- src/conf/secret_conf.c | 20 ++++------- src/util/virsecret.c | 3 +- .../disk-network-source-auth.args | 8 ++++- ...isk-network-source-auth.x86_64-2.12.0.args | 9 +++++ ...isk-network-source-auth.x86_64-latest.args | 29 +++++++++++----- .../disk-network-source-auth.xml | 12 +++++++ tests/qemuxml2argvtest.c | 3 +- .../disk-network-source-auth.xml | 13 ++++++++ tests/secretxml2xmlin/usage-ceph-space.xml | 7 ++++ tests/secretxml2xmltest.c | 1 + 15 files changed, 107 insertions(+), 64 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-ceph-space.xml -- 2.29.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 273987bdb4..9cacc43e28 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -52,7 +52,7 @@ static int virSecretDefParseUsage(xmlXPathContextPtr ctxt, virSecretDefPtr def) { - char *type_str; + g_autofree char *type_str = NULL; int type; type_str = virXPathString("string(./usage/@type)", ctxt); @@ -65,10 +65,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, if (type < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown secret usage type %s"), type_str); - VIR_FREE(type_str); return -1; } - VIR_FREE(type_str); def->usage_type = type; switch (def->usage_type) { case VIR_SECRET_USAGE_TYPE_NONE: -- 2.29.2
Use one variable per extracted property instead of reusing strings and drop needless VIR_FREE calls. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 9cacc43e28..4d7d685d6e 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -131,7 +131,8 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) { g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(virSecretDef) def = NULL; - g_autofree char *prop = NULL; + g_autofree char *ephemeralstr = NULL; + g_autofree char *privatestr = NULL; g_autofree char *uuidstr = NULL; if (!virXMLNodeNameEqual(root, "secret")) { @@ -149,24 +150,20 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) def = g_new0(virSecretDef, 1); - prop = virXPathString("string(./@ephemeral)", ctxt); - if (prop != NULL) { - if (virStringParseYesNo(prop, &def->isephemeral) < 0) { + if ((ephemeralstr = virXPathString("string(./@ephemeral)", ctxt))) { + if (virStringParseYesNo(ephemeralstr, &def->isephemeral) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'ephemeral'")); return NULL; } - VIR_FREE(prop); } - prop = virXPathString("string(./@private)", ctxt); - if (prop != NULL) { - if (virStringParseYesNo(prop, &def->isprivate) < 0) { + if ((privatestr = virXPathString("string(./@private)", ctxt))) { + if (virStringParseYesNo(privatestr, &def->isprivate) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'private'")); return NULL; } - VIR_FREE(prop); } uuidstr = virXPathString("string(./uuid)", ctxt); @@ -182,7 +179,6 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) "%s", _("malformed uuid element")); return NULL; } - VIR_FREE(uuidstr); } def->description = virXPathString("string(./description)", ctxt); -- 2.29.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virsecret.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 9ed803d45b..57c8583539 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -90,8 +90,7 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, } def->type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { - def->u.usage = usage; - usage = NULL; + def->u.usage = g_steal_pointer(&usage); def->type = VIR_SECRET_LOOKUP_TYPE_USAGE; } return 0; -- 2.29.2
'genericName' allows arbitrary numeric strings so using an explicit 'unsignedInt' choice is pointless. The elements take an username or a uid which is prefixed by '+', both of which are covered by 'genericName'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 24b4994670..bcbaee9db8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -436,16 +436,10 @@ </optional> <optional> <element name="inituser"> - <choice> - <ref name="unsignedInt"/> - <ref name="genericName"/> - </choice> + <ref name="genericName"/> </element> <element name="initgroup"> - <choice> - <ref name="unsignedInt"/> - <ref name="genericName"/> - </choice> + <ref name="genericName"/> </element> </optional> </interleave> -- 2.29.2
New libxml2 handles '\n' properly so the literal newline is not necessary. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 8 ++------ docs/schemas/storagecommon.rng | 4 +--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index bcbaee9db8..39bed92115 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -19,9 +19,7 @@ <define name="title"> <element name="title"> <data type="string"> - <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> - <param name="pattern">[^ -]+</param> + <param name="pattern">[^\n]+</param> </data> </element> </define> @@ -6948,9 +6946,7 @@ </define> <define name="domainName"> <data type="string"> - <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> - <param name="pattern">[^ -]+</param> + <param name="pattern">[^\n]+</param> </data> </define> <define name="diskSerial"> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 6597ebc824..54619d4cb0 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -8,9 +8,7 @@ <define name="poolName"> <data type="string"> - <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> - <param name="pattern">[^/ -]+</param> + <param name="pattern">[^/\n]+</param> </data> </define> -- 2.29.2
On a Wednesday in 2021, Peter Krempa wrote:
New libxml2 handles '\n' properly so the literal newline is not necessary.
, because 2.9.1 is the minimum version we support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 8 ++------ docs/schemas/storagecommon.rng | 4 +--- 2 files changed, 3 insertions(+), 9 deletions(-)
Jano
Objects such as domain, pool, etc re-define the regex for the format. Add more generic types for objects with/without a slash which we'll be able to reuse also for other objects. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/basictypes.rng | 13 +++++++++++++ docs/schemas/domaincommon.rng | 17 +++++------------ docs/schemas/storagecommon.rng | 6 ------ docs/schemas/storagepool.rng | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index fc52799466..a221ff6295 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -261,6 +261,19 @@ </choice> </define> + <!-- objectName represents any generic string for naming objects like domain --> + <define name="objectNameWithSlash"> + <data type="string"> + <param name="pattern">[^\n]+</param> + </data> + </define> + + <define name="objectName"> + <data type="string"> + <param name="pattern">[^/\n]+</param> + </data> + </define> + <define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 39bed92115..4fc6a7ee7a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -18,9 +18,7 @@ <define name="title"> <element name="title"> - <data type="string"> - <param name="pattern">[^\n]+</param> - </data> + <ref name="objectNameWithSlash"/> </element> </define> @@ -559,7 +557,7 @@ </optional> <interleave> <element name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/> </element> <optional> <element name="uuid"> @@ -1391,7 +1389,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/> </attribute> <empty/> </element> @@ -2056,7 +2054,7 @@ <element name="source"> <interleave> <attribute name="pool"> - <ref name="poolName"/> + <ref name="objectName"/> </attribute> <attribute name="volume"> <ref name="volName"/> @@ -3269,7 +3267,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/> </attribute> <empty/> </element> @@ -6944,11 +6942,6 @@ <param name="maxInclusive">1000</param> </data> </define> - <define name="domainName"> - <data type="string"> - <param name="pattern">[^\n]+</param> - </data> - </define> <define name="diskSerial"> <data type="string"> <param name="pattern">[A-Za-z0-9_\.\+\- ]+</param> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 54619d4cb0..e3d08a8410 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -6,12 +6,6 @@ <!-- This schema is not designed for standalone use; another file must include both this file and basictypes.rng --> - <define name="poolName"> - <data type="string"> - <param name="pattern">[^/\n]+</param> - </data> - </define> - <define name="encryption"> <element name="encryption"> <attribute name="format"> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index a87d22f6fc..bd24b8b8d0 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -247,7 +247,7 @@ <interleave> <optional> <element name="name"> - <ref name="poolName"/> + <ref name="objectName"/> </element> </optional> <optional> @@ -261,7 +261,7 @@ <define name="commonmetadata"> <interleave> <element name="name"> - <ref name="poolName"/> + <ref name="objectName"/> </element> <optional> <element name="uuid"> -- 2.29.2
On a Wednesday in 2021, Peter Krempa wrote:
Objects such as domain, pool, etc re-define the regex for the format. Add more generic types for objects with/without a slash which we'll be able to reuse also for other objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/basictypes.rng | 13 +++++++++++++ docs/schemas/domaincommon.rng | 17 +++++------------ docs/schemas/storagecommon.rng | 6 ------ docs/schemas/storagepool.rng | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index fc52799466..a221ff6295 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -261,6 +261,19 @@ </choice> </define>
+ <!-- objectName represents any generic string for naming objects like domain --> + <define name="objectNameWithSlash"> + <data type="string"> + <param name="pattern">[^\n]+</param> + </data> + </define> + + <define name="objectName"> + <data type="string"> + <param name="pattern">[^/\n]+</param> + </data> + </define> + <define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 39bed92115..4fc6a7ee7a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -18,9 +18,7 @@
<define name="title"> <element name="title"> - <data type="string"> - <param name="pattern">[^\n]+</param> - </data> + <ref name="objectNameWithSlash"/> </element> </define>
@@ -559,7 +557,7 @@ </optional> <interleave> <element name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
This is the domain name and cannot contain a slash.
</element> <optional> <element name="uuid"> @@ -1391,7 +1389,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
Added by: conf: support backend domain name in disk and network devices At least Xen supports backend drivers in another domain (aka "driver domain"). This patch introduces an XML config option for specifying the backend domain name for <disk> and <interface> devices. E.g. <disk> <backenddomain name='diskvm'/> ... </disk> <interface type='bridge'> <backenddomain name='netvm'/> ... </interface> I see no need to relax this to allow a slash.
</attribute> <empty/> </element> @@ -2056,7 +2054,7 @@ <element name="source"> <interleave> <attribute name="pool"> - <ref name="poolName"/> + <ref name="objectName"/> </attribute> <attribute name="volume"> <ref name="volName"/> @@ -3269,7 +3267,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
Same here.
</attribute> <empty/> </element> @@ -6944,11 +6942,6 @@ <param name="maxInclusive">1000</param> </data> </define> - <define name="domainName"> - <data type="string"> - <param name="pattern">[^\n]+</param> - </data> - </define> <define name="diskSerial"> <data type="string"> <param name="pattern">[A-Za-z0-9_\.\+\- ]+</param>
Jano
On Wed, Jan 06, 2021 at 21:59:21 +0100, Ján Tomko wrote:
On a Wednesday in 2021, Peter Krempa wrote:
Objects such as domain, pool, etc re-define the regex for the format. Add more generic types for objects with/without a slash which we'll be able to reuse also for other objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/basictypes.rng | 13 +++++++++++++ docs/schemas/domaincommon.rng | 17 +++++------------ docs/schemas/storagecommon.rng | 6 ------ docs/schemas/storagepool.rng | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-)
[...]
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 39bed92115..4fc6a7ee7a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng
[...]
@@ -559,7 +557,7 @@ </optional> <interleave> <element name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
This is the domain name and cannot contain a slash.
This is not true entirely. The check in the code that enforces that '/' is not in the domain name is conditional. It's based on the VIR_DOMAIN_DEF_FEATURE_NAME_SLASH flag being asserted by the hypervisors which don't refuse slash in domain name. The following drivers use it: src/openvz/openvz_conf.c: .features = VIR_DOMAIN_DEF_FEATURE_NAME_SLASH, src/vbox/vbox_common.c: .features = VIR_DOMAIN_DEF_FEATURE_NAME_SLASH, src/vmx/vmx.c: VIR_DOMAIN_DEF_FEATURE_NAME_SLASH |
</element> <optional> <element name="uuid"> @@ -1391,7 +1389,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
Added by:
conf: support backend domain name in disk and network devices
At least Xen supports backend drivers in another domain (aka "driver domain"). This patch introduces an XML config option for specifying the backend domain name for <disk> and <interface> devices. E.g.
<disk> <backenddomain name='diskvm'/> ... </disk> <interface type='bridge'> <backenddomain name='netvm'/> ... </interface>
I see no need to relax this to allow a slash.
This isn't relaxing it, but merely keeping it as it was, since 'domainName' rng type allows slash. On the other hand, nothing actually uses 'backenddomain', so we can use the stricter version without any problem.
</attribute> <empty/> </element>
On a Thursday in 2021, Peter Krempa wrote:
On Wed, Jan 06, 2021 at 21:59:21 +0100, Ján Tomko wrote:
On a Wednesday in 2021, Peter Krempa wrote:
Objects such as domain, pool, etc re-define the regex for the format. Add more generic types for objects with/without a slash which we'll be able to reuse also for other objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/basictypes.rng | 13 +++++++++++++ docs/schemas/domaincommon.rng | 17 +++++------------ docs/schemas/storagecommon.rng | 6 ------ docs/schemas/storagepool.rng | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-)
[...]
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 39bed92115..4fc6a7ee7a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng
[...]
@@ -559,7 +557,7 @@ </optional> <interleave> <element name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
This is the domain name and cannot contain a slash.
This is not true entirely. The check in the code that enforces that '/' is not in the domain name is conditional.
Right, I did not realize that.
It's based on the VIR_DOMAIN_DEF_FEATURE_NAME_SLASH flag being asserted by the hypervisors which don't refuse slash in domain name. The following drivers use it:
src/openvz/openvz_conf.c: .features = VIR_DOMAIN_DEF_FEATURE_NAME_SLASH, src/vbox/vbox_common.c: .features = VIR_DOMAIN_DEF_FEATURE_NAME_SLASH, src/vmx/vmx.c: VIR_DOMAIN_DEF_FEATURE_NAME_SLASH |
</element> <optional> <element name="uuid"> @@ -1391,7 +1389,7 @@ <optional> <element name="backenddomain"> <attribute name="name"> - <ref name="domainName"/> + <ref name="objectNameWithSlash"/>
Added by:
conf: support backend domain name in disk and network devices
At least Xen supports backend drivers in another domain (aka "driver domain"). This patch introduces an XML config option for specifying the backend domain name for <disk> and <interface> devices. E.g.
<disk> <backenddomain name='diskvm'/> ... </disk> <interface type='bridge'> <backenddomain name='netvm'/> ... </interface>
I see no need to relax this to allow a slash.
This isn't relaxing it, but merely keeping it as it was, since 'domainName' rng type allows slash.
On the other hand, nothing actually uses 'backenddomain', so we can use the stricter version without any problem.
it is passed to libxl: src/libxl/libxl_conf.c: if (l_disk->domain_name) { src/libxl/libxl_conf.c: x_disk->backend_domname = g_strdup(l_disk->domain_name); src/libxl/libxl_conf.c: if (l_nic->domain_name) { src/libxl/libxl_conf.c: x_nic->backend_domname = g_strdup(l_nic->domain_name); Jano
</attribute> <empty/> </element>
There's plenty of existing documentation [1] which shows as example a name which contains a space and a dot ('client.admin secret') as ceph usage name. Use a more relaxed type in the RNG schema since the usage name is actually just a string used to look up the secret. [1]: https://docs.ceph.com/en/latest/rbd/libvirt/#configuring-the-vm https://documentation.suse.com/ses/6/html/ses-all/cha-ceph-libvirt.html#ceph... Libvirt docs were correct though: https://libvirt.org/formatsecret.html#CephUsageType Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1689168 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 2 +- docs/schemas/secret.rng | 8 ++--- .../disk-network-source-auth.args | 8 ++++- ...isk-network-source-auth.x86_64-2.12.0.args | 9 ++++++ ...isk-network-source-auth.x86_64-latest.args | 29 ++++++++++++++----- .../disk-network-source-auth.xml | 12 ++++++++ tests/qemuxml2argvtest.c | 3 +- .../disk-network-source-auth.xml | 13 +++++++++ tests/secretxml2xmlin/usage-ceph-space.xml | 7 +++++ tests/secretxml2xmltest.c | 1 + 10 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-ceph-space.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4fc6a7ee7a..701db7e7d2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6261,7 +6261,7 @@ <ref name="UUID"/> </attribute> <attribute name="usage"> - <ref name="genericName"/> + <ref name="objectName"/> </attribute> </choice> </element> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 1aafe03e61..c90e2eb81f 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -60,7 +60,7 @@ <value>ceph</value> </attribute> <element name="name"> - <ref name="genericName"/> + <ref name="objectName"/> </element> </define> @@ -69,7 +69,7 @@ <value>iscsi</value> </attribute> <element name="target"> - <ref name="genericName"/> + <ref name="objectName"/> </element> </define> @@ -78,7 +78,7 @@ <value>tls</value> </attribute> <element name="name"> - <ref name="genericName"/> + <ref name="objectName"/> </element> </define> @@ -87,7 +87,7 @@ <value>vtpm</value> </attribute> <element name="name"> - <ref name="genericName"/> + <ref name="objectName"/> </element> </define> diff --git a/tests/qemuxml2argvdata/disk-network-source-auth.args b/tests/qemuxml2argvdata/disk-network-source-auth.args index e68b81fde0..18d48b263f 100644 --- a/tests/qemuxml2argvdata/disk-network-source-auth.args +++ b/tests/qemuxml2argvdata/disk-network-source-auth.args @@ -35,4 +35,10 @@ key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ auth_supported=cephx\;none:mon_host=mon1.example.org\:6321\;mon2.example.org\:\ 6322\;mon3.example.org\:6322,format=raw,if=none,id=drive-virtio-disk1' \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ -id=virtio-disk1 +id=virtio-disk1 \ +-drive 'file=rbd:pool/image2:id=myname:\ +key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ +auth_supported=cephx\;none:mon_host=mon1.example.org\:6321\;mon2.example.org\:\ +6322\;mon3.example.org\:6322,format=raw,if=none,id=drive-virtio-disk2' \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\ +id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args index 279d5c73ec..0ccf3df106 100644 --- a/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args @@ -45,6 +45,15 @@ mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:\ id=drive-virtio-disk1' \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk1,\ id=virtio-disk1 \ +-object secret,id=virtio-disk2-auth-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive 'file=rbd:pool/image2:id=myname:auth_supported=cephx\;none:\ +mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:\ +6322,file.password-secret=virtio-disk2-auth-secret0,format=raw,if=none,\ +id=drive-virtio-disk2' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk2,\ +id=virtio-disk2 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-latest.args index 257ca1376c..879a52123c 100644 --- a/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-network-source-auth.x86_64-latest.args @@ -29,21 +29,34 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --object secret,id=libvirt-2-storage-auth-secret0,\ +-object secret,id=libvirt-3-storage-auth-secret0,\ data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -blockdev '{"driver":"iscsi","portal":"example.org:6000",\ "target":"iqn.1992-01.com.example:storage","lun":1,"transport":"tcp",\ -"user":"myname","password-secret":"libvirt-2-storage-auth-secret0",\ -"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +"user":"myname","password-secret":"libvirt-3-storage-auth-secret0",\ +"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw",\ +"file":"libvirt-3-storage"}' \ +-device virtio-blk-pci,bus=pci.0,addr=0x2,drive=libvirt-3-format,\ +id=virtio-disk0,bootindex=1 \ +-object secret,id=libvirt-2-storage-auth-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image",\ +"server":[{"host":"mon1.example.org","port":"6321"},{"host":"mon2.example.org",\ +"port":"6322"},{"host":"mon3.example.org","port":"6322"}],"user":"myname",\ +"auth-client-required":["cephx","none"],\ +"key-secret":"libvirt-2-storage-auth-secret0","node-name":"libvirt-2-storage",\ +"auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw",\ "file":"libvirt-2-storage"}' \ --device virtio-blk-pci,bus=pci.0,addr=0x2,drive=libvirt-2-format,\ -id=virtio-disk0,bootindex=1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=libvirt-2-format,\ +id=virtio-disk1 \ -object secret,id=libvirt-1-storage-auth-secret0,\ data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ --blockdev '{"driver":"rbd","pool":"pool","image":"image",\ +-blockdev '{"driver":"rbd","pool":"pool","image":"image2",\ "server":[{"host":"mon1.example.org","port":"6321"},{"host":"mon2.example.org",\ "port":"6322"},{"host":"mon3.example.org","port":"6322"}],"user":"myname",\ "auth-client-required":["cephx","none"],\ @@ -51,8 +64,8 @@ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ "auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ "file":"libvirt-1-storage"}' \ --device virtio-blk-pci,bus=pci.0,addr=0x3,drive=libvirt-1-format,\ -id=virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-1-format,\ +id=virtio-disk2 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-source-auth.xml b/tests/qemuxml2argvdata/disk-network-source-auth.xml index 7cc5c96ae7..0f8d29070f 100644 --- a/tests/qemuxml2argvdata/disk-network-source-auth.xml +++ b/tests/qemuxml2argvdata/disk-network-source-auth.xml @@ -36,6 +36,18 @@ </source> <target dev='vdb' bus='virtio'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image2'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + <auth username='myname'> + <secret type='ceph' usage='client.admin secret'/> + </auth> + </source> + <target dev='vdc' bus='virtio'/> + </disk> <controller type='usb' index='0'/> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d2712e0dce..b63ba29739 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -64,7 +64,8 @@ fakeSecretLookupByUsage(virConnectPtr conn, usageID); return NULL; } - } else if (STRNEQ(usageID, "mycluster_myname")) { + } else if (STRNEQ(usageID, "mycluster_myname") && + STRNEQ(usageID, "client.admin secret")) { virReportError(VIR_ERR_INTERNAL_ERROR, "test provided incorrect usage '%s'", usageID); return NULL; diff --git a/tests/qemuxml2xmloutdata/disk-network-source-auth.xml b/tests/qemuxml2xmloutdata/disk-network-source-auth.xml index b9f06448c1..d9c85c478d 100644 --- a/tests/qemuxml2xmloutdata/disk-network-source-auth.xml +++ b/tests/qemuxml2xmloutdata/disk-network-source-auth.xml @@ -38,6 +38,19 @@ <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image2'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + <auth username='myname'> + <secret type='ceph' usage='client.admin secret'/> + </auth> + </source> + <target dev='vdc' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> diff --git a/tests/secretxml2xmlin/usage-ceph-space.xml b/tests/secretxml2xmlin/usage-ceph-space.xml new file mode 100644 index 0000000000..557b12474d --- /dev/null +++ b/tests/secretxml2xmlin/usage-ceph-space.xml @@ -0,0 +1,7 @@ +<secret ephemeral='no' private='yes'> + <uuid>f52a81b2-424e-490c-823d-6bd4235bc573</uuid> + <description>Ceph secret with space and dot</description> + <usage type='ceph'> + <name>client.admin secret</name> + </usage> +</secret> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index 9eb3c460e7..74a262e1e8 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -74,6 +74,7 @@ mymain(void) DO_TEST("ephemeral-usage-volume"); DO_TEST("usage-volume"); DO_TEST("usage-ceph"); + DO_TEST("usage-ceph-space"); DO_TEST("usage-iscsi"); DO_TEST("usage-tls"); DO_TEST("usage-vtpm"); -- 2.29.2
On a Wednesday in 2021, Peter Krempa wrote:
There's plenty of existing documentation [1] which shows as example a name which contains a space and a dot ('client.admin secret') as ceph usage name.
Use a more relaxed type in the RNG schema since the usage name is actually just a string used to look up the secret.
[1]: https://docs.ceph.com/en/latest/rbd/libvirt/#configuring-the-vm https://documentation.suse.com/ses/6/html/ses-all/cha-ceph-libvirt.html#ceph... Libvirt docs were correct though: https://libvirt.org/formatsecret.html#CephUsageType
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1689168
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 2 +- docs/schemas/secret.rng | 8 ++--- .../disk-network-source-auth.args | 8 ++++- ...isk-network-source-auth.x86_64-2.12.0.args | 9 ++++++ ...isk-network-source-auth.x86_64-latest.args | 29 ++++++++++++++----- .../disk-network-source-auth.xml | 12 ++++++++ tests/qemuxml2argvtest.c | 3 +- .../disk-network-source-auth.xml | 13 +++++++++ tests/secretxml2xmlin/usage-ceph-space.xml | 7 +++++ tests/secretxml2xmltest.c | 1 + 10 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-ceph-space.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4fc6a7ee7a..701db7e7d2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6261,7 +6261,7 @@ <ref name="UUID"/> </attribute> <attribute name="usage"> - <ref name="genericName"/> + <ref name="objectName"/> </attribute> </choice> </element>
The auth info in storagepool.rng is not as restrictive: <define name="sourceinfoauthsecret"> <element name="secret"> <choice> <attribute name="uuid"> <text/> </attribute> <attribute name="usage"> <text/> </attribute> </choice> </element> </define> Jano
On a Wednesday in 2021, Peter Krempa wrote:
Patches 1-6 are mostly cleanups noticed while looking at the schema.
See patch 7/7 for explanation.
Peter Krempa (7): virSecretDefParseUsage: Use g_autofree for type_str secretXMLParseNode: Clean up freeing of memory virSecretLookupParseSecret: Use g_steal_pointer schema: domaincommon: Remove pointless 'choice' from 'inituser'/'initgroup' schema: Remove workaround for bug in libxml2 2.7.6 schema: Add define for object names schema: secret: Relax requirements for usage name
docs/schemas/basictypes.rng | 13 ++++++++ docs/schemas/domaincommon.rng | 33 +++++-------------- docs/schemas/secret.rng | 8 ++--- docs/schemas/storagecommon.rng | 8 ----- docs/schemas/storagepool.rng | 4 +-- src/conf/secret_conf.c | 20 ++++------- src/util/virsecret.c | 3 +- .../disk-network-source-auth.args | 8 ++++- ...isk-network-source-auth.x86_64-2.12.0.args | 9 +++++ ...isk-network-source-auth.x86_64-latest.args | 29 +++++++++++----- .../disk-network-source-auth.xml | 12 +++++++ tests/qemuxml2argvtest.c | 3 +- .../disk-network-source-auth.xml | 13 ++++++++ tests/secretxml2xmlin/usage-ceph-space.xml | 7 ++++ tests/secretxml2xmltest.c | 1 + 15 files changed, 107 insertions(+), 64 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-ceph-space.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa