Merge pull request #1059 from pohly/xfs-ioctl

XFS: fix creating volumes on OpenShift

Author: pohly

pkg/pmem-csi-driver/nodeserver.go

@@ -30,6 +30,7 @@ import (
 	"github.com/intel/pmem-csi/pkg/pmem-csi-driver/parameters"
 	pmdmanager "github.com/intel/pmem-csi/pkg/pmem-device-manager"
 	"github.com/intel/pmem-csi/pkg/volumepathhandler"
+	"github.com/intel/pmem-csi/pkg/xfs"
 )
 
 const (
@@ -285,12 +286,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 
 	if ephemeral && fsType == "xfs" {
-		// FS was created only in ephemeral case.
-		// Only if created filesytem and it was XFS:
-		// Tune XFS file system to serve huge pages:
-		// Set file system extent size to 2 MiB sized and aligned block allocations.
-		_, err := pmemexec.RunCommand(ctx, "xfs_io", "-c", "extsize 2m", hostMount)
-		if err != nil {
+		if err := xfs.ConfigureFS(hostMount); err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 	}
@@ -587,11 +583,8 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	if existingFsType == "" && requestedFsType == "xfs" {
-		// Only if created a new filesytem and it was XFS:
-		// Align XFS file system for hugepages
-		_, err := pmemexec.RunCommand(ctx, "xfs_io", "-c", "extsize 2m", stagingtargetPath)
-		if err != nil {
+	if requestedFsType == "xfs" {
+		if err := xfs.ConfigureFS(stagingtargetPath); err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 	}

pkg/xfs/xfs.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2022 Intel Corporation
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package xfs
+
+// #include <linux/fs.h>
+// #include <sys/ioctl.h>
+// #include <errno.h>
+// #include <string.h>
+//
+// char *getxattr(int fd, struct fsxattr *arg) {
+//     return ioctl(fd, FS_IOC_FSGETXATTR, arg) == 0 ? 0 : strerror(errno);
+// }
+//
+// char *setxattr(int fd, struct fsxattr *arg) {
+//     return ioctl(fd, FS_IOC_FSSETXATTR, arg) == 0 ? 0 : strerror(errno);
+// }
+import "C"
+
+import (
+	"fmt"
+	"os"
+)
+
+// ConfigureFS must be called after mkfs.xfs for the mounted
+// XFS filesystem to prepare the volume for usage as fsdax.
+// It is idempotent.
+func ConfigureFS(path string) error {
+	// Operate on root directory.
+	file, err := os.Open(path)
+	if err != nil {
+		return fmt.Errorf("open %q: %v", path, err)
+	}
+	defer file.Close()
+	fd := C.int(file.Fd())
+
+	// Get extended attributes.
+	var attr C.struct_fsxattr
+	if errnostr := C.getxattr(fd, &attr); errnostr != nil {
+		return fmt.Errorf("FS_IOC_FSGETXATTR for %q: %v", path, C.GoString(errnostr))
+	}
+
+	// Set extsize to 2m to enable hugepages in combination with
+	// fsdax. This is equivalent to the "xfs_io -c 'extsize 2m'" invocation
+	// mentioned in https://nvdimm.wiki.kernel.org/2mib_fs_dax
+	attr.fsx_xflags |= C.FS_XFLAG_EXTSZINHERIT
+	attr.fsx_extsize = 2 * 1024 * 1024
+	if errnostr := C.setxattr(fd, &attr); errnostr != nil {
+		return fmt.Errorf("FS_IOC_FSSETXATTR for %q: %v", path, C.GoString(errnostr))
+	}
+
+	return nil
+}

pkg/xfs/xfs_test.go

@@ -0,0 +1,21 @@
+/*
+Copyright 2022 Intel Corporation
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package xfs
+
+import (
+	"testing"
+)
+
+func Test_ConfigureFS(t *testing.T) {
+	// This is assumed to be backed by tmpfs and thus doesn't support xattr.
+	tmp := t.TempDir()
+	err := ConfigureFS(tmp)
+	if err == nil {
+		t.Fatal("did not get expected error")
+	}
+	t.Logf("got expected error: %v", err)
+}

test/e2e/storage/dax/dax.go

@@ -123,7 +123,7 @@ func (p *daxTestSuite) DefineTests(driver storageframework.TestDriver, pattern s
 		init()
 		defer cleanup()
 
-		testDaxInPod(f, l.root, l.resource.Pattern.VolMode, l.resource.VolSource, l.config, withKataContainers, p.daxSupported)
+		testDaxInPod(f, l.root, l.resource.Pattern.VolMode, l.resource.VolSource, l.config, withKataContainers, p.daxSupported, l.resource.Pattern.FsType)
 	})
 }
 
@@ -135,6 +135,7 @@ func testDaxInPod(
 	config *storageframework.PerTestConfig,
 	withKataContainers bool,
 	daxSupported bool,
+	fstype string,
 ) {
 	expectDax := daxSupported
 	if withKataContainers {
@@ -151,11 +152,19 @@ func testDaxInPod(
 		}
 	}
 
+	// Workaround for https://github.com/kubernetes/kubernetes/issues/107286:
+	// the storage framework should set FSType but doesn't.
+	if source.CSI != nil &&
+		source.CSI.FSType == nil &&
+		fstype != "" {
+		source.CSI.FSType = &fstype
+	}
+
 	pod := CreatePod(f, "dax-volume-test", volumeMode, source, config, withKataContainers)
 	defer func() {
 		DeletePod(f, pod)
 	}()
-	checkWithNormalRuntime := testDax(f, pod, root, volumeMode, source, withKataContainers, expectDax)
+	checkWithNormalRuntime := testDax(f, pod, root, volumeMode, source, withKataContainers, expectDax, fstype)
 	DeletePod(f, pod)
 	if checkWithNormalRuntime {
 		testDaxOutside(f, pod, root)
@@ -336,6 +345,7 @@ func testDax(
 	source *v1.VolumeSource,
 	withKataContainers bool,
 	expectDax bool,
+	fstype string,
 ) bool {
 	ns := f.Namespace.Name
 	containerName := pod.Spec.Containers[0].Name
@@ -351,6 +361,12 @@ func testDax(
 	if expectDax {
 		By("checking volume for DAX support")
 		pmempod.RunInPod(f, root, nil, "lsblk; mount | grep /mnt; /usr/local/bin/pmem-dax-check /mnt/daxtest", ns, pod.Name, containerName)
+
+		if fstype == "xfs" {
+			By("checking volume for extsize 2m")
+			// "xfs_io -c extsize" prints "[2097152] /mnt".
+			pmempod.RunInPod(f, root, nil, "xfs_io -c extsize /mnt | tee /dev/stderr | grep -q -w 2097152", ns, pod.Name, containerName)
+		}
 	} else {
 		By("checking volume for missing DAX support")
 		stdout, _ := pmempod.RunInPod(f, root, nil, "ndctl list -NR; lsblk; mount | grep /mnt; /usr/local/bin/pmem-dax-check /mnt/daxtest; if [ $? -ne 1 ]; then echo should have reported missing DAX >&2; exit 1; fi", ns, pod.Name, containerName)