[PATCH v8 0/3] qemu: Support rbd namespace

Diff from v7: - Squash a commit - Rebase to latest code v7: https://listman.redhat.com/archives/libvir-list/2020-November/msg00480.html Han Han (3): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace from source name qemu: Implement rbd namespace to the source name attribute docs/formatdomain.rst | 16 ++++++ src/conf/domain_conf.c | 47 +++++++++++++++-- src/conf/storage_source_conf.c | 2 + src/conf/storage_source_conf.h | 1 + src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 8 +++ .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 38 ++++++++++++++ .../disk-network-rbd-namespace.xml | 40 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 27 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml -- 2.31.1

The capability flag will be used for rbd namespace option. The rbd namespace is introduced since ceph Nautilus and qemu v5.0.0. Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + 16 files changed, 17 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 75dd01f06e..ea6c3648ea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -630,6 +630,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "compat-deprecated", "acpi-index", "input-linux", + "rbd.namespace", ); @@ -1571,6 +1572,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP }, { "object-add/arg-type/qom-type/^secret", QEMU_CAPS_OBJECT_QAPIFIED }, + { "blockdev-add/arg-type/+rbd/namespace", QEMU_CAPS_RBD_NAMESPACE }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 94f2fad05a..0d0dc52c1c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -610,6 +610,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_COMPAT_DEPRECATED, /* -compat deprecated-(input|output) is supported */ QEMU_CAPS_ACPI_INDEX, /* PCI device 'acpi-index' property */ QEMU_CAPS_INPUT_LINUX, /* -object input-linux */ + QEMU_CAPS_RBD_NAMESPACE, /* -blockdev '{"driver":"rbd",...,"namespace":str}' */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 331309bbe3..7a2214c3b9 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -202,6 +202,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index 444b7a222f..1b5c1f9025 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -210,6 +210,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml index 84a57d1e0d..8e239d4ec7 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -196,6 +196,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index a3d9338e86..5a1b333b6b 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -247,6 +247,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml index 3075172082..0fd9f1bd39 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml @@ -111,6 +111,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index 0dae0ad892..d232da4d6c 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -249,6 +249,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml index 7615bd9cb6..b08a64f0ff 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml @@ -206,6 +206,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml index ac5667c1a9..2c56c37f29 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml @@ -212,6 +212,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml index 192f9bc7d4..20051da150 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml @@ -198,6 +198,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml index db72107a0d..5c68251c1b 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml @@ -160,6 +160,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 7cfbebe323..ccbc6007d0 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -250,6 +250,7 @@ <flag name='blockdev-backup'/> <flag name='rotation-rate'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index 3c07e7159b..e4ea351af6 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -165,6 +165,7 @@ <flag name='compat-deprecated'/> <flag name='acpi-index'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 80ec2e77c1..a38d1f0b5e 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -255,6 +255,7 @@ <flag name='compat-deprecated'/> <flag name='acpi-index'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 4df3200219..5a38ba4fcd 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -254,6 +254,7 @@ <flag name='compat-deprecated'/> <flag name='acpi-index'/> <flag name='input-linux'/> + <flag name='rbd.namespace'/> <version>6000050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.31.1

Signed-off-by: Han Han <hhan@redhat.com> --- docs/formatdomain.rst | 16 ++++++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++--- src/conf/storage_source_conf.c | 2 ++ src/conf/storage_source_conf.h | 1 + 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index aa7bb8da14..cb3914d1bb 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2479,6 +2479,16 @@ paravirtualized driver is specified via the ``disk`` element. </source> <target dev='vdf' bus='virtio'/> </disk> + <disk type='network'> + <driver name="qemu" type="raw"/> + <source protocol="rbd" name="pool/namespace/image"> + <host name="hostname" port="7000"/> + <auth username='myuser'> + <secret type='ceph' usage='mypassid'/> + </auth> + </source> + <target dev="vdg" bus="virtio"/> + </disk> </devices> ... @@ -2570,6 +2580,12 @@ paravirtualized driver is specified via the ``disk`` element. the host by the ``nbd_tls`` and ``nbd_tls_x509_cert_dir`` in /etc/libvirt/qemu.conf. ('tls' :since:`Since 4.5.0` ) + For "rbd", the ``name`` attribute could be two formats: the format of + ``pool_name/image_name`` includes the rbd pool name and image name with + default rbd pool namespace; for the customized namespace, the format is + ``pool_name/namespace/image_name`` ( :since:`Since 6.9.0 and QEMU 5.0` ). + The pool name, namespace and image are separated by slash. + For protocols ``http`` and ``https`` an optional attribute ``query`` specifies the query string. ( :since:`Since 6.2.0` ) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d90041bf8..eac59dd00e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8197,6 +8197,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, { virStorageNetProtocol protocol; xmlNodePtr tmpnode; + char **tmp_split_paths; if (virXMLPropEnum(node, "protocol", virStorageNetProtocolTypeFromString, VIR_XML_PROP_REQUIRED, &protocol) < 0) @@ -8226,8 +8227,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, /* for historical reasons we store the volume and image name in one XML * element although it complicates thing when attempting to access them. */ if (src->path && - (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER || - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { char *tmp; if (!(tmp = strchr(src->path, '/')) || tmp == src->path) { @@ -8244,6 +8244,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, tmp[0] = '\0'; } + /* the name of rbd could be <pool>/<image> or <pool>/<namespace>/<image> */ + if (src->path && + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && + (tmp_split_paths = g_strsplit(src->path, "/", 3))) { + if (virStringIsEmpty(tmp_split_paths[0]) || + !tmp_split_paths[1] || + STREQ_NULLABLE(tmp_split_paths[2], "") || + (virStringIsEmpty(tmp_split_paths[1]) && + !tmp_split_paths[2])) { + virStringListFreeCount(tmp_split_paths, 3); + virReportError(VIR_ERR_XML_ERROR, + _("can't split path '%s' into pool name, pool " + "namespace, image name OR pool name, image name"), + src->path); + return -1; + } + + VIR_FREE(src->path); + src->volume = g_strdup(tmp_split_paths[0]); + /* the format of <pool>/<image> */ + if (!tmp_split_paths[2]) + src->path = g_strdup(tmp_split_paths[1]); + + if (tmp_split_paths[2]) { + /* the format of <pool>/<ns>/<image> */ + if (STRNEQ_NULLABLE(tmp_split_paths[1], "")) + src->ns = g_strdup(tmp_split_paths[1]); + + /* the format of <pool>//<image> */ + src->path = g_strdup(tmp_split_paths[2]); + } + + virStringListFreeCount(tmp_split_paths, 3); + } + /* snapshot currently works only for remote disks */ src->snapshot = virXPathString("string(./snapshot/@name)", ctxt); @@ -22948,8 +22983,12 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(attrBuf, " protocol='%s'", virStorageNetProtocolTypeToString(src->protocol)); - if (src->volume) - path = g_strdup_printf("%s/%s", src->volume, src->path); + if (src->volume) { + if (src->ns) + path = g_strdup_printf("%s/%s/%s", src->volume, src->ns, src->path); + else + path = g_strdup_printf("%s/%s", src->volume, src->path); + } virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path); virBufferEscapeString(attrBuf, " query='%s'", src->query); diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index 5ca06fa30a..7b765d0d81 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -829,6 +829,7 @@ virStorageSourceCopy(const virStorageSource *src, def->path = g_strdup(src->path); def->volume = g_strdup(src->volume); + def->ns = g_strdup(src->ns); def->relPath = g_strdup(src->relPath); def->backingStoreRaw = g_strdup(src->backingStoreRaw); def->backingStoreRawFormat = src->backingStoreRawFormat; @@ -1115,6 +1116,7 @@ virStorageSourceClear(virStorageSource *def) VIR_FREE(def->path); VIR_FREE(def->volume); + VIR_FREE(def->ns); VIR_FREE(def->snapshot); VIR_FREE(def->configFile); VIR_FREE(def->query); diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 389c7b56d1..dcc643ceba 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -275,6 +275,7 @@ struct _virStorageSource { char *snapshot; /* for storage systems supporting internal snapshots */ char *configFile; /* some storage systems use config file as part of the source definition */ + char *ns; /* for the storage systems supporting namespace */ char *query; /* query string for HTTP based protocols */ size_t nhosts; virStorageNetHostDef *hosts; -- 2.31.1

On Wed, May 26, 2021 at 21:35:11 +0800, Han Han wrote:
Signed-off-by: Han Han <hhan@redhat.com> --- docs/formatdomain.rst | 16 ++++++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++--- src/conf/storage_source_conf.c | 2 ++ src/conf/storage_source_conf.h | 1 + 4 files changed, 62 insertions(+), 4 deletions(-)
[...]
@@ -2570,6 +2580,12 @@ paravirtualized driver is specified via the ``disk`` element. the host by the ``nbd_tls`` and ``nbd_tls_x509_cert_dir`` in /etc/libvirt/qemu.conf. ('tls' :since:`Since 4.5.0` )
+ For "rbd", the ``name`` attribute could be two formats: the format of + ``pool_name/image_name`` includes the rbd pool name and image name with + default rbd pool namespace; for the customized namespace, the format is + ``pool_name/namespace/image_name`` ( :since:`Since 6.9.0 and QEMU 5.0` ). + The pool name, namespace and image are separated by slash.
7.5.0 at this point.
+ For protocols ``http`` and ``https`` an optional attribute ``query`` specifies the query string. ( :since:`Since 6.2.0` )
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d90041bf8..eac59dd00e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8197,6 +8197,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, { virStorageNetProtocol protocol; xmlNodePtr tmpnode; + char **tmp_split_paths;
if (virXMLPropEnum(node, "protocol", virStorageNetProtocolTypeFromString, VIR_XML_PROP_REQUIRED, &protocol) < 0) @@ -8226,8 +8227,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, /* for historical reasons we store the volume and image name in one XML * element although it complicates thing when attempting to access them. */ if (src->path && - (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER || - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { char *tmp; if (!(tmp = strchr(src->path, '/')) || tmp == src->path) { @@ -8244,6 +8244,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, tmp[0] = '\0'; }
+ /* the name of rbd could be <pool>/<image> or <pool>/<namespace>/<image> */ + if (src->path && + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
This algorithm is still needlessly obscure and very hard to follow.
+ (tmp_split_paths = g_strsplit(src->path, "/", 3))) { + if (virStringIsEmpty(tmp_split_paths[0]) || + !tmp_split_paths[1] || + STREQ_NULLABLE(tmp_split_paths[2], "") ||
Firstly on 3 lines you have 3 distinct empty sting checks.
+ (virStringIsEmpty(tmp_split_paths[1]) && + !tmp_split_paths[2])) { + virStringListFreeCount(tmp_split_paths, 3);
Then there's no need for explicit freeing just declare the helper variable as g_auto(GStrv). Additionally put it into the scope here by making the top level condition just: if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { ...
+ virReportError(VIR_ERR_XML_ERROR, + _("can't split path '%s' into pool name, pool " + "namespace, image name OR pool name, image name"),
error string should be a signle line. Also the error format is not using the expected format that you require. (it should contain the slashes).
+ src->path); + return -1; + } + + VIR_FREE(src->path); + src->volume = g_strdup(tmp_split_paths[0]); + /* the format of <pool>/<image> */ + if (!tmp_split_paths[2]) + src->path = g_strdup(tmp_split_paths[1]); + + if (tmp_split_paths[2]) {
this is the 'else' section of the above condition.
+ /* the format of <pool>/<ns>/<image> */ + if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
This check is duplicated.
+ src->ns = g_strdup(tmp_split_paths[1]); + + /* the format of <pool>//<image> */ + src->path = g_strdup(tmp_split_paths[2]);
Well, so the qemu parser for the stringified name actually supports escape sequences.
+ } + + virStringListFreeCount(tmp_split_paths, 3);
I suggest the following implementation which should have the same logic as what you've proposed: /* the name of rbd could be <pool>/<image> or <pool>/<namespace>/<image> */ if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { g_auto(GStrv) path_components = g_strsplit(src->path, "/", 3); size_t npath_components = g_strv_length(path_components); if (npath_components < 2 || virStringIsEmpty(path_components[0]) || virStringIsEmpty(path_components[npath_components - 1])) { virReportError(VIR_ERR_XML_ERROR, _("path '%s' doesn't consist of 'pool_name/image_name', or 'pool_name/namespace_name/image_name'"), src->path); return -1; } VIR_FREE(src->path); src->volume = g_strdup(path_components[0]); if (npath_components == 2 || virStringIsEmpty(path_components[1])) { src->path = g_strdup(path_components[1]); } else { src->ns = g_strdup(path_components[1]); src->path = g_strdup(path_components[2]); } }
+ } + /* snapshot currently works only for remote disks */ src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
@@ -22948,8 +22983,12 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(attrBuf, " protocol='%s'", virStorageNetProtocolTypeToString(src->protocol));
- if (src->volume) - path = g_strdup_printf("%s/%s", src->volume, src->path); + if (src->volume) { + if (src->ns) + path = g_strdup_printf("%s/%s/%s", src->volume, src->ns, src->path); + else + path = g_strdup_printf("%s/%s", src->volume, src->path); + }
virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path); virBufferEscapeString(attrBuf, " query='%s'", src->query);
The series is also missing the parsers for the backing store strings which we parse from the image metadata in cases when the backing chain is not fully defined in the XML. There's also a possibility that a similar treatment will be needed in the URI parser for the RBD case, but I'm not sure whether qemu accepts URIs as RBD source. Namely: virStorageSourceParseRBDColonString and TEST_BACKING_PARSE test cases in tests/virstoragetest.c and virStorageSourceParseBackingJSONRBD and also a test case in the above mentioned test file.

On Mon, Jun 07, 2021 at 16:34:39 +0200, Peter Krempa wrote:
On Wed, May 26, 2021 at 21:35:11 +0800, Han Han wrote:
Signed-off-by: Han Han <hhan@redhat.com> --- docs/formatdomain.rst | 16 ++++++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++--- src/conf/storage_source_conf.c | 2 ++ src/conf/storage_source_conf.h | 1 + 4 files changed, 62 insertions(+), 4 deletions(-)
[...]
@@ -2570,6 +2580,12 @@ paravirtualized driver is specified via the ``disk`` element. the host by the ``nbd_tls`` and ``nbd_tls_x509_cert_dir`` in /etc/libvirt/qemu.conf. ('tls' :since:`Since 4.5.0` )
+ For "rbd", the ``name`` attribute could be two formats: the format of + ``pool_name/image_name`` includes the rbd pool name and image name with + default rbd pool namespace; for the customized namespace, the format is + ``pool_name/namespace/image_name`` ( :since:`Since 6.9.0 and QEMU 5.0` ). + The pool name, namespace and image are separated by slash.
7.5.0 at this point.
+ For protocols ``http`` and ``https`` an optional attribute ``query`` specifies the query string. ( :since:`Since 6.2.0` )
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d90041bf8..eac59dd00e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8197,6 +8197,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, { virStorageNetProtocol protocol; xmlNodePtr tmpnode; + char **tmp_split_paths;
if (virXMLPropEnum(node, "protocol", virStorageNetProtocolTypeFromString, VIR_XML_PROP_REQUIRED, &protocol) < 0) @@ -8226,8 +8227,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, /* for historical reasons we store the volume and image name in one XML * element although it complicates thing when attempting to access them. */ if (src->path && - (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER || - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { char *tmp; if (!(tmp = strchr(src->path, '/')) || tmp == src->path) { @@ -8244,6 +8244,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, tmp[0] = '\0'; }
+ /* the name of rbd could be <pool>/<image> or <pool>/<namespace>/<image> */ + if (src->path && + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
This algorithm is still needlessly obscure and very hard to follow.
+ (tmp_split_paths = g_strsplit(src->path, "/", 3))) { + if (virStringIsEmpty(tmp_split_paths[0]) || + !tmp_split_paths[1] || + STREQ_NULLABLE(tmp_split_paths[2], "") ||
Firstly on 3 lines you have 3 distinct empty sting checks.
+ (virStringIsEmpty(tmp_split_paths[1]) && + !tmp_split_paths[2])) { + virStringListFreeCount(tmp_split_paths, 3);
Then there's no need for explicit freeing just declare the helper variable as g_auto(GStrv).
Additionally put it into the scope here by making the top level condition just: if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { ...
+ virReportError(VIR_ERR_XML_ERROR, + _("can't split path '%s' into pool name, pool " + "namespace, image name OR pool name, image name"),
error string should be a signle line. Also the error format is not using the expected format that you require. (it should contain the slashes).
+ src->path); + return -1; + } + + VIR_FREE(src->path); + src->volume = g_strdup(tmp_split_paths[0]); + /* the format of <pool>/<image> */ + if (!tmp_split_paths[2]) + src->path = g_strdup(tmp_split_paths[1]); + + if (tmp_split_paths[2]) {
this is the 'else' section of the above condition.
+ /* the format of <pool>/<ns>/<image> */ + if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
This check is duplicated.
+ src->ns = g_strdup(tmp_split_paths[1]); + + /* the format of <pool>//<image> */ + src->path = g_strdup(tmp_split_paths[2]);
Well, so the qemu parser for the stringified name actually supports escape sequences.
+ } + + virStringListFreeCount(tmp_split_paths, 3);
I suggest the following implementation which should have the same logic as what you've proposed:
/* the name of rbd could be <pool>/<image> or <pool>/<namespace>/<image> */ if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { g_auto(GStrv) path_components = g_strsplit(src->path, "/", 3); size_t npath_components = g_strv_length(path_components);
if (npath_components < 2 || virStringIsEmpty(path_components[0]) || virStringIsEmpty(path_components[npath_components - 1])) { virReportError(VIR_ERR_XML_ERROR, _("path '%s' doesn't consist of 'pool_name/image_name', or 'pool_name/namespace_name/image_name'"), src->path); return -1; }
VIR_FREE(src->path); src->volume = g_strdup(path_components[0]);
if (npath_components == 2 || virStringIsEmpty(path_components[1])) { src->path = g_strdup(path_components[1]); } else { src->ns = g_strdup(path_components[1]); src->path = g_strdup(path_components[2]); }
Actually the above hunk should be: if (npath_components == 2) { src->path = g_strdup(path_components[1]); } else if (virStringIsEmpty(path_components[1])) { src->path = g_strdup(path_components[2]); } else { src->ns = g_strdup(path_components[1]); src->path = g_strdup(path_components[2]); }
}
+ } + /* snapshot currently works only for remote disks */ src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
@@ -22948,8 +22983,12 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(attrBuf, " protocol='%s'", virStorageNetProtocolTypeToString(src->protocol));
- if (src->volume) - path = g_strdup_printf("%s/%s", src->volume, src->path); + if (src->volume) { + if (src->ns) + path = g_strdup_printf("%s/%s/%s", src->volume, src->ns, src->path); + else + path = g_strdup_printf("%s/%s", src->volume, src->path); + }
virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path); virBufferEscapeString(attrBuf, " query='%s'", src->query);
The series is also missing the parsers for the backing store strings which we parse from the image metadata in cases when the backing chain is not fully defined in the XML.
There's also a possibility that a similar treatment will be needed in the URI parser for the RBD case, but I'm not sure whether qemu accepts URIs as RBD source.
Namely:
virStorageSourceParseRBDColonString and TEST_BACKING_PARSE test cases in tests/virstoragetest.c
and
virStorageSourceParseBackingJSONRBD and also a test case in the above mentioned test file.

Since Nautilus ceph supports separate image namespaces within a pool for tenant isolation and QEMU adds it as a rbd blockdev options from 5.0.0. The source name with format "<pool>/<namespace>/<image>" could be used to access a rbd image with namespace. Add unit tests for this attribute. https://bugzilla.redhat.com/show_bug.cgi?id=1816909 Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_block.c | 1 + src/qemu/qemu_domain.c | 8 +++ ...k-network-rbd-namespace.x86_64-latest.args | 38 ++++++++++++++ .../disk-network-rbd-namespace.xml | 40 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 139 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6627d044cd..59230101fd 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -916,6 +916,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src, "s:pool", src->volume, "s:image", src->path, "S:snapshot", src->snapshot, + "S:namespace", src->ns, "S:conf", src->configFile, "A:server", &servers, "S:user", username, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7f57cae00e..b298f4b841 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4895,6 +4895,14 @@ qemuDomainValidateStorageSource(virStorageSource *src, } } + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && + src->ns && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_RBD_NAMESPACE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("rbd namespace is not supported by this QEMU")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args new file mode 100644 index 0000000000..f89773e3e1 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image","namespace":"ns","server":[{"host":"mon1.example.org","port":"6321"},{"host":"mon2.example.org","port":"6322"},{"host":"mon3.example.org","port":"6322"}],"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 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image","server":[{"host":"mon1.example.org","port":"6321"}],"node-name":"libvirt-1-storage","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 \ +-audiodev id=audio1,driver=none \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml b/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml new file mode 100644 index 0000000000..5389cbda2f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/ns/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool//image'> + <host name='mon1.example.org' port='6321'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f9ec81eb8e..ab805a2bd1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1392,6 +1392,7 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-network-gluster"); DO_TEST_CAPS_VER("disk-network-rbd", "2.12.0"); DO_TEST_CAPS_LATEST("disk-network-rbd"); + DO_TEST_CAPS_LATEST("disk-network-rbd-namespace"); DO_TEST_FAILURE("disk-network-rbd-no-colon", NONE); DO_TEST("disk-network-sheepdog", NONE); DO_TEST_CAPS_VER("disk-network-sheepdog", "2.12.0"); diff --git a/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml new file mode 100644 index 0000000000..710938e9ee --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/ns/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image'> + <host name='mon1.example.org' port='6321'/> + </source> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index dd039bd846..0f115df8ad 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -317,6 +317,7 @@ mymain(void) QEMU_CAPS_SCSI_BLOCK); DO_TEST("disk-network-gluster", NONE); DO_TEST("disk-network-rbd", NONE); + DO_TEST_CAPS_ARCH_LATEST("disk-network-rbd-namespace", "x86_64"); DO_TEST("disk-network-source-auth", NONE); DO_TEST("disk-network-sheepdog", NONE); DO_TEST("disk-network-vxhs", NONE); -- 2.31.1

On Wed, May 26, 2021 at 21:35:12 +0800, Han Han wrote:
Since Nautilus ceph supports separate image namespaces within a pool for tenant isolation and QEMU adds it as a rbd blockdev options from 5.0.0. The source name with format "<pool>/<namespace>/<image>" could be used to access a rbd image with namespace.
Add unit tests for this attribute.
https://bugzilla.redhat.com/show_bug.cgi?id=1816909
Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_block.c | 1 + src/qemu/qemu_domain.c | 8 +++ ...k-network-rbd-namespace.x86_64-latest.args | 38 ++++++++++++++ .../disk-network-rbd-namespace.xml | 40 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 139 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
diff --git a/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args new file mode 100644 index 0000000000..f89773e3e1 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image","namespace":"ns","server":[{"host":"mon1.example.org","port":"6321"},{"host":"mon2.example.org","port":"6322"},{"host":"mon3.example.org","port":"6322"}],"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 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image","server":[{"host":"mon1.example.org","port":"6321"}],"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
diff --git a/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml b/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml new file mode 100644 index 0000000000..5389cbda2f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml @@ -0,0 +1,40 @@
So, prior to patch 2/3 in this series ...
+ <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/ns/image'>
what would this refer to when used with qemu? this woudl become: "pool": "pool", "image": "ns/image"
+ <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool//image'>
and this: "pool": "pool", "image": "/image" Are those valid names in RBD? if yes, this will most probably cause a regression.
+ <host name='mon1.example.org' port='6321'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain>
diff --git a/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml new file mode 100644 index 0000000000..710938e9ee --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/ns/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image'>
This removed the extra '/', but it depends on the semantics of the previously existing code whether that's okay or not.
+ <host name='mon1.example.org' port='6321'/> + </source> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </disk>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index dd039bd846..0f115df8ad 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -317,6 +317,7 @@ mymain(void) QEMU_CAPS_SCSI_BLOCK); DO_TEST("disk-network-gluster", NONE); DO_TEST("disk-network-rbd", NONE); + DO_TEST_CAPS_ARCH_LATEST("disk-network-rbd-namespace", "x86_64");
DO_TEST_CAPS_LATEST is enough for x86_64
DO_TEST("disk-network-source-auth", NONE); DO_TEST("disk-network-sheepdog", NONE); DO_TEST("disk-network-vxhs", NONE); -- 2.31.1

PING On Wed, May 26, 2021 at 9:35 PM Han Han <hhan@redhat.com> wrote:
Diff from v7: - Squash a commit - Rebase to latest code
v7: https://listman.redhat.com/archives/libvir-list/2020-November/msg00480.html
Han Han (3): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace from source name qemu: Implement rbd namespace to the source name attribute
docs/formatdomain.rst | 16 ++++++ src/conf/domain_conf.c | 47 +++++++++++++++-- src/conf/storage_source_conf.c | 2 + src/conf/storage_source_conf.h | 1 + src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 8 +++ .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 38 ++++++++++++++ .../disk-network-rbd-namespace.xml | 40 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 50 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 27 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
-- 2.31.1
participants (2)
-
Han Han
-
Peter Krempa