lxd/storage/drivers/zfs: Fix content type detection for custom block volumes #11991
Conversation
| // Detect if a volume is block content type using both the defined suffix and the dataset type. | ||
| isBlock := strings.HasSuffix(volName, zfsBlockVolSuffix) && zfsContentType == "volume" | ||
| // Detect if a volume is block content type using only the dataset type. | ||
| isBlock := zfsContentType == "volume" |
There was a problem hiding this comment.
@monstermunchkin did custom block volumes not use the .block suffix ever? or did that change recently?
There was a problem hiding this comment.
Does this function support detecting filesystem volumes with zfs.block_mode=true?
There was a problem hiding this comment.
They never have. That's only for VMs as they have two datasets/volumes.
There was a problem hiding this comment.
Does this function support detecting filesystem volumes with zfs.block_mode=true?
No, this function only takes the output of zfs list. zfs.block_mode is a LXD config key and is therefore unknown by this function. There's also no need for this config key as it wouldn't change anything.
There was a problem hiding this comment.
What this function does not support is listing custom ISO volumes. That is because they follow the same naming schema as custom block volumes. We might want to change this in the future, by suffixing ISOs with .iso or something.
There was a problem hiding this comment.
No, this function only takes the output of
zfs list.zfs.block_modeis a LXD config key and is therefore unknown by this function. There's also no need for this config key as it wouldn't change anything.
But those filesystem block volumes would be unusable after recovery though right?
There was a problem hiding this comment.
What this function does not support is listing custom ISO volumes. That is because they follow the same naming schema as custom block volumes. We might want to change this in the future, by suffixing ISOs with
.isoor something.
hrm yeah can you open an issue and assign that to you to sort out please.
There was a problem hiding this comment.
But those filesystem block volumes would be unusable after recovery though right?
Oh, I see what you mean, and you're right. As a workaround, we could do lxc storage volume set zfs vol1 zfs.block_mode=true, right?
There was a problem hiding this comment.
I dont think you can do that on a volume once it created - if you can thats a bug.
There was a problem hiding this comment.
We probably need to have a content type suffix for those volumes too
|
@monstermunchkin would it be possible to update the lxd recover tests to check for block volume support? |
Yes, that should be possible. I just came across User Properties which might be interesting in the way we name our datasets and volumes. We could add the |
|
Sounds very interesting indeed! Just need to check it works with the versions of ZFS we support. |
|
Is this awaiting review or should it be in draft? ZFS tests seem to have failed. |
|
This is ready for review. ZFS tests are failing in migration. Don't know why though. @tomponline Could you restart the tests? |
|
Please can you rebase and that will trigger it too |
…volumes This fixes an issue where custom block volumes would be listed as datasets instead of volumes. Fixes canonical#11984 Signed-off-by: Thomas Hipp <[email protected]>
Signed-off-by: Thomas Hipp <[email protected]>
monstermunchkin
force-pushed
the
issues/11984-lxd-recover
branch
from
ee58796 to
c4f1d67
Compare
This fixes an issue where custom block volumes would be listed as
datasets instead of volumes.
Fixes #11984
Signed-off-by: Thomas Hipp [email protected]