[pbs-devel] [PATCH proxmox-backup v12 00/26] add removable datastores
Hannes Laimer
h.laimer at proxmox.com
Tue Oct 29 17:18:43 CET 2024
Thanks a lot for taking the time to review this series, I added some
comments to things you mentioned. And I'll fix the obviouse
shortcommings of this version in the next one.
On Mon Oct 14, 2024 at 3:43 PM CEST, Fabian Grünbichler wrote:
> 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 at .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 at .service.in
> > create mode 100644 www/form/PartitionSelector.js
> >
> > --
> > 2.39.2
> >
> >
> >
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel at lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> >
> >
> >
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
More information about the pbs-devel
mailing list