[PATCH v4 0/4] qemu: Support rbd namespace attribute
Han Han
hhan at redhat.com
Mon Aug 31 02:21:10 UTC 2020
On Tue, Aug 25, 2020 at 7:32 PM Jason Dillaman <jdillama at redhat.com> wrote:
> On Tue, Aug 25, 2020 at 2:55 AM Han Han <hhan at redhat.com> wrote:
> >
> >
> >
> > On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman <jdillama at redhat.com>
> wrote:
> >>
> >> 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 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/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 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 at 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 at redhat.com
Phone: +861065339333
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200831/eb1044b9/attachment-0001.htm>
More information about the libvir-list
mailing list