Add `linstor` storage driver #1621

luissimas · 2025-02-01T00:36:10Z

Description

This PR introduces a new linstor storage driver to enable provisioning of volumes backed by LINSTOR, as discussed in #564.

All storage driver features are implemented except for storage buckets. Storage buckets cannot be implemented natively on LINSTOR, and the most logical approach would be to fallback to the generic implementation like it's done for all drivers other than Ceph. We didn't go in that route due to the fact that today Incus considers that any remote storage driver supports an optimized implementation of storage buckets, but that's not the case for LINSTOR ( https://github.com/lxc/incus/blob/main/internal/server/storage/backend.go#L3827).

Although most driver features are implemented, not all of them are optimized using LINSTOR native capabilities. We fallback to the generic implementation in several cases, including:

Migration across different storage pools
Copying volumes with snapshots
Backups
Refreshing volumes

Closes #564

luissimas · 2025-02-01T00:40:53Z

We expect to send integration tests validating the scenarios in the next few days. I think we'll probably need some Linstor setup on the testing environment as well.

stgraber · 2025-02-02T11:40:01Z

There's a bunch of static analysis failure around code styling/formatting that should be pretty easy to handle.

cmd/incusd/daemon.go

internal/server/storage/drivers/driver_linstor.go

internal/server/storage/drivers/driver_linstor_utils.go

internal/server/storage/drivers/driver_linstor_volumes.go

internal/server/storage/linstor/linstor.go

bensmrs · 2025-02-04T19:38:16Z

cmd/incusd/daemon.go

+func (d *Daemon) getLinstor() (*linstor.Client, error) {
+	// Setup the client if it does not exist
+	if d.linstor == nil {
+		err := d.setupLinstor()


I’m seeing a small race condition leading to a double execution of setupLinstor:

getLinstor gets called until after the d.linstor == nil condition is checked

setupLinstor gets called before err := d.setupLinstor() gets called

setupLinstor gets called again by err := d.setupLinstor().

I’d put a synchronization here…

internal/server/storage/drivers/driver_linstor_utils.go

cmd/incusd/daemon.go

internal/server/storage/drivers/driver_linstor.go

luissimas · 2025-02-05T22:28:16Z

Hey @stgraber just a quick update on the state of things. @winiciusallan, @bensmrs and I did a quick call about the current state of the implementation and the next steps. Here are the main takeaways:

@bensmrs is working on the CI tests for the driver
While looking at the tests, we've identified that the lack of support for storage buckets may be a block for merging this, at least from the test's perspective. Can you confirm that?
@winiciusallan will look into how much work would it take to support storage buckets in the linstor driver
Renaming volumes is most likely a block for merging. Our initial approach will be to use the volume ID in the Incus database as the Resource Definition name in LINSTOR. We'll need to check if the ID is available on all code paths that create the volumes. We also need to be aware of the 48 character limit on LINSTOR object names. @bensmrs is probably the best person to tackle this, as he is the most familiar with the Incus code base.
We still need to figure out a good way to validate that all Incus nodes also run the linstor-satellite service and that the node names are consistent between Incus and LINSTOR
I'll finish addressing @bensmrs's comments on this PR and then I'll check if it is possible to have multiple LINSTOR controllers managing the same set of satellites. I'm guessing that is not supported, but if it is, then we should set the controller connection string at the storage pool level rather than in a global config option

Signed-off-by: Luís Simas <[email protected]>

…ng of volumes with snapshots Signed-off-by: Luís Simas <[email protected]>

Signed-off-by: Benjamin Somers <[email protected]>

Signed-off-by: Luís Simas <[email protected]>

Signed-off-by: Benjamin Somers <[email protected]>

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

…r resource definitions Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

…rieving device paths Signed-off-by: Luís Simas <[email protected]>

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

…volumes Signed-off-by: Luís Simas <[email protected]>

…napshots Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

…umes Signed-off-by: Luís Simas <[email protected]>

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

Among other changes, this commit changes quota sizes to accomodate slightly bigger volumes on LINSTOR and to avoid having tolerance ranges intersect for QUOTA1 and QUOTA2. Signed-off-by: Benjamin Somers <[email protected]> Co-authored-by: Luís Simas <[email protected]>

Signed-off-by: Benjamin Somers <[email protected]>

Signed-off-by: Luís Simas <[email protected]>

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

Signed-off-by: Benjamin Somers <[email protected]>

stgraber · 2025-03-26T18:16:09Z

Rebase done, just waiting for the tests to clear, then will merge.
Looks like I'll have to force merge it to bypass the DCO test failure.

bensmrs · 2025-03-26T22:58:10Z

Well that’s very nice to read!
Thanks @luissimas and @winiciusallan, it was a real pleasure working with you and debugging all these nasty edge cases :)

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [lxc/incus](https://github.com/lxc/incus) | minor | `v6.10.1` -> `v6.11.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>lxc/incus (lxc/incus)</summary> ### [`v6.11.0`](https://github.com/lxc/incus/releases/tag/v6.11.0): Incus 6.11 [Compare Source](lxc/incus@v6.10.1...v6.11.0) ### Announcement https://discuss.linuxcontainers.org/t/incus-6-11-has-been-released/23322 #### What's Changed - Allow ICMP and low ports for unprivileged users in OCI containers by [@gwenya](https://github.com/gwenya) in lxc/incus#1706 - doc: Clarify virtiofsd requirements by [@stgraber](https://github.com/stgraber) in lxc/incus#1718 - Fix generate-database usage for incusd/db by [@breml](https://github.com/breml) in lxc/incus#1719 - Do not allow mounting of custom block volume snapshots by [@presztak](https://github.com/presztak) in lxc/incus#1720 - generate-database: Abstract db connection / db transaction by [@breml](https://github.com/breml) in lxc/incus#1721 - Fix snapshot size handling in cross-pool copy/move by [@presztak](https://github.com/presztak) in lxc/incus#1717 - generate-database: Accept interface in PrepareStmts by [@breml](https://github.com/breml) in lxc/incus#1725 - Simplify `evaluateShorthandFilter` by reducing nesting levels by [@presztak](https://github.com/presztak) in lxc/incus#1727 - incusd/storage: Don't use sparse writer on thick LVM by [@stgraber](https://github.com/stgraber) in lxc/incus#1729 - generate-database: Add support for marshal to JSON by [@breml](https://github.com/breml) in lxc/incus#1731 - Fixed incus edk2 path overwrite issue by [@nanjj](https://github.com/nanjj) in lxc/incus#1726 - Do not download instance types if cache loadable by [@nanjj](https://github.com/nanjj) in lxc/incus#1732 - Clarify security.secureboot setting by [@gwenya](https://github.com/gwenya) in lxc/incus#1740 - Fix DNS for isolated OVN networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1738 - Allow announcing extra routes through DHCPv4 by [@gwenya](https://github.com/gwenya) in lxc/incus#1734 - Fix link parsing failure on non-ethernet devices by [@stgraber](https://github.com/stgraber) in lxc/incus#1742 - Fix revert on OCI container creation failure by [@gwenya](https://github.com/gwenya) in lxc/incus#1744 - generate-database: Handle non tx DB connections by [@breml](https://github.com/breml) in lxc/incus#1745 - incus file edit extension by [@gwenya](https://github.com/gwenya) in lxc/incus#1746 - Cleanup internal API endpoints by [@stgraber](https://github.com/stgraber) in lxc/incus#1747 - Tweak help message for rebuild by [@stgraber](https://github.com/stgraber) in lxc/incus#1754 - Use lego binary for DNS-01 challenge by [@accuser](https://github.com/accuser) in lxc/incus#1753 - incusd/storage/zfs: Fix ZFS CreateVolume deletes pre-existing data on failure by [@mrstux](https://github.com/mrstux) in lxc/incus#1749 - incus/file: Always use 1MB chunks for SFTP by [@stgraber](https://github.com/stgraber) in lxc/incus#1758 - Use the correct path for ingesting DNS-01 challenge certificate outputs by [@accuser](https://github.com/accuser) in lxc/incus#1759 - incusd/bgp: Rework start/stop logic by [@stgraber](https://github.com/stgraber) in lxc/incus#1761 - incusd/network/ovn: Skip existing static routes by [@stgraber](https://github.com/stgraber) in lxc/incus#1762 - incusd/instance/qemu: Set caching-mode with intel-iommu by [@stgraber](https://github.com/stgraber) in lxc/incus#1772 - incus-agent: Improve SFTP performance by [@stgraber](https://github.com/stgraber) in lxc/incus#1773 - incusd/network/ovn: Keep getting router name when network none by [@diegofernandes](https://github.com/diegofernandes) in lxc/incus#1771 - make `incus copy --device xx,type=none` drop remaining device properties by [@schnoddelbotz](https://github.com/schnoddelbotz) in lxc/incus#1764 - incusd/instance/qemu: rtc base localtime for windows by [@nanjj](https://github.com/nanjj) in lxc/incus#1767 - Add option to configure DNS server for bridge and OVN networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1739 - Use lego binary for http 01 challenge by [@accuser](https://github.com/accuser) in lxc/incus#1770 - Handle live migration between QEMU versions by [@stgraber](https://github.com/stgraber) in lxc/incus#1775 - incusd/instance/qemu: Skip to link nvram to itself by [@nanjj](https://github.com/nanjj) in lxc/incus#1760 - Switch to new MAC address prefix by [@stgraber](https://github.com/stgraber) in lxc/incus#1776 - client: Fix spelling errors found by codespell by [@cjwatson](https://github.com/cjwatson) in lxc/incus#1777 - Add ipv4.dhcp.expiry option for ovn networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1781 - Configure DHCP on existing instance interfaces when it is enabled on a network by [@gwenya](https://github.com/gwenya) in lxc/incus#1780 - incusd/instance/edk2: Select SecureBoot capable firmware on Debian by [@stgraber](https://github.com/stgraber) in lxc/incus#1782 - Fix some `go vet` warnings by [@stgraber](https://github.com/stgraber) in lxc/incus#1784 - Clear gofumpt by [@stgraber](https://github.com/stgraber) in lxc/incus#1803 - Fix some BGP issues by [@stgraber](https://github.com/stgraber) in lxc/incus#1805 - incusd/instance/qemu: bad pid check by [@nanjj](https://github.com/nanjj) in lxc/incus#1806 - Fix spelling errors and run codespell automatically by [@cjwatson](https://github.com/cjwatson) in lxc/incus#1778 - incus/file: Properly handle relative source paths by [@stgraber](https://github.com/stgraber) in lxc/incus#1809 - cmd/storage: incorrect CLI syntax in storage pool creation examples by [@ViniRodrig](https://github.com/ViniRodrig) in lxc/incus#1810 - Improve DB performance by [@stgraber](https://github.com/stgraber) in lxc/incus#1811 - incusd/network/ovn: Fix default DNS IPv4 server by [@stgraber](https://github.com/stgraber) in lxc/incus#1812 - Extend OS detection logic by [@stgraber](https://github.com/stgraber) in lxc/incus#1813 - Add allocated CPU time to instance state by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1807 - incusd/certificates: Properly handle bad PEM data by [@stgraber](https://github.com/stgraber) in lxc/incus#1816 - Extra `generate-database` features by [@masnax](https://github.com/masnax) in lxc/incus#1817 - incusd/network/common: Handle missing BGP peer by [@stgraber](https://github.com/stgraber) in lxc/incus#1818 - incusd/cluster/evacuate: Don't live-migrate stopped instances by [@stgraber](https://github.com/stgraber) in lxc/incus#1819 - Fix generator table pluralization by [@masnax](https://github.com/masnax) in lxc/incus#1823 - incusd/instance/qemu enable s4 by default by [@nanjj](https://github.com/nanjj) in lxc/incus#1820 - Add support for USB NICs by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1814 - incusd/storage/s3 Fixed minio client mc too ambious issue by [@nanjj](https://github.com/nanjj) in lxc/incus#1821 - incusd/networks: Validate configuration on join too by [@stgraber](https://github.com/stgraber) in lxc/incus#1824 - Update gomod for go-jwt vulnerability by [@stgraber](https://github.com/stgraber) in lxc/incus#1825 - cmd/generate-database/db: Fix GetNames spacing by [@masnax](https://github.com/masnax) in lxc/incus#1826 - github: Rework issue templates by [@stgraber](https://github.com/stgraber) in lxc/incus#1827 - Update Debian installation documentation by [@gibmat](https://github.com/gibmat) in lxc/incus#1830 - Extend minio client naming by [@gibmat](https://github.com/gibmat) in lxc/incus#1829 - Various fixes from address set MR by [@stgraber](https://github.com/stgraber) in lxc/incus#1831 - incusd/instance/lxc: Cleanup OCI mount paths by [@stgraber](https://github.com/stgraber) in lxc/incus#1834 - Add `io.bus=usb` for disks by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1835 - golangci: Upgrade to version 2 by [@stgraber](https://github.com/stgraber) in lxc/incus#1836 - golangci: Disable STI005 error checks by [@stgraber](https://github.com/stgraber) in lxc/incus#1841 - Standalone changes from the Linstor branch by [@stgraber](https://github.com/stgraber) in lxc/incus#1842 - incusd/storage/s3 minio client check enhancement by [@nanjj](https://github.com/nanjj) in lxc/incus#1839 - incusd/network/ovn: Remove internal routes to forward/load-balancers by [@stgraber](https://github.com/stgraber) in lxc/incus#1843 - incusd/instance/edk2: Always prefer the EDK2 override by [@stgraber](https://github.com/stgraber) in lxc/incus#1847 - Fixes from Linstor branch by [@stgraber](https://github.com/stgraber) in lxc/incus#1846 - Add `linstor` storage driver by [@luissimas](https://github.com/luissimas) in lxc/incus#1621 - Add `linstor.remove_snapshots` config option by [@luissimas](https://github.com/luissimas) in lxc/incus#1848 - doc/support: Update feature release version by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1853 - incusd/instance: Don't enforce device/config validation on snapshots by [@stgraber](https://github.com/stgraber) in lxc/incus#1854 - OCI entrypoint configuration by [@gwenya](https://github.com/gwenya) in lxc/incus#1845 #### New Contributors - [@mrstux](https://github.com/mrstux) made their first contribution in lxc/incus#1749 - [@diegofernandes](https://github.com/diegofernandes) made their first contribution in lxc/incus#1771 - [@schnoddelbotz](https://github.com/schnoddelbotz) made their first contribution in lxc/incus#1764 - [@cjwatson](https://github.com/cjwatson) made their first contribution in lxc/incus#1777 - [@ViniRodrig](https://github.com/ViniRodrig) made their first contribution in lxc/incus#1810 - [@masnax](https://github.com/masnax) made their first contribution in lxc/incus#1817 **Full Changelog**: lxc/incus@v6.10.1...v6.11.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

luissimas requested a review from stgraber as a code owner February 1, 2025 00:36

github-actions bot added the Documentation Documentation needs updating label Feb 1, 2025

luissimas force-pushed the linstor branch 2 times, most recently from ea94519 to cf18bb1 Compare February 1, 2025 01:09

luissimas force-pushed the linstor branch 4 times, most recently from 5723ea8 to 6d4dede Compare February 3, 2025 19:54

bensmrs reviewed Feb 4, 2025

View reviewed changes

bensmrs reviewed Feb 5, 2025

View reviewed changes

internal/server/storage/drivers/driver_linstor_utils.go Outdated Show resolved Hide resolved

bensmrs reviewed Feb 5, 2025

View reviewed changes

cmd/incusd/daemon.go Show resolved Hide resolved

bensmrs reviewed Feb 5, 2025

View reviewed changes

internal/server/storage/drivers/driver_linstor.go Show resolved Hide resolved

luissimas force-pushed the linstor branch 5 times, most recently from 7c04d43 to 3d7e8c7 Compare February 5, 2025 21:22

luissimas force-pushed the linstor branch 3 times, most recently from c38398e to 120ae22 Compare February 6, 2025 11:17

luissimas changed the title ~~Add Linstor storage driver~~ Add linstor storage driver Feb 6, 2025

bensmrs force-pushed the linstor branch from 120ae22 to 4729601 Compare February 6, 2025 13:10

github-actions bot added the API Changes to the REST API label Feb 6, 2025

bensmrs force-pushed the linstor branch from 4729601 to 85fdf8c Compare February 6, 2025 14:44

luissimas marked this pull request as draft February 6, 2025 21:35

bensmrs force-pushed the linstor branch from 85fdf8c to 14c2c32 Compare February 7, 2025 13:43

luissimas and others added 22 commits March 26, 2025 14:14

incusd/storage/linstor: Add support for optimized images

b34e65b

Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: List snapshots to support non optimized copyi…

2639813

…ng of volumes with snapshots Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Implement GetResources

79cad37

Signed-off-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Add support for updating storage pool config

638b042

Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Add support for generic refreshing of volumes

1c85bc8

Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Add support for fetching volume disk usage

803afa2

Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Add support for non optimized backups

e2b000e

Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Encode snapshot names in metadata

bee4434

Signed-off-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Add support for listing volumes

b3926a0

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Use aux property to map incus names to linsto…

12f8874

…r resource definitions Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Ensure volumes are available locally when ret…

3a3bf61

…rieving device paths Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Fallback to generic migration implementation

4cefc5d

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Freeze and sync filesystem when snapshotting …

9aef3c8

…volumes Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Cleanup temporary resource definitions from s…

57371f6

…napshots Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

incusd/storage/linstor: Delete diskless resources when unmounting vol…

05865f0

…umes Signed-off-by: Luís Simas <[email protected]>

incusd/storage/linstor: Add drbd resource config options

714cfd8

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

tests: Add clustered LINSTOR tests

5e6edb4

Signed-off-by: Benjamin Somers <[email protected]>

internal/migration: Update protobuf

f515ade

Signed-off-by: Luís Simas <[email protected]>

doc: Update configs

038d9c4

Signed-off-by: Luís Simas <[email protected]> Co-authored-by: Benjamin Somers <[email protected]>

gomod: Tidy dependencies and add LINSTOR

1c0fdc0

Signed-off-by: Benjamin Somers <[email protected]>

github: Add LINSTOR CI tests

809c559

Signed-off-by: Benjamin Somers <[email protected]>

stgraber force-pushed the linstor branch from 894abe8 to 809c559 Compare March 26, 2025 18:15

stgraber enabled auto-merge March 26, 2025 18:15

stgraber disabled auto-merge March 26, 2025 18:16

luissimas mentioned this pull request Mar 26, 2025

Add linstor.remove_snapshots config option #1848

Merged

stgraber merged commit 90a09b1 into lxc:main Mar 26, 2025
37 of 38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `linstor` storage driver #1621

Add `linstor` storage driver #1621

luissimas commented Feb 1, 2025 •

edited

Loading

luissimas commented Feb 1, 2025

stgraber commented Feb 2, 2025

bensmrs Feb 4, 2025

luissimas commented Feb 5, 2025

stgraber commented Mar 26, 2025

bensmrs commented Mar 26, 2025

Add linstor storage driver #1621

Add linstor storage driver #1621

Conversation

luissimas commented Feb 1, 2025 • edited Loading

Description

luissimas commented Feb 1, 2025

stgraber commented Feb 2, 2025

bensmrs Feb 4, 2025

Choose a reason for hiding this comment

luissimas commented Feb 5, 2025

stgraber commented Mar 26, 2025

bensmrs commented Mar 26, 2025

Add `linstor` storage driver #1621

Add `linstor` storage driver #1621

luissimas commented Feb 1, 2025 •

edited

Loading