[PATCH v4 0/4] qemu: Support rbd namespace attribute
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4 Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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.27.0
Add rbd namespace in aarch64 capability replies. 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 | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ 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.x86_64.xml | 1 + 7 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ff6ba8c9e9..7656d5f8cb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -597,6 +597,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "spapr-tpm-proxy", "numa.hmat", "blockdev-hostdev-scsi", + + /* 380 */ + "rbd.namespace", ); @@ -1525,6 +1528,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "migrate-set-parameters/arg-type/downtime-limit", QEMU_CAPS_MIGRATION_PARAM_DOWNTIME }, { "migrate-set-parameters/arg-type/xbzrle-cache-size", QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE }, { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, + { "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 5d08941538..ef7ea9aa20 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -578,6 +578,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_NUMA_HMAT, /* -numa hmat */ QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI, /* -blockdev used for (i)SCSI hostdevs */ + /* 380 */ + 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 928af2a01c..75622ca5fd 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -199,6 +199,7 @@ <flag name='migration-param.xbzrle-cache-size'/> <flag name='numa.hmat'/> <flag name='blockdev-hostdev-scsi'/> + <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 e8668a25a9..1780c8a5bc 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -208,6 +208,7 @@ <flag name='spapr-tpm-proxy'/> <flag name='numa.hmat'/> <flag name='blockdev-hostdev-scsi'/> + <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 85a8a46dac..d885bd9bb3 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -195,6 +195,7 @@ <flag name='migration-param.xbzrle-cache-size'/> <flag name='numa.hmat'/> <flag name='blockdev-hostdev-scsi'/> + <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 546b9b0422..e93483f0c6 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -243,6 +243,7 @@ <flag name='intel-iommu.aw-bits'/> <flag name='numa.hmat'/> <flag name='blockdev-hostdev-scsi'/> + <flag name='rbd.namespace'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index a9d82661e3..387a11a0c6 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -242,6 +242,7 @@ <flag name='intel-iommu.aw-bits'/> <flag name='numa.hmat'/> <flag name='blockdev-hostdev-scsi'/> + <flag name='rbd.namespace'/> <version>5000092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> -- 2.27.0
On Fri, Aug 07, 2020 at 05:49:12PM +0800, Han Han wrote:
Add rbd namespace in aarch64 capability replies.
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 | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ 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.x86_64.xml | 1 + 7 files changed, 12 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Han Han <hhan@redhat.com> --- docs/formatdomain.rst | 5 ++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 4 ++++ src/util/virstoragefile.h | 1 + 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 218f0c1718..431abc6f56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2256,7 +2256,7 @@ paravirtualized driver is specified via the ``disk`` element. </disk> <disk type='network'> <driver name="qemu" type="raw"/> - <source protocol="rbd" name="image_name2"> + <source protocol="rbd" name="image_name2" namespace="ns""> <host name="hostname" port="7000"/> <snapshot name="snapname"/> <config file="/path/to/file"/> @@ -2497,6 +2497,9 @@ paravirtualized driver is specified via the ``disk`` element. For protocols ``http`` and ``https`` an optional attribute ``query`` specifies the query string. ( :since:`Since 6.2.0` ) + For protocol ``rbd``, an optional attribute ``namespace`` specifies the + namespace of a rbd pool. ( :since:`Since 6.7.0 and QEMU 5.0.0` ) + For "iscsi" ( :since:`since 1.0.4` ), the ``name`` attribute may include a logical unit number, separated from the target's name by a slash (e.g., ``iqn.2013-07.com.example:iscsi-pool/1``). If not specified, the default diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0d0dcbc5ce..ee09da3c7c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1762,6 +1762,9 @@ <optional> <attribute name="name"/> </optional> + <optional> + <attribute name="namespace"/> + </optional> <zeroOrMore> <ref name="diskSourceNetworkHost"/> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef67efa1da..4b5e752c6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9601,6 +9601,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, /* config file currently only works with remote disks */ src->configFile = virXPathString("string(./config/@file)", ctxt); + src->ns = virXPathString("string(./@namespace)", ctxt); if (src->protocol == VIR_STORAGE_NET_PROTOCOL_HTTP || src->protocol == VIR_STORAGE_NET_PROTOCOL_HTTPS) @@ -25083,6 +25084,9 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, path = g_strdup_printf("%s/%s", src->volume, src->path); virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path); + if (src->ns) + virBufferEscapeString(attrBuf, " namespace='%s'", src->ns); + virBufferEscapeString(attrBuf, " query='%s'", src->query); if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT && diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index f73b3ee005..f027989462 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -284,6 +284,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; virStorageNetHostDefPtr hosts; -- 2.27.0
On Fri, Aug 07, 2020 at 05:49:13PM +0800, Han Han wrote:
Signed-off-by: Han Han <hhan@redhat.com> --- docs/formatdomain.rst | 5 ++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 4 ++++ src/util/virstoragefile.h | 1 + 4 files changed, 12 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Ceph Nautilus supports separate image namespaces within a pool for tenant isolation and QEMU added it as a rbd blockdev options from 5.0.0. This optional attribute is used to access a 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 | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 126 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 26c1b42428..d698bdec67 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -872,6 +872,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr 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 c440c79e1d..534975df75 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4536,6 +4536,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + 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..a744805d74 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args @@ -0,0 +1,41 @@ +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 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 214 \ +-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,nowait \ +-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-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=0x2,drive=libvirt-1-format,\ +id=virtio-disk0,bootindex=1 \ +-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..8b526c4a20 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-rbd-namespace.xml @@ -0,0 +1,33 @@ +<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/image' namespace='ns'> + <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> + <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 01839cb88c..c6c18c4335 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1174,6 +1174,7 @@ mymain(void) DO_TEST_CAPS_VER("disk-network-rbd", "2.5.0"); 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..2166696ca8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml @@ -0,0 +1,41 @@ +<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/image' namespace='ns'> + <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> + <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'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a07e2b7553..653e5caa86 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -344,6 +344,7 @@ mymain(void) DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI); DO_TEST("disk-network-gluster", NONE); DO_TEST("disk-network-rbd", NONE); + DO_TEST_CAPS_LATEST("disk-network-rbd-namespace"); DO_TEST("disk-network-source-auth", NONE); DO_TEST("disk-network-sheepdog", NONE); DO_TEST("disk-network-vxhs", NONE); -- 2.27.0
On Fri, Aug 07, 2020 at 05:49:14PM +0800, Han Han wrote:
Ceph Nautilus supports separate image namespaces within a pool for tenant isolation and QEMU added it as a rbd blockdev options from 5.0.0. This optional attribute is used to access a 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 | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 126 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
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Han Han <hhan@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 63ca689b43..57d8a40731 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,12 @@ v6.7.0 (unreleased) * **New features** + * qemu: Support rbd namespace attribute + + The namespaces is for the tenant isolation within a rbd pool, introduced + from Ceph Nautilus, supported since QEMU 5.0.0. In libvirt, using it by + the namespace attribute in the source element of rbd disk. + * **Improvements** * **Bug fixes** -- 2.27.0
On Fri, Aug 07, 2020 at 05:49:15PM +0800, Han Han wrote:
Signed-off-by: Han Han <hhan@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
ping On Fri, Aug 7, 2020 at 5:49 PM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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.27.0
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute. Reviewed-by: Jason Dillaman <dillaman@redhat.com> Thanks, Jason
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
Reviewed-by: Jason Dillaman <dillaman@redhat.com>
Thanks, Jason
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
That would create back compat issues too, so I can't see us splitting that. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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 100644https://www.spinics.net/linux/fedora/libvir/msg201067.html tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
-- 2.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
The problem is that having separate "<pool>/<image>" and "<pool-name>" attributes is semi-nonsensicle. At that point, you might as well just drop the "pool_namespace" attribute" and parse the image name just like the rest of Ceph/RBD code does as "[<pool-name>/[<pool_namespace>/]]<image-name>". Why should libvirt invent its own custom way to describe the image location? See this thread here [1] from back in April where you said you would split it out.
That would create back compat issues too, so I can't see us splitting that.
Yes, I understand the backwards compatibility concerns so you would need to continue to support "<pool>/<image>", but you could at least force the new format if a "<pool-namespace>" was specified.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[1] https://www.spinics.net/linux/fedora/libvir/msg201067.html -- Jason
On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com>
wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41
+++++++++++++++++++
.../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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 https://www.spinics.net/linux/fedora/libvir/msg201067.html tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
-- 2.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
The problem is that having separate "<pool>/<image>" and "<pool-name>"
<pool-name> ? I am confused here. Do you mean the pool-namespace?
attributes is semi-nonsensicle. At that point, you might as well just
From these precedents, I think it is no problem to use the 2nd pattern in
I think the only separated namespace is sensible here. Because there are 3 ways to express the rbd image path with namespace: 1. <pool>/<namespace>/<image> e.g: the rbd info comand and the qemu-img comand with legacy rbd path: ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/hhan/1 rbd image '1': size 100 MiB in 25 objects ... ➜ ~ qemu-img info rbd:rbd/hhan/1:conf=/home/hhan/.ceph/ceph.conf:id=admin:key=XXXXXXX image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ... Note that the missing namespace attribute in image json is caused by https://bugzilla.redhat.com/show_bug.cgi?id=1821528 2. only separated namespace: <pool>/><image> and namespace attribute or option e.g: the rbd command with namespace option ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/1 --namespace hhan rbd image '1': size 100 MiB in 25 objects order 22 (4 MiB objects) ... 3. separated pool, namespace, image e.g: qemu blockdev options of rbd: https://github.com/qemu/qemu/blob/30aa19446d82358a30eac3b556b4d6641e00b7c1/q... ➜ ~ qemu-img info 'json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin", "namespace":"hhan"}}' image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ... libvirt XML. I reject the 1st pattern because of compat issues. Suppose the 1st pattern is used, it will cause problems in the following case. Since rbd allows the image name contains '/' ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring -p rbd ls attach-new copy hhan/2 If I used 'hhan/2' image in libvirt XML at the previous libvirt and then I updated libvirt to the version support 1st pattern, the new libvirt parse the name='rbd/hhan/2' as pool 'rbd', namespace 'hhan', image '2',instead of the pool 'rbd', image 'hhan/2', default namespace. For the 3rd pattern, separating all the attributes, the xml will look like <source protocol="rbd" pool="POOL" image="IMG" namespace="NS"> However it cannot replace the old attribute name='<pool>/<image>' because of the compatibility. What about keeping the old attribute the adapting this new pattern? Well, it looks weird only rbd protocol adapts this new pattern because all the network protocols in libvirt use the old xml pattern <source protocol="PROTO" name="XX"> (seeing the examples in https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms) How about adapting this new pattern to all the network protocols? Considering the effort and the benifits of that, I think it is not worthwhile. drop the "pool_namespace" attribute" and parse the image name just
like the rest of Ceph/RBD code does as "[<pool-name>/[<pool_namespace>/]]<image-name>". Why should libvirt invent its own custom way to describe the image location?
See this thread here [1] from back in April where you said you would split it out.
Yes. The above is why I changed my mind and decided to use the only separated attribute namespace.
That would create back compat issues too, so I can't see us splitting that.
Yes, I understand the backwards compatibility concerns so you would need to continue to support "<pool>/<image>", but you could at least force the new format if a "<pool-namespace>" was specified.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[1] https://www.spinics.net/linux/fedora/libvir/msg201067.html
-- Jason
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
On Tue, Aug 25, 2020 at 2:55 AM Han Han <hhan@redhat.com> wrote:
On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote:
Diff from v3: - add the check for capability of rbd namespace - rename the item of rbd namespace in disk source struct - combine the commit of doc into the commit of patch - remove the code for -drive
gitlab branch: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4
Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace attribute qemu: Implement rbd namespace attribute news: qemu: Support rbd namespace
NEWS.rst | 6 +++ docs/formatdomain.rst | 5 ++- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 4 ++ src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.h | 1 + .../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 + .../caps_5.1.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 156 insertions(+), 1 deletion(-) 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 100644https://www.spinics.net/linux/fedora/libvir/msg201067.html tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
-- 2.27.0
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
The problem is that having separate "<pool>/<image>" and "<pool-name>"
<pool-name> ? I am confused here. Do you mean the pool-namespace?
Yes
attributes is semi-nonsensicle. At that point, you might as well just
I think the only separated namespace is sensible here. Because there are 3 ways to express the rbd image path with namespace:
Except "pool-namespace" is not a standalone property, it's tied to the pool that you are hiding in the "name". A "pool-namespace" of "ns1" in pool "pool1" is not the same as the namespace "ns1" in pool "pool2". Like I said, you seem to be developing your own unique RBD image-spec format.
1. <pool>/<namespace>/<image> e.g: the rbd info comand and the qemu-img comand with legacy rbd path: ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/hhan/1 rbd image '1': size 100 MiB in 25 objects
$ rbd namespace create rbd/ns1 $ rbd create --size 1G rbd/ns1/image1 $ rbd info rbd/ns1/image1 rbd image 'image1': size 1 GiB in 256 objects ... snip ... The rbd CLI will always treat that middle section as a namespace so for your example it would be pool rbd, namespace hhan, and image name 1.
... ➜ ~ qemu-img info rbd:rbd/hhan/1:conf=/home/hhan/.ceph/ceph.conf:id=admin:key=XXXXXXX image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ... Note that the missing namespace attribute in image json is caused by https://bugzilla.redhat.com/show_bug.cgi?id=1821528
2. only separated namespace: <pool>/><image> and namespace attribute or option e.g: the rbd command with namespace option ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/1 --namespace hhan rbd image '1': size 100 MiB in 25 objects order 22 (4 MiB objects) ...
3. separated pool, namespace, image e.g: qemu blockdev options of rbd: https://github.com/qemu/qemu/blob/30aa19446d82358a30eac3b556b4d6641e00b7c1/q... ➜ ~ qemu-img info 'json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin", "namespace":"hhan"}}' image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ...
From these precedents, I think it is no problem to use the 2nd pattern in libvirt XML. I reject the 1st pattern because of compat issues. Suppose the 1st pattern is used, it will cause problems in the following case. Since rbd allows the image name contains '/'
That's not true, RBD has not permitted "/" in pool nor image names for years -- and it's definitely not allowed when creating images in namespaces: $ rbd create --size 1G rbd/ns1/image1/ rbd: invalid spec 'rbd/ns1/image1/' $ rbd create --size 1G --pool rbd --namespace ns1 --image "image1/" rbd: invalid spec 'image1/' Is this a bug in "qemu-img" that allowed you to create "hhan/2" below? Does it allow you to create that image in a namespace or does your example no longer work? QEMU should also parse that middle section as the namespace [1].
➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring -p rbd ls attach-new copy hhan/2
If I used 'hhan/2' image in libvirt XML at the previous libvirt and then I updated libvirt to the version support 1st pattern, the new libvirt parse the name='rbd/hhan/2' as pool 'rbd', namespace 'hhan', image '2',instead of the pool 'rbd', image 'hhan/2', default namespace.
For the 3rd pattern, separating all the attributes, the xml will look like <source protocol="rbd" pool="POOL" image="IMG" namespace="NS">
However it cannot replace the old attribute name='<pool>/<image>' because of the compatibility. What about keeping the old attribute the adapting this new pattern? Well, it looks weird only rbd protocol adapts this new pattern because all the network protocols in libvirt use the old xml pattern <source protocol="PROTO" name="XX"> (seeing the examples in https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms) How about adapting this new pattern to all the network protocols? Considering the effort and the benifits of that, I think it is not worthwhile.
drop the "pool_namespace" attribute" and parse the image name just like the rest of Ceph/RBD code does as "[<pool-name>/[<pool_namespace>/]]<image-name>". Why should libvirt invent its own custom way to describe the image location?
See this thread here [1] from back in April where you said you would split it out.
Yes. The above is why I changed my mind and decided to use the only separated attribute namespace.
That would create back compat issues too, so I can't see us splitting that.
Yes, I understand the backwards compatibility concerns so you would need to continue to support "<pool>/<image>", but you could at least force the new format if a "<pool-namespace>" was specified.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[1] https://www.spinics.net/linux/fedora/libvir/msg201067.html
-- Jason
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat.
Email: hhan@redhat.com Phone: +861065339333
[1] https://github.com/qemu/qemu/blob/master/block/rbd.c#L150 -- Jason
On Tue, Aug 25, 2020 at 7:32 PM Jason Dillaman <jdillama@redhat.com> wrote:
On Tue, Aug 25, 2020 at 2:55 AM Han Han <hhan@redhat.com> wrote:
On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman <jdillama@redhat.com>
wrote:
On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <
berrange@redhat.com> wrote:
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <jdillama@redhat.com>
wrote:
On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan@redhat.com> wrote: > > Diff from v3: > - add the check for capability of rbd namespace > - rename the item of rbd namespace in disk source struct > - combine the commit of doc into the commit of patch > - remove the code for -drive > > gitlab branch: > https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4 > > Han Han (4): > qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE > conf: Support to parse rbd namespace attribute > qemu: Implement rbd namespace attribute > news: qemu: Support rbd namespace > > NEWS.rst | 6 +++ > docs/formatdomain.rst | 5 ++- > docs/schemas/domaincommon.rng | 3 ++ > src/conf/domain_conf.c | 4 ++ > src/qemu/qemu_block.c | 1 + > src/qemu/qemu_capabilities.c | 4 ++ > src/qemu/qemu_capabilities.h | 3 ++ > src/qemu/qemu_domain.c | 8 ++++ > src/util/virstoragefile.h | 1 + > .../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 + > .../caps_5.1.0.x86_64.xml | 1 + > ...k-network-rbd-namespace.x86_64-latest.args | 41
+++++++++++++++++++
> .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ > tests/qemuxml2argvtest.c | 1 + > ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 + > 19 files changed, 156 insertions(+), 1 deletion(-) > 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 https://www.spinics.net/linux/fedora/libvir/msg201067.html
tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml
> > -- > 2.27.0 >
Hopefully you still plan to add a "pool" attribute in a future series to help split-up the overloaded "pool/image" name attribute.
From my opinions, I think it's ok to keep "pool/image" in the name attribute if the meaning of this attribute is clarified in libvirt docs. Currently I have no plan to split the "pool/image".
The problem is that having separate "<pool>/<image>" and "<pool-name>"
<pool-name> ? I am confused here. Do you mean the pool-namespace?
Yes
attributes is semi-nonsensicle. At that point, you might as well just
I think the only separated namespace is sensible here. Because there are 3 ways to express the rbd image path with namespace:
Except "pool-namespace" is not a standalone property, it's tied to the pool that you are hiding in the "name". A "pool-namespace" of "ns1" in pool "pool1" is not the same as the namespace "ns1" in pool "pool2". Like I said, you seem to be developing your own unique RBD image-spec format.
1. <pool>/<namespace>/<image> e.g: the rbd info comand and the qemu-img comand with legacy rbd path: ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/hhan/1 rbd image '1': size 100 MiB in 25 objects
$ rbd namespace create rbd/ns1 $ rbd create --size 1G rbd/ns1/image1 $ rbd info rbd/ns1/image1 rbd image 'image1': size 1 GiB in 256 objects ... snip ...
The rbd CLI will always treat that middle section as a namespace so for your example it would be pool rbd, namespace hhan, and image name 1.
... ➜ ~ qemu-img info rbd:rbd/hhan/1:conf=/home/hhan/.ceph/ceph.conf:id=admin:key=XXXXXXX image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ... Note that the missing namespace attribute in image json is caused by https://bugzilla.redhat.com/show_bug.cgi?id=1821528
2. only separated namespace: <pool>/><image> and namespace attribute or option e.g: the rbd command with namespace option ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/1 --namespace hhan rbd image '1': size 100 MiB in 25 objects order 22 (4 MiB objects) ...
3. separated pool, namespace, image e.g: qemu blockdev options of rbd: https://github.com/qemu/qemu/blob/30aa19446d82358a30eac3b556b4d6641e00b7c1/q... ➜ ~ qemu-img info 'json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin", "namespace":"hhan"}}' image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw ...
From these precedents, I think it is no problem to use the 2nd pattern in libvirt XML. I reject the 1st pattern because of compat issues. Suppose the 1st pattern is used, it will cause problems in the following case. Since rbd allows the image name contains '/'
That's not true, RBD has not permitted "/" in pool nor image names for years -- and it's definitely not allowed when creating images in namespaces:
$ rbd create --size 1G rbd/ns1/image1/ rbd: invalid spec 'rbd/ns1/image1/' $ rbd create --size 1G --pool rbd --namespace ns1 --image "image1/" rbd: invalid spec 'image1/'
I am not sure how I created the image 'hhan/2' in the default namespace. It cannot be created by rbd command now. However it seems buggy on qemu-img(librbd1-12.2.7-9.el8.x86_64 qemu-img-5.1.0-3.module+el8.3.0+7708+740a1315.x86_64): ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring namespace ls NAME hhan test ➜ ~ qemu-img create 'rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ==' 1M Formatting 'rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ==', fmt=raw size=1048576 ➜ ~ qemu-img create 'rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ==' 1M Formatting 'rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ==', fmt=raw size=1048576 qemu-img: rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ==: error rbd create: File exists ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring namespace ls NAME hhan test ➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring -p rbd ls|grep new1 ➜ ~ qemu-img info rbd:rbd/aa/new1:conf=/root/.ceph/ceph.conf:id=admin:key=AQBm9fldc9zhMhAAeDDedFhu55XjV1YhdqDOkQ== image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "new1", "conf": "/root/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}} file format: raw virtual size: 1 MiB (1048576 bytes) disk size: unavailable cluster_size: 4194304 Well, the image "rbd/aa/new1" could be found by qemu-img but cannot be listed by rbd. Very confusing here. Since the '/' in image name is invalid, the format of name attribute "<pool>/[pool-ns]<image>" is more sensible. I will adapt that in the next patch series.
Is this a bug in "qemu-img" that allowed you to create "hhan/2" below? Does it allow you to create that image in a namespace or does your example no longer work? QEMU should also parse that middle section as the namespace [1].
➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring -p rbd ls attach-new copy hhan/2
If I used 'hhan/2' image in libvirt XML at the previous libvirt and then I updated libvirt to the version support 1st pattern, the new libvirt parse the name='rbd/hhan/2' as pool 'rbd', namespace 'hhan', image '2',instead of the pool 'rbd', image 'hhan/2', default namespace.
For the 3rd pattern, separating all the attributes, the xml will look like <source protocol="rbd" pool="POOL" image="IMG" namespace="NS">
However it cannot replace the old attribute name='<pool>/<image>' because of the compatibility. What about keeping the old attribute the adapting this new pattern? Well, it looks weird only rbd protocol adapts this new pattern because all the network protocols in libvirt use the old xml pattern <source protocol="PROTO" name="XX"> (seeing the examples in https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms) How about adapting this new pattern to all the network protocols? Considering the effort and the benifits of that, I think it is not worthwhile.
drop the "pool_namespace" attribute" and parse the image name just like the rest of Ceph/RBD code does as "[<pool-name>/[<pool_namespace>/]]<image-name>". Why should libvirt invent its own custom way to describe the image location?
See this thread here [1] from back in April where you said you would split it out.
Yes. The above is why I changed my mind and decided to use the only separated attribute namespace.
That would create back compat issues too, so I can't see us splitting that.
Yes, I understand the backwards compatibility concerns so you would need to continue to support "<pool>/<image>", but you could at least force the new format if a "<pool-namespace>" was specified.
Regards, Daniel -- |: https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[1] https://www.spinics.net/linux/fedora/libvir/msg201067.html
-- Jason
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat.
Email: hhan@redhat.com Phone: +861065339333
[1] https://github.com/qemu/qemu/blob/master/block/rbd.c#L150
-- Jason
-- Best regards, ----------------------------------- Han Han Senior Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
participants (3)
-
Daniel P. Berrangé -
Han Han -
Jason Dillaman