Re: [PATCH proxmox-backup v3 09/10] api: config: unlocked s3 bucket access check for datastore creation

[Robert Obkircher <r.obkircher@proxmox.com>]

> Instead of holding the lock for the whole datastore creation, use the
> maintenance-mode `create`, protected by the maintenance-mode lock.
> It is inserted and written to the config when setting the maintenance
> mode before even starting any datastore related operation. Cleanup
> from config if creation failed must be performed manually.
> 
> This has the advantage that a potentially long running creation does
> not block any concurrent access to the datastore config, thereby
> blocking also any unrelated datastore.
> 
> The config has to be re-parsed a second time for this, but given that
> this unblocks the config and this is no performance critical path
> that tradeoff is better than its alternative. By refactoring the
> helpers to set and unset the maintenance mode, they can be reused for
> all call sites.
> 
> Rely on re-acquisition of the lock to also succeed if other operations
> are being performed, due to the 10s timeout.
> 
> Ordering of locking cannot always be preserved, but deadlocks are
> excluded by timeouts.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>[..]
> @@ -253,8 +253,27 @@ pub fn create_datastore_disk(
>                      bail!("datastore '{}' already exists.", datastore.name);
>                  }
>  
> +                let maintenance_mode_lock =
> +                    pbs_datastore::maintenance_mode_lock(&datastore.name, &lock)?;
> +                crate::api2::helpers::set_maintenance_type(
> +                    lock,
> +                    &maintenance_mode_lock,
> +                    &mut datastore,
> +                    MaintenanceType::Create,
> +                )?;
> +
Isn't this too early to drop the config lock? 

For example the `datastore.ensure_not_nested(&existing_stores)` inside
do_create_datastore could now potentially miss a newly created
datastore.

-- 
Robert Obkircher <r.obkircher@proxmox.com>