<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 24, 2020 at 11:09 PM Jason Dillaman <<a href="mailto:jdillama@redhat.com">jdillama@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Aug 24, 2020 at 10:52 AM Daniel P. Berrangé <<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>> wrote:<br> ><br> > On Mon, Aug 24, 2020 at 10:19:59PM +0800, Han Han wrote:<br> > > On Fri, Aug 21, 2020 at 8:01 PM Jason Dillaman <<a href="mailto:jdillama@redhat.com" target="_blank">jdillama@redhat.com</a>> wrote:<br> > ><br> > > > On Fri, Aug 7, 2020 at 5:50 AM Han Han <<a href="mailto:hhan@redhat.com" target="_blank">hhan@redhat.com</a>> wrote:<br> > > > ><br> > > > > Diff from v3:<br> > > > > - add the check for capability of rbd namespace<br> > > > > - rename the item of rbd namespace in disk source struct<br> > > > > - combine the commit of doc into the commit of patch<br> > > > > - remove the code for -drive<br> > > > ><br> > > > > gitlab branch:<br> > > > > <a href="https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4" rel="noreferrer" target="_blank">https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v4</a><br> > > > ><br> > > > > Han Han (4):<br> > > > > qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE<br> > > > > conf: Support to parse rbd namespace attribute<br> > > > > qemu: Implement rbd namespace attribute<br> > > > > news: qemu: Support rbd namespace<br> > > > ><br> > > > > NEWS.rst | 6 +++<br> > > > > docs/formatdomain.rst | 5 ++-<br> > > > > docs/schemas/domaincommon.rng | 3 ++<br> > > > > src/conf/domain_conf.c | 4 ++<br> > > > > src/qemu/qemu_block.c | 1 +<br> > > > > src/qemu/qemu_capabilities.c | 4 ++<br> > > > > src/qemu/qemu_capabilities.h | 3 ++<br> > > > > src/qemu/qemu_domain.c | 8 ++++<br> > > > > src/util/virstoragefile.h | 1 +<br> > > > > .../caps_5.0.0.aarch64.xml | 1 +<br> > > > > .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +<br> > > > > .../caps_5.0.0.riscv64.xml | 1 +<br> > > > > .../caps_5.0.0.x86_64.xml | 1 +<br> > > > > .../caps_5.1.0.x86_64.xml | 1 +<br> > > > > ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++<br> > > > > .../disk-network-rbd-namespace.xml | 33 +++++++++++++++<br> > > > > tests/qemuxml2argvtest.c | 1 +<br> > > > > ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++<br> > > > > tests/qemuxml2xmltest.c | 1 +<br> > > > > 19 files changed, 156 insertions(+), 1 deletion(-)<br> > > > > create mode 100644<br> > > > tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args<br> > > > > create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml<br> > > > > create mode 100644<a href="https://www.spinics.net/linux/fedora/libvir/msg201067.html" rel="noreferrer" target="_blank">https://www.spinics.net/linux/fedora/libvir/msg201067.html</a><br> > > > tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml<br> > > > ><br> > > > > --<br> > > > > 2.27.0<br> > > > ><br> > > ><br> > > > Hopefully you still plan to add a "pool" attribute in a future series<br> > > > to help split-up the overloaded "pool/image" name attribute.<br> > > ><br> > > >From my opinions, I think it's ok to keep "pool/image" in the name<br> > > attribute if the meaning of this attribute<br> > > is clarified in libvirt docs.<br> > > Currently I have no plan to split the "pool/image". <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> The problem is that having separate "<pool>/<image>" and "<pool-name>"<br></blockquote><div><pool-name> ? I am confused here. Do you mean the pool-namespace?<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> attributes is semi-nonsensicle. At that point, you might as well just<br></blockquote><div>I think the only separated namespace is sensible here. Because there are 3 ways</div><div>to express the rbd image path with namespace:</div><div>1. <pool>/<namespace>/<image></div><div>e.g: the rbd info comand and the qemu-img comand with legacy rbd path:</div>➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/hhan/1 <br>rbd image '1':<br> size 100 MiB in 25 objects<br> ...</div><div class="gmail_quote">➜ ~ qemu-img info rbd:rbd/hhan/1:conf=/home/hhan/.ceph/ceph.conf:id=admin:key=XXXXXXX<br>image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}}<br>file format: raw<br></div><div class="gmail_quote">...</div><div class="gmail_quote">Note that the missing namespace attribute in image json is caused by <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1821528">https://bugzilla.redhat.com/show_bug.cgi?id=1821528</a></div><div class="gmail_quote"><br></div><div class="gmail_quote">2. only separated namespace: <pool>/><image> and namespace attribute or option</div><div class="gmail_quote">e.g: the rbd command with namespace option</div><div class="gmail_quote">➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring info rbd/1 --namespace hhan<br>rbd image '1':<br> size 100 MiB in 25 objects<br> order 22 (4 MiB objects)</div><div class="gmail_quote">...</div><div class="gmail_quote"><br></div><div class="gmail_quote">3. separated pool, namespace, image</div><div class="gmail_quote">e.g: qemu blockdev options of rbd: <a href="https://github.com/qemu/qemu/blob/30aa19446d82358a30eac3b556b4d6641e00b7c1/qapi/block-core.json#L3585">https://github.com/qemu/qemu/blob/30aa19446d82358a30eac3b556b4d6641e00b7c1/qapi/block-core.json#L3585</a></div><div class="gmail_quote">➜ ~ qemu-img info 'json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin", "namespace":"hhan"}}'<br>image: json:{"driver": "raw", "file": {"pool": "rbd", "image": "1", "conf": "/home/hhan/.ceph/ceph.conf", "driver": "rbd", "user": "admin"}}<br>file format: raw</div><div class="gmail_quote">...</div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><div>From these precedents, I think it is no problem to use the 2nd pattern in libvirt XML.</div><div>I reject the 1st pattern because of compat issues. </div><div>Suppose the 1st pattern is used, it will cause problems in the following case.</div><div>Since rbd allows the image name contains '/'</div><div>➜ ~ rbd -c ~/.ceph/ceph.conf -k ~/.ceph/ceph.client.admin.keyring -p rbd ls <br>attach-new<br>copy<br>hhan/2</div><div><br></div><div>If I used 'hhan/2' image in libvirt XML at the previous libvirt and then I updated libvirt to the version support 1st pattern,</div><div>the new libvirt parse the name='rbd/hhan/2' as pool 'rbd', namespace 'hhan', image '2',instead of the pool 'rbd', image</div><div>'hhan/2', default namespace.</div><div><br></div><div><br></div><div>For the 3rd pattern, separating all the attributes, the xml will look like</div><div><source protocol="rbd" pool="POOL" image="IMG" namespace="NS"></div><div><br></div><div>However it cannot replace the old attribute name='<pool>/<image>' because of the compatibility.</div><div>What about keeping the old attribute the adapting this new pattern?</div><div>Well, it looks <span><span>weird only rbd protocol adapts this new pattern because all the network protocols in libvirt</span></span></div><div><span><span>use the old xml pattern <source protocol="PROTO" name="XX"> (seeing the examples in <a href="https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms">https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms</a>)<br></span></span></div><div><span><span>How about adapting this new pattern to all the network protocols?</span></span></div><div><span><span>Considering the effort and the benifits of that, I think it is not worthwhile.<br></span></span></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> drop the "pool_namespace" attribute" and parse the image name just<br> like the rest of Ceph/RBD code does as<br> "[<pool-name>/[<pool_namespace>/]]<image-name>". Why should libvirt<br> invent its own custom way to describe the image location?<br> <br> See this thread here [1] from back in April where you said you would<br> split it out.<br> <br></blockquote><div>Yes.</div><div>The above is why I changed my mind and decided to use the only separated attribute namespace.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > That would create back compat issues too, so I can't see us splitting<br> > that.<br> <br> Yes, I understand the backwards compatibility concerns so you would<br> need to continue to support "<pool>/<image>", but you could at least<br> force the new format if a "<pool-namespace>" was specified.<br> <br> <br> <br> > Regards,<br> > Daniel<br> > --<br> > |: <a href="https://berrange.com" rel="noreferrer" target="_blank">https://berrange.com</a> -o- <a href="https://www.flickr.com/photos/dberrange" rel="noreferrer" target="_blank">https://www.flickr.com/photos/dberrange</a> :|<br> > |: <a href="https://libvirt.org" rel="noreferrer" target="_blank">https://libvirt.org</a> -o- <a href="https://fstop138.berrange.com" rel="noreferrer" target="_blank">https://fstop138.berrange.com</a> :|<br> > |: <a href="https://entangle-photo.org" rel="noreferrer" target="_blank">https://entangle-photo.org</a> -o- <a href="https://www.instagram.com/dberrange" rel="noreferrer" target="_blank">https://www.instagram.com/dberrange</a> :|<br> ><br> <br> [1] <a href="https://www.spinics.net/linux/fedora/libvir/msg201067.html" rel="noreferrer" target="_blank">https://www.spinics.net/linux/fedora/libvir/msg201067.html</a><br> <br> --<br> Jason<br> <br> </blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Best regards,</div><div dir="ltr">-----------------------------------<br></div><div dir="ltr">Han Han<br>Senior Quality Engineer<br>Redhat.<br><br>Email: <a href="mailto:hhan@redhat.com" target="_blank">hhan@redhat.com</a><br>Phone: +861065339333<br></div></div></div></div></div></div></div>