Re: [pbs-devel] [PATCH proxmox-backup] tape: fix 'eject-before-unload' api type

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 13/12/2023 um 11:11 schrieb Dominik Csapak:
> by converting the bool into an option, otherwise having the options not
> set at all will fail the unload while deserializing with
> 'eject-before-unload is not optional'
> 
> Also if we can automatically decide this in the future, we can now
> detect if the option was explicitely set or not.
> 
> Fixes: 66402cdc ("fix #4904: tape changer: add option to eject before unload")
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  pbs-api-types/src/tape/changer.rs | 17 ++++++++++-------
>  src/tape/changer/mod.rs           |  2 +-
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/pbs-api-types/src/tape/changer.rs b/pbs-api-types/src/tape/changer.rs
> index e3cf27c1..9e36b12e 100644
> --- a/pbs-api-types/src/tape/changer.rs
> +++ b/pbs-api-types/src/tape/changer.rs
> @@ -39,18 +39,21 @@ Import/Export, i.e. any media in those slots are considered to be
>  .format(&ApiStringFormat::PropertyString(&SLOT_ARRAY_SCHEMA))
>  .schema();
>  
> -fn is_false(b: &bool) -> bool {
> -    !b
> -}
> -
> -#[api]
> +#[api(
> +    properties: {
> +        "eject-before-unload": {
> +            optional: true,
> +            default: false,
> +        },
> +    },
> +)]
>  #[derive(Serialize, Deserialize)]
>  #[serde(rename_all = "kebab-case")]
>  /// Options for Changers
>  pub struct ChangerOptions {

semi-related to your change, but as I talked with Dietmar off-list about this and
thus checked it out a bit more closely:

As this is already on the changer section type it might make more sense to just have
the "eject-before-unload" option there directly though.

As having a property-string here seems a bit odd, we mostly use them if we want to
configure more things at once on a single (sub)-subject, like network for VMs, but
if you'd have this as "plain" option for the change section-type, it would already
affect the correct subject.

If we get many more such options in the future, we can always move them into a
property-string grouping for a next major release, but tbh. I would be slightly
surprised if it's more than a handful such new options in the next decade.
And even if, as long as they affect the changer, not a sub-subject, it can be
still fine to have any such new option also standalone on the top-level