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