Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ZFS mirror/striping #754

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

ps-gill
Copy link
Contributor

@ps-gill ps-gill commented Apr 16, 2024

This pull request resolves issue #752

Summary:

  • Updating ZFS driver to support multiple block devices and vdev types (mirror, raidz1 and raidz2). Default vdev type, stripe, is already supported.
  • Updating ZFS driver docs on how to use the new config.

@github-actions github-actions bot added the Documentation Documentation needs updating label Apr 16, 2024
@ps-gill ps-gill marked this pull request as ready for review April 16, 2024 00:07
@ps-gill ps-gill requested a review from stgraber as a code owner April 16, 2024 00:07
@stgraber
Copy link
Member

General structure looks good, just a few styling nits and some smaller bits that need tweaking/fixing.

You should also add an extra commit, api: storage_zfs_vdev containing changes to both doc/api-extensions.md and internal/version/api.go introducing an API extension of that same name so API clients can detect that Incus supports the new syntax.

@ps-gill ps-gill force-pushed the feature/752-zfs-pool-type-support branch from 010d634 to d1431ec Compare April 21, 2024 00:14
@github-actions github-actions bot added the API Changes to the REST API label Apr 21, 2024
@ps-gill ps-gill force-pushed the feature/752-zfs-pool-type-support branch from eb8165a to b1412e3 Compare April 21, 2024 00:35
@ps-gill ps-gill requested a review from stgraber April 21, 2024 00:37
@ps-gill ps-gill requested a review from stgraber April 21, 2024 19:19
@stgraber stgraber force-pushed the feature/752-zfs-pool-type-support branch from 521a8c8 to 6298f69 Compare April 22, 2024 11:35
@stgraber
Copy link
Member

Just did a quick rebase, merged two commits together and re-titled the commits as well as ran make static-analysis and fixed the 2-3 things that picked up.

Should be all good now, just waiting for Github to run all the tests.

Thanks!

@ps-gill
Copy link
Contributor Author

ps-gill commented Apr 22, 2024

The documentation test is failing due to vdev being lowercase and config (short for configuration). If you prefer, I can get those patched.

@stgraber
Copy link
Member

Ah yeah, you probably need to escape vdev as it's not a dictionary word and config indeed needs to become configuration.

Please make those two edits directly in the doc commit then when things pass on Github I can merge it.

I'm on my phone for the rest of the day here so can't directly edit commits.

@ps-gill ps-gill force-pushed the feature/752-zfs-pool-type-support branch from 6298f69 to 339bc81 Compare April 22, 2024 13:53
@ps-gill
Copy link
Contributor Author

ps-gill commented Apr 22, 2024

Ah yeah, you probably need to escape vdev as it's not a dictionary word and config indeed needs to become configuration.

Please make those two edits directly in the doc commit then when things pass on Github I can merge it.

I'm on my phone for the rest of the day here so can't directly edit commits.

It is done.

@stgraber
Copy link
Member

The api-extensions.md fix needed to go in the api: commit, not the doc: commit, sorry :)

@ps-gill ps-gill force-pushed the feature/752-zfs-pool-type-support branch from 339bc81 to e375d8b Compare April 22, 2024 17:42
@ps-gill
Copy link
Contributor Author

ps-gill commented Apr 22, 2024

The api-extensions.md fix needed to go in the api: commit, not the doc: commit, sorry :)

Just fixed it.

Sorry, I should have done it that way the first time. A workflow where commit message and contents are tightly integrated with CI is new to me.

@stgraber stgraber merged commit bf24e5a into lxc:main Apr 22, 2024
25 checks passed
@ps-gill ps-gill deleted the feature/752-zfs-pool-type-support branch April 22, 2024 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Development

Successfully merging this pull request may close these issues.

2 participants