Am 19.06.25 um 23:20 schrieb Ilya Dryomov: > On Thu, Jun 19, 2025 at 8:38 PM Ilya Dryomov <idryo...@gmail.com> wrote: >> >> On Mon, Jun 16, 2025 at 2:38 PM Fiona Ebner <f.eb...@proxmox.com> wrote: >>> >>> Am 16.06.25 um 12:28 schrieb Ilya Dryomov: >>>> On Mon, Jun 16, 2025 at 11:52 AM Daniel P. Berrangé <berra...@redhat.com> >>>> wrote: >>>>> On Mon, Jun 16, 2025 at 11:25:54AM +0200, Ilya Dryomov wrote: >>>>>> On Thu, May 15, 2025 at 1:29 PM Fiona Ebner <f.eb...@proxmox.com> wrote: >>>>>>> ## >>>>>>> # @BlockdevOptionsRbd: >>>>>>> # >>>>>>> @@ -4327,6 +4360,9 @@ >>>>>>> # authentication. This maps to Ceph configuration option "key". >>>>>>> # (Since 3.0) >>>>>>> # >>>>>>> +# @key-value-pairs: Key-value pairs for additional Ceph configuraton. >>>>>>> +# (Since 10.1) >>>>>>> +# >>>>>>> # @server: Monitor host address and port. This maps to the "mon_host" >>>>>>> # Ceph option. >>>>>>> # >>>>>>> @@ -4342,6 +4378,7 @@ >>>>>>> '*user': 'str', >>>>>>> '*auth-client-required': ['RbdAuthMode'], >>>>>>> '*key-secret': 'str', >>>>>>> + '*key-value-pairs' : 'RbdKeyValuePairs', >>>>>> >>>>>> To side-step all of the above, have you considered implementing >>>>>> a straightforward passthrough to Ceph instead? Something like >>>>>> >>>>>> '*key-value-pairs': ['RbdKeyValuePair'] >>>>>> >>>>>> where RbdKeyValuePair is just a pair arbitrary strings (and >>>>>> key-value-pairs is thus an optional list of those). rados_conf_set() >>>>>> would be called just the same but the user would be able to override >>>>>> any Ceph option they wish, not just a few that we thought of here. >>>>> >>>>> Passing through arbitrary key/value pairs as strings is essentially >>>>> abdicating our design responsibility in QAPI. enums would no longer >>>>> be introspectable. Integers / booleans would require abnormal formatting >>>>> by clients. API stability / deprecation promises can no longer be made. >>>>> and more besides. >>> >>> Yes, and I also was under the impression that there is no desire to >>> re-introduce arbitrary key-value pairs with QMP/blockdev options. >> >> Hi Fiona, >> >> What do you mean by re-introduce?
Sorry, should've stated this better. It is possible to specify arbitrary key-value pairs with '-drive', and by "re-introduce" I mean allowing the same for '-blockdev'. >>>>> Given that limitation, if we did go the string pairs route, I would >>>>> expect it to be marked as "unstable" in the QAPI schema, so apps have >>>>> a suitable warning NOT to rely on this. >>>> >>>> This sounds sensible to me. We can continue exposing the most common >>>> Ceph options through a proper QAPI schema but add key-value-pairs as an >>>> alternative low-level route for those who want to avoid dealing with >>>> physical configuration files. >>> >>> As written in the commit message, the cache option should not apply to >>> all volumes, so using configuration files is rather impractical there. >>> >>> I'd prefer defining the cache option(s) explicitly, and have people add >>> additional key-value pairs they require explicitly going forward. But if >>> you really don't want me to, I can still go with the unstable, arbitrary >>> strings approach instead. >> >> The RBD cache policy option would definitely count as one of the most >> common, so I don't have an objection to it being added in an explicit >> form. I'm also fine with the "disabled" enum value that you expressed >> a preference for in another email. > > The QEMU block layer-wide "cache" option is kind of in the way though: > if it's set to "off"/"none" or the more specific "cache.direct" option > is set to "on", we disable the RBD cache. So there is an existing way > to control that, but it's at another level and QEMU cache modes aren't > distinguished (i.e. there is no mapping to RBD which means that one can > have "cache=writeback" set in QEMU but still get e.g. "writethrough" > RBD cache policy come from the ceph.conf file). An extra enum value > for disabling the RBD cache might muddy the waters further. Even with "cache=writeback" in QEMU, the RBD cache policy will default to "writearound". And we do need "writeback" on the RBD side too: https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=738dc81cbae03ff03592c30a82c7e4d0d39a54ba;hp=e43b19109e57e6e7654c4fa6e2ed14a39b4a6fe2 > Another thing that comes to mind is that if you need to control the > cache policy (or any other RBD option) on a per-image basis as opposed > to per-user basis, you can employ image-level configuration overrides > on the RBD side -- see "rbd config image get/set/ls/rm" commands. Oh, that sounds like a good alternative :) Best Regards, Fiona
