Re: [PATCH v7 3/4] conf: Support to parse rbd namespace from source name
On Wed, Nov 11, 2020 at 12:37 AM Han Han wrote:
>
>
>
> On Wed, Nov 11, 2020 at 10:24 AM Jason Dillaman wrote:
>>
>> On Tue, Nov 10, 2020 at 8:43 PM Han Han wrote:
>> >
>> > Signed-off-by: Han Han
>> > ---
>> > docs/formatdomain.rst | 16 ++
>> > src/conf/domain_conf.c | 47 ++
>> > 2 files changed, 59 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> > index a6ab845f92..2b760e6a39 100644
>> > --- a/docs/formatdomain.rst
>> > +++ b/docs/formatdomain.rst
>> > @@ -2409,6 +2409,16 @@ paravirtualized driver is specified via the
>> > ``disk`` element.
>> >
>> >
>> >
>> > +
>> > +
>> > +
>> > +
>> > +
>> > +
>> > +
>> > +
>> > +
>> > +
>> >
>> > ...
>> >
>> > @@ -2500,6 +2510,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 d84d0327ad..a337051e30 100644
>> > --- a/src/conf/domain_conf.c
>> > +++ b/src/conf/domain_conf.c
>> > @@ -9554,6 +9554,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
>> > g_autofree char *tlsCfg = NULL;
>> > g_autofree char *sslverifystr = NULL;
>> > xmlNodePtr tmpnode;
>> > +char **tmp_split_paths;
>> >
>> > if (!(protocol = virXMLPropString(node, "protocol"))) {
>> > virReportError(VIR_ERR_XML_ERROR, "%s",
>> > @@ -9595,8 +9596,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) {
>> > @@ -9613,6 +9613,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
>> > tmp[0] = '\0';
>> > }
>> >
>> > +/* the name of rbd could be / or
>> > // */
>> > +if (src->path &&
>> > +src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
>> > +(tmp_split_paths = virStringSplit(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 / */
>> > +if (!tmp_split_paths[2])
>> > +src->path = g_strdup(tmp_split_paths[1]);
>> > +
>> > +if (tmp_split_paths[2]) {
>> > +/* the format of // */
>> > +if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
>> > +src->ns = g_strdup(tmp_split_paths[1]);
>> > +
>> > +/* the format of // */
>> > +src->path = g_strdup(tmp_split_paths[2]);
>>
>> Nit: I think QEMU and the rbd CLI will both treat this case as an
>> image named "/" instead of "".
>
> The results of qemu(qemu-kvm-5.1.0-5.fc33.x86_64) and rbd
> CLI(ceph-common-15.2.5-1.fc33.x86_64)
> are different here:
> ➜ ~ qemu-img info rbd:rbd//copy:conf=$HOME/.ceph/ceph.conf:key=XX
> image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "copy",
> "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd"}}
> file format: raw
> virtual size: 10 GiB (10737418240 bytes)
> disk size: unavailable
> cluster_size: 4194304
>
> ➜ ~ rbd info rbd//copy
> rbd: error opening image /copy: (2) No such fil
Re: [PATCH v7 3/4] conf: Support to parse rbd namespace from source name
On Wed, Nov 11, 2020 at 10:24 AM Jason Dillaman wrote:
> On Tue, Nov 10, 2020 at 8:43 PM Han Han wrote:
> >
> > Signed-off-by: Han Han
> > ---
> > docs/formatdomain.rst | 16 ++
> > src/conf/domain_conf.c | 47 ++
> > 2 files changed, 59 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index a6ab845f92..2b760e6a39 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -2409,6 +2409,16 @@ paravirtualized driver is specified via the
> ``disk`` element.
> >
> >
> >
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> >
> > ...
> >
> > @@ -2500,6 +2510,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 d84d0327ad..a337051e30 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -9554,6 +9554,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> > g_autofree char *tlsCfg = NULL;
> > g_autofree char *sslverifystr = NULL;
> > xmlNodePtr tmpnode;
> > +char **tmp_split_paths;
> >
> > if (!(protocol = virXMLPropString(node, "protocol"))) {
> > virReportError(VIR_ERR_XML_ERROR, "%s",
> > @@ -9595,8 +9596,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) {
> > @@ -9613,6 +9613,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> > tmp[0] = '\0';
> > }
> >
> > +/* the name of rbd could be / or
> // */
> > +if (src->path &&
> > +src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
> > +(tmp_split_paths = virStringSplit(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 / */
> > +if (!tmp_split_paths[2])
> > +src->path = g_strdup(tmp_split_paths[1]);
> > +
> > +if (tmp_split_paths[2]) {
> > +/* the format of // */
> > +if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
> > +src->ns = g_strdup(tmp_split_paths[1]);
> > +
> > +/* the format of // */
> > +src->path = g_strdup(tmp_split_paths[2]);
>
> Nit: I think QEMU and the rbd CLI will both treat this case as an
> image named "/" instead of "".
>
The results of qemu(qemu-kvm-5.1.0-5.fc33.x86_64) and rbd
CLI(ceph-common-15.2.5-1.fc33.x86_64)
are different here:
➜ ~ qemu-img info rbd:rbd//copy:conf=$HOME/.ceph/ceph.conf:key=XX
image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "copy",
"conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd"}}
file format: raw
virtual size: 10 GiB (10737418240 bytes)
disk size: unavailable
cluster_size: 4194304
➜ ~ rbd info rbd//copy
rbd: error opening image /copy: (2) No such file or directory
➜ ~ rbd info rbd/copy
rbd image 'copy':
size 10 GiB in 2560 objects
order 22 (4 MiB objects)
snapshot_count: 0
id: 4a4956b8b4567
block_name_prefix: rbd_data.4a4956b8b
Re: [PATCH v7 3/4] conf: Support to parse rbd namespace from source name
On Tue, Nov 10, 2020 at 8:43 PM Han Han wrote:
>
> Signed-off-by: Han Han
> ---
> docs/formatdomain.rst | 16 ++
> src/conf/domain_conf.c | 47 ++
> 2 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index a6ab845f92..2b760e6a39 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2409,6 +2409,16 @@ paravirtualized driver is specified via the ``disk``
> element.
>
>
>
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
>
> ...
>
> @@ -2500,6 +2510,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 d84d0327ad..a337051e30 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9554,6 +9554,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> g_autofree char *tlsCfg = NULL;
> g_autofree char *sslverifystr = NULL;
> xmlNodePtr tmpnode;
> +char **tmp_split_paths;
>
> if (!(protocol = virXMLPropString(node, "protocol"))) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -9595,8 +9596,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) {
> @@ -9613,6 +9613,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> tmp[0] = '\0';
> }
>
> +/* the name of rbd could be / or //
> */
> +if (src->path &&
> +src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
> +(tmp_split_paths = virStringSplit(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 / */
> +if (!tmp_split_paths[2])
> +src->path = g_strdup(tmp_split_paths[1]);
> +
> +if (tmp_split_paths[2]) {
> +/* the format of // */
> +if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
> +src->ns = g_strdup(tmp_split_paths[1]);
> +
> +/* the format of // */
> +src->path = g_strdup(tmp_split_paths[2]);
Nit: I think QEMU and the rbd CLI will both treat this case as an
image named "/" instead of "".
Otherwise,
Reviewed-by: Jason Dillaman
> +}
> +
> +virStringListFreeCount(tmp_split_paths, 3);
> +}
> +
> /* snapshot currently works only for remote disks */
> src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
>
> @@ -25351,8 +25386,12 @@ virDomainDiskSourceFormatNetwork(virBufferPtr
> 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);
> --
> 2.28.0
>
--
Jason
[PATCH v7 3/4] conf: Support to parse rbd namespace from source name
Signed-off-by: Han Han
---
docs/formatdomain.rst | 16 ++
src/conf/domain_conf.c | 47 ++
2 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index a6ab845f92..2b760e6a39 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2409,6 +2409,16 @@ paravirtualized driver is specified via the ``disk``
element.
+
+
+
+
+
+
+
+
+
+
...
@@ -2500,6 +2510,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 d84d0327ad..a337051e30 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9554,6 +9554,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
g_autofree char *tlsCfg = NULL;
g_autofree char *sslverifystr = NULL;
xmlNodePtr tmpnode;
+char **tmp_split_paths;
if (!(protocol = virXMLPropString(node, "protocol"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -9595,8 +9596,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) {
@@ -9613,6 +9613,41 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
tmp[0] = '\0';
}
+/* the name of rbd could be / or // */
+if (src->path &&
+src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
+(tmp_split_paths = virStringSplit(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 / */
+if (!tmp_split_paths[2])
+src->path = g_strdup(tmp_split_paths[1]);
+
+if (tmp_split_paths[2]) {
+/* the format of // */
+if (STRNEQ_NULLABLE(tmp_split_paths[1], ""))
+src->ns = g_strdup(tmp_split_paths[1]);
+
+/* the format of // */
+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);
@@ -25351,8 +25386,12 @@ virDomainDiskSourceFormatNetwork(virBufferPtr 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);
--
2.28.0
