Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-30 Thread Han Han
On Tue, Aug 25, 2020 at 7:32 PM Jason Dillaman  wrote:

> On Tue, Aug 25, 2020 at 2:55 AM Han Han  wrote:
> >
> >
> >
> > On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman 
> wrote:
> >>
> >> On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <
> berra...@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 
> wrote:
> >> > >
> >> > > > On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 "/" and ""
> >
> >  ? 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. //
> > 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=XXX
> > 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/

Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-25 Thread Jason Dillaman
On Tue, Aug 25, 2020 at 2:55 AM Han Han  wrote:
>
>
>
> On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman  wrote:
>>
>> On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé  
>> 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  
>> > > wrote:
>> > >
>> > > > On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 "/" and ""
>
>  ? 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. //
> 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=XXX
> 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: /> 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 

Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-25 Thread Han Han
On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman  wrote:

> On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé 
> 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 
> wrote:
> > >
> > > > On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 "/" and ""
>
 ? I am confused here. Do you mean the pool-namespace?

> 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:
1. //
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=XXX
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: /> 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/qapi/block-core.json#L3585
➜  ~ 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 a

Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-24 Thread Jason Dillaman
On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé  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  wrote:
> >
> > > On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 "/" and ""
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
"[/[/]]". 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 "/", but you could at least
force the new format if a "" 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




Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-24 Thread Daniel P . Berrangé
On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:
> On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman  wrote:
> 
> > On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 :|



Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-24 Thread Han Han
On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman  wrote:

> On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 
>
>
> Thanks,
> Jason
>
>

-- 
Best regards,
---
Han Han
Senior Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333


Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-21 Thread Jason Dillaman
On Fri, Aug 7, 2020 at 5:50 AM Han Han  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 


Thanks,
Jason



Re: [PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-17 Thread Han Han
ping

On Fri, Aug 7, 2020 at 5:49 PM Han Han  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: h...@redhat.com
Phone: +861065339333


[PATCH v4 0/4] qemu: Support rbd namespace attribute

2020-08-07 Thread Han Han
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