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

[Jason Dillaman]

On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <berrange at 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 at redhat.com> wrote:
> >
> > > On Fri, Aug 7, 2020 at 5:50 AM Han Han <hhan at 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