Re: [pbs-devel] [PATCH proxmox-backup v12 00/26] add removable datastores

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

41 files changed, 1756 insertions(+), 733 deletions(-)

26 patches

3(!) of them have a commit message, all of them very terse and not on
the patches that implement the meat of this series.. no notes on patches
either calling out history of the patches themselves.

NACKed in its current form on that alone, you should know better!

On September 4, 2024 4:11 pm, Hannes Laimer wrote:
> These patches add support for removable datastores. All removable
> datastores have a backing-device(a UUID) associated with them. Removable
> datastores work like normal ones, just that they can be unplugged. It is
> possible to create a removable datastore, sync backups onto it, unplug
> it and use it on a different PBS.
> 
> The datastore path specified is relative to the root of the used device.
> Removable datastores are bind mounted to /mnt/datastore/<NAME>.
> If we decide to allow multiple datastores on the same device in the future
> they can be mounted/unmounted individually.

see comment on patch #7 regarding this..

and other nits and comments on individual patches as well..

> When a removable datastore is deleted and 'destroy-data' is set, the
> device has to be mounted. If 'destroy-data' is not set the datastore
> can be deleted even if the device is not present. Removable datastores
> are automatically mounted when plugged in. 
> 
> v12: thanks @Wolfgang
>  * use bind mounts, so now
>     <DEVICE>/path/to/ds is mounted to /mnt/datastore/<NAME>
>     this is a bit cleaner and allows for multiple datastores
>     on a single device to be mounted individually, if we
>     want to allow that in the future
>  * small code improvements
> 
> 
> v11:
>  * rebase onto master
> 
> v10: thanks @Gabriel and @Wolfgang
>  * make is_datastore_available more robust
>  * fix a lot of wording
>  * drop format on uuid_mount command for UUID
>  * only gather_disk_stats if datastore is available
>  * overall code improvements
>  * ui: include model in partition selector
>  * rebased onto master
> 
> v9:
>  * change mount point to `/mnt/datastore/<name>`
>  * update "Directory" list UI
>  * add `absolute_path()` from Dietmar's RFC
>  * update docs
> 
> v8:
>  * still depends on [1]
>  * paths for removable datastores are now relative to 
>     `/mnt/removable_datastore/<UUID>`
>  * add support for creation of removable datastore through the 
>     "create directory" endpoint (last 3 patches)
>  * update datastore creation UI
>  * update docs
> 
> v7:
>  * depends on [1]
>  * improve logging when waiting for tasks
>  * drop `update-datatore-cache` refactoring
>  * fix some commit messages
> 
> [1] https://lists.proxmox.com/pipermail/pbs-devel/2024-April/008739.html
> 
> v6:
>  * remove 'drop' flag in datastore cache
>  * use maintenance-mode 'unmount' for unmounting process, only for the
>     unmounting not for being unmounted
>  * rename/simplify update-datastore-cache command
>  * ui: integrate new unmounting maintenance mode
>  * basically a mix of v3 and v4
> 
> v5: thanks @Dietmar and @Christian
>  * drop --force for unmount since it'll always fail if tasks are still running, and if
>     there are not normal unount will work
>  * improve several commit messages
>  * improve error message wording
>  * add removable datastore section to docs
>  * add documentation for is_datastore_available
> 
> v4: thanks a lot @Dietmar and @Christian
>  * make check if mounted wayyy faster
>  * don't keep track of mounting state
>  * drop Unplugged maintenance mode
>  * use UUID_FORMAT for uuid field
>  * a lot of small things, like use of bail!, inline format!, ...
>  * include improvement to cache handling
> 
> v3:
>  * remove lazy unmounting (since 9cba51ac782d04085c0af55128f32178e5132358 is applied)
>  * fix CLI (un)mount command, thanks @Gabriel
>  * add removable datastore CLI autocomplete helper
>  * rebase onto master
>  * move ui patches to the end
> 
> thanks @Lukas and @Thomas for the feedback
> v2:
>  * fix datastore 'add' button in the UI
>  * some format!("{}", a) -> format!("{a}")
>  * replace `const` with `let` in js code
>  * change icon `fa-usb` -> `fa-plug`
>  * add some docs
>  * add JDoc for parseMaintenanceMode
>  * proxmox-schema dep bump
> 
> Dietmar Maurer (2):
>   config: factor out method to get the absolute datastore path
>   maintenance: add 'Unmount' maintenance type
> 
> Hannes Laimer (24):
>   tools: add disks utility functions
>   pbs-api-types: add backing-device to DataStoreConfig
>   disks: add UUID to partition info
>   datastore: add helper for checking if a removable datastore is
>     available
>   api: admin: add (un)mount endpoint for removable datastores
>   api: removable datastore creation
>   api: disks list: add exclude-used flag
>   pbs-api-types: add removable/is-available flag to DataStoreListItem
>   bin: manager: add (un)mount command
>   add auto-mounting for removable datastores
>   datastore: handle deletion of removable datastore properly
>   docs: add removable datastores section
>   ui: add partition selector form
>   ui: add removable datastore creation support
>   ui: add (un)mount button to summary
>   ui: tree: render unmounted datastores correctly
>   ui: utils: make parseMaintenanceMode more robust
>   ui: add datastore status mask for unmounted removable datastores
>   ui: maintenance: fix disable msg field if no type is selected
>   ui: render 'unmount' maintenance mode correctly
>   api: node: allow creation of removable datastore through directory
>     endpoint
>   api: node: include removable datastores in directory list
>   node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR
>   ui: support create removable datastore through directory creation
> 
>  debian/proxmox-backup-server.install        |   1 +
>  debian/proxmox-backup-server.udev           |   3 +
>  docs/storage.rst                            |  29 +++
>  etc/Makefile                                |   3 +-
>  etc/removable-device-attach@.service.in     |   8 +
>  pbs-api-types/src/datastore.rs              |  42 +++-
>  pbs-api-types/src/maintenance.rs            |  11 +-
>  pbs-config/src/datastore.rs                 |  14 ++
>  pbs-datastore/src/datastore.rs              |  73 +++++-
>  pbs-datastore/src/lib.rs                    |   2 +-
>  src/api2/admin/datastore.rs                 | 260 ++++++++++++++++++--
>  src/api2/config/datastore.rs                |  69 +++++-
>  src/api2/node/disks/directory.rs            | 102 ++++++--
>  src/api2/node/disks/mod.rs                  |   8 +
>  src/api2/status.rs                          |  18 +-
>  src/bin/proxmox-backup-proxy.rs             |   9 +-
>  src/bin/proxmox_backup_manager/datastore.rs | 129 +++++++++-
>  src/tools/disks/mod.rs                      |  99 +++++++-
>  www/DirectoryList.js                        |  13 +
>  www/Makefile                                |   1 +
>  www/NavigationTree.js                       |  15 +-
>  www/Utils.js                                |  33 ++-
>  www/css/ext6-pbs.css                        |  20 ++
>  www/datastore/DataStoreListSummary.js       |   1 +
>  www/datastore/Summary.js                    | 113 ++++++++-
>  www/form/PartitionSelector.js               |  81 ++++++
>  www/window/CreateDirectory.js               |  14 ++
>  www/window/DataStoreEdit.js                 |  37 +++
>  www/window/MaintenanceOptions.js            |  17 +-
>  29 files changed, 1141 insertions(+), 84 deletions(-)
>  create mode 100644 etc/removable-device-attach@.service.in
>  create mode 100644 www/form/PartitionSelector.js
> 
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> 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