-
-
Notifications
You must be signed in to change notification settings - Fork 362
feat: support NFS storage pools #2025
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
package drivers | ||
|
||
import ( | ||
"strings" | ||
|
||
"github.com/lxc/incus/v6/internal/linux" | ||
"github.com/lxc/incus/v6/internal/migration" | ||
deviceConfig "github.com/lxc/incus/v6/internal/server/device/config" | ||
localMigration "github.com/lxc/incus/v6/internal/server/migration" | ||
"github.com/lxc/incus/v6/shared/util" | ||
) | ||
|
||
type nfs struct { | ||
dir | ||
} | ||
|
||
// Info returns info about the driver and its environment. | ||
func (n *nfs) Info() Info { | ||
return Info{ | ||
Name: "nfs", | ||
Version: "1", | ||
DefaultVMBlockFilesystemSize: deviceConfig.DefaultVMBlockFilesystemSize, | ||
OptimizedImages: false, | ||
PreservesInodes: false, | ||
Remote: n.isRemote(), | ||
VolumeTypes: []VolumeType{VolumeTypeBucket, VolumeTypeCustom, VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM}, | ||
VolumeMultiNode: n.isRemote(), | ||
BlockBacking: false, | ||
RunningCopyFreeze: false, | ||
DirectIO: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about that one, does NFS handle O_DIRECT? |
||
MountedRoot: true, | ||
Buckets: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that should be set to false given above. |
||
} | ||
} | ||
|
||
// isRemote returns true indicating this driver uses remote storage. | ||
func (n *nfs) isRemote() bool { | ||
return true | ||
} | ||
|
||
// FillConfig populates the storage pool's configuration file with the default values. | ||
func (n *nfs) FillConfig() error { | ||
uri := strings.Split(n.config["source"], ":") | ||
|
||
// URI should be first part of IP:PORT | ||
n.config["nfs.addr"] = uri[0] | ||
|
||
return nil | ||
} | ||
|
||
func (n *nfs) getMountOptions() string { | ||
// Allow overriding the default options. | ||
if n.config["nfs.mount_options"] != "" { | ||
return n.config["nfs.mount_options"] | ||
} | ||
// We only really support vers=4.2 | ||
return "vers=4.2,addr=" + n.config["nfs.addr"] | ||
} | ||
|
||
// Create is called during pool creation and is effectively using an empty driver struct. | ||
// WARNING: The Create() function cannot rely on any of the struct attributes being set. | ||
func (n *nfs) Create() error { | ||
err := n.FillConfig() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
sourcePath := n.config["source"] | ||
|
||
// Mount the nfs driver. | ||
mntFlags, mntOptions := linux.ResolveMountOptions(strings.Split(n.getMountOptions(), ",")) | ||
err = TryMount(sourcePath, GetPoolMountPath(n.name), "nfs4", mntFlags, mntOptions) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
defer func() { _, _ = forceUnmount(GetPoolMountPath(n.name)) }() | ||
|
||
return nil | ||
} | ||
|
||
func (n *nfs) Mount() (bool, error) { | ||
Check failure on line 82 in internal/server/storage/drivers/driver_nfs.go
|
||
path := GetPoolMountPath(n.name) | ||
|
||
// Check if already mounted. | ||
if linux.IsMountPoint(path) { | ||
return false, nil | ||
} | ||
|
||
sourcePath := n.config["source"] | ||
n.config["nfs.path"] = sourcePath | ||
|
||
// Check if we're dealing with an external mount. | ||
if sourcePath == path { | ||
return false, nil | ||
} | ||
|
||
// Mount the nfs driver. | ||
mntFlags, mntOptions := linux.ResolveMountOptions(strings.Split(n.getMountOptions(), ",")) | ||
err := TryMount(sourcePath, GetPoolMountPath(n.name), "nfs4", mntFlags, mntOptions) | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
return true, nil | ||
} | ||
|
||
func (n *nfs) MigrationTypes(contentType ContentType, refresh bool, copySnapshots bool, clusterMove bool, storageMove bool) []localMigration.Type { | ||
Check failure on line 108 in internal/server/storage/drivers/driver_nfs.go
|
||
// NFS does not support xattr | ||
rsyncFeatures := []string{"delete", "bidirectional"} | ||
if util.IsTrue(n.Config()["rsync.compression"]) { | ||
rsyncFeatures = append(rsyncFeatures, "compress") | ||
} | ||
|
||
return []localMigration.Type{ | ||
{ | ||
FSType: migration.MigrationFSType_BLOCK_AND_RSYNC, | ||
Features: rsyncFeatures, | ||
}, | ||
{ | ||
FSType: migration.MigrationFSType_RSYNC, | ||
Features: rsyncFeatures, | ||
}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So initially my thought was to only allow
VolumeTypeCustom
on there, so only allowing NFS to be used for additional storage to instances either stored on local storage or stored on a proper block shared storage solution (clustered LVM, Ceph or Linstor).I think we should at least exclude
VolumeTypeBucket
andVolumeTypeImage
.Container and VM, we'd need to see how well they work. My gut feeling given NFS locking is "not well", the question is then whether we still allow it but document that it's really not recommended or we plain not allow it and only keep custom volumes on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it depends on the amount of work required. 4 hours of work that could potentially go to the trash is ok. 4 days, not so much.