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

Disallow block settings for VM block volumes #12267

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

MusicDin
Copy link
Member

This PR disallows setting zfs.block_mode for custom volumes and VM block volumes. It also disallows setting block.* settings for VMs.

@MusicDin MusicDin force-pushed the fix/vm-zfs-block_mode branch 2 times, most recently from 0398c33 to ada6c5d Compare September 18, 2023 10:59
@MusicDin
Copy link
Member Author

MusicDin commented Sep 18, 2023

The tests are failing on the following scenario (simplified):

# Create a ZFS pool with with volume.block_mode set to true.
$ lxc storage create test zfs
$ lxc storage set test volume.zfs.block_mode true

# Create custom volume
$ lxc storage volume create test vol1

# The custom volume gets initialized as "filesystem".
[ "$(zfs get -H -o value type test/custom/default_vol1")" = "volume" ]

So, I can fix the test to reflect a new change, however, I'm unsure about the following scenario (see the comments above volume create commands):

# Normal behavior: Even though `volume.zfs.block_mode` is set on the pool, it is not applied.
$ lxc storage volume create test vol1
$ lxc storage volume show test vol1
#   config: {}
#   content_type: filesystem
#   ...

# Normal behavior: We get an error, because `zfs.block_mode` cannot be set to custom volumes.
$ lxc storage volume create test vol2 zfs.block_mode=false
Error: Invalid option for volume "default_vol2" option "zfs.block_mode"

# [ !!! ] Unsure about this one: The content_type is set to filesystem, but the volume config gets 
# populated with redundant settings.
#
# Should I ensure those settings are not applied?
# Or should an error be thrown in such case (note that zfs.block_mode matches with the default pool settings)?
$ lxc storage volume create test vol3 zfs.block_mode=true
$ lxc storage volume show test vol3
#   config:
#     block.filesystem: ext4
#     block.mount_options: discard
#     zfs.block_mode: "true"
#   content_type: filesystem

@tomponline
Copy link
Member

Normal behavior: We get an error, because zfs.block_mode cannot be set to custom volumes.

This is untrue. zfs.block_mode is absolutely valid for custom filesystem volumes, just not for custom block volumes (--type=block).

So this error Error: Invalid option for volume "default_vol2" option "zfs.block_mode" is a bug I think.

This scenario is correct and valid behaviour because a custom filesystem volume is being create, along with the defaults for block backed filesystem volumes.

$ lxc storage volume create test vol3 zfs.block_mode=true
$ lxc storage volume show test vol3
#   config:
#     block.filesystem: ext4
#     block.mount_options: discard
#     zfs.block_mode: "true"
#   content_type: filesystem

So it looks like theres a misunderstanding here about what should and shouldn't be allowed.

There are 2 types of custom volumes:

  • Filesystem
  • Block

The block.* settings only apply to filesystem type custom volumes that are block-backed, not block volumes (as these don't have a LXD managed filesystem on them).

For ZFS pools, the block.* settings only apply to filesystem volumes if zfs.block_modeis enabled, as then the volume is block-backed, and thus must have a LXD manage filesystem applied to them, rather than the normal ZFS dataset filesystem.

@MusicDin
Copy link
Member Author

So if we are trying to create a block custom volume, zfs.block_mode should not be applicable?

For example, the following should result in an error?

lxc storage volume create default vol1 zfs.block_mode=true --type=block

@tomponline
Copy link
Member

Correct

…de for VM block volumes

Signed-off-by: Din Music <din.music@canonical.com>
@MusicDin
Copy link
Member Author

@tomponline Can you review this PR again?

Based on your comments, I've done the following:

  • If volume is either VM block or custom block volume - We ignore zfs.block_mode and block.* options.
  • If volume is custom filesystem volume that is not block-backed - We exclude only block.* options.

@tomponline
Copy link
Member

@MusicDin please can you show me the test scenarios you used for this? Thanks

@MusicDin
Copy link
Member Author

Custom volumes:

# OK
lxc storage volume create default vol1 --type=block
lxc storage volume create default vol2 zfs.block_mode=false
lxc storage volume create default vol3 zfs.block_mode=true
lxc storage volume create default vol4 zfs.block_mode=true block.filesystem=btrfs

# Error: Invalid option for volume "default_vol1" option "block.filesystem"
lxc storage volume create default vol zfs.block_mode=false block.filesystem=btrfs

# Error: Invalid option for volume "default_vol1" option "<option>"
lxc storage volume create default vol --type=block zfs.block_mode=true 
lxc storage volume create default vol --type=block zfs.block_mode=false
lxc storage volume create default vol --type=block block.filesystem=btrfs
lxc storage volume create default vol --type=block block.mount_options=noatime

VMs:

# OK

lxc storage set default volume.zfs.block_mode=true
lxc storage set default volume.block.filesystem=btrfs
lxc launch ubuntu:22.04 vm1 --vm

lxc storage set default volume.zfs.block_mode=false
lxc launch ubuntu:22.04 vm2 --vm

Merging with initial configs branch:

$ lxc launch ubuntu:22.04 vm3 --vm -d root,initial.zfs.block_mode=true
Creating vm
Error: Failed instance creation: Failed creating instance record: Failed initialising instance: Failed add validation for device "root": Invalid initial device configuration: Invalid option for volume "root" option "zfs.block_mode"


$ lxc launch ubuntu:22.04 vm --vm -d root,initial.zfs.block_mode=false
Creating vm
Error: Failed instance creation: Failed creating instance record: Failed initialising instance: Failed add validation for device "root": Invalid initial device configuration: Invalid option for volume "root" option "zfs.block_mode"

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@tomponline tomponline merged commit 05910a6 into canonical:main Sep 19, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants