Re: [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Hannes Laimer <h.laimer@proxmox.com> hat am 25.11.2024 15:48 CET geschrieben:
> 
>  
> On 11/25/24 14:44, Fabian Grünbichler wrote:
> > On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> >> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> >> ---
> >> * new in v14, and not removable datastore specific
> >>
> >>   src/api2/config/datastore.rs | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> >> index 420f8ddd0..75e1a1a56 100644
> >> --- a/src/api2/config/datastore.rs
> >> +++ b/src/api2/config/datastore.rs
> >> @@ -81,6 +81,20 @@ pub(crate) fn do_create_datastore(
> >>           bail!("cannot create datastore in root path");
> >>       }
> >>   
> >> +    for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
> >> +        if store.backing_device != datastore.backing_device {
> >> +            continue;
> >> +        }
> > 
> > this is not needed, if you compare absolute_paths below?
> > 
> 
> absolute path is DS_MNT_DIR/{name}, it does not contain the on-device 
> path, we need this

fair enough, but the current form is still broken across the board..

we then actually need two checks here:
- absolute paths not overlapping for removable and regular datastores
- on device paths not overlapping within a device for removable datastores

> 
> >> +        if store.path.starts_with(&datastore.path) || datastore.path.starts_with(&store.path) {
> > 
> > this is broken, as `path` is a String here, and not a Path, so
> > `starts_with` doesn't properly match on path components, but on
> > arbitrary substrings..
> > 
> > i.e., I can't configure two removable datastores, one using "removable"
> > and one using "removable2" as path atm..

and this still needs to be addressed..

> > 
> >> +            param_bail!(
> >> +                "path",
> >> +                "nested datastores not allowed: '{}' already in '{}'",
> >> +                store.name,
> >> +                store.path
> >> +            );
> >> +        }
> >> +    }
> >> +
> >>       let need_unmount = datastore.backing_device.is_some();
> >>       if need_unmount {
> >>           do_mount_device(datastore.clone())?;
> >> -- 
> >> 2.39.5
> >>
> >>
> >>
> >> _______________________________________________
> >> pbs-devel mailing list
> >> pbs-devel@lists.proxmox.com
> >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> >>
> >>
> >>
> > 
> > 
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> > 
> > 
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel