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?
>
> >
> > >> 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.
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.
Thanks,
Ilya
