Skip to content

Commit

Permalink
XFS: fix creating volumes on OpenShift
Browse files Browse the repository at this point in the history
On OpenShift, the xfs_io command that was used while creating XFS volumes hangs
while getting loaded (kernel or container runtime bug?). This problem can be
avoided by making the same ioctl calls as in that binary directly from
the PMEM-CSI driver.

Fixes: intel#1058
  • Loading branch information
pohly committed Jan 2, 2022
1 parent 70ecb2b commit 975ef5a
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 6 deletions.
7 changes: 3 additions & 4 deletions pkg/pmem-csi-driver/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -289,8 +290,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
// 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.SetXFSExtsize(hostMount); err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
}
Expand Down Expand Up @@ -590,8 +590,7 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
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 err := xfs.SetXFSExtsize(stagingtargetPath); err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/xfs/xfs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
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"
)

// SetXFSExtsize sets the inherit flag for extsize and the 2M extsize that is
// needed for fsdax huge pages on XFS.
func SetXFSExtsize(path string) error {
file, err := os.Open(path)
if err != nil {
return fmt.Errorf("open %q: %v", path, err)
}
defer file.Close()
fd := C.int(file.Fd())

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))
}

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
}
21 changes: 21 additions & 0 deletions pkg/xfs/xfs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
Copyright 2022 Intel Corporation
SPDX-License-Identifier: Apache-2.0
*/

package xfs

import (
"testing"
)

func Test_SetXFSExtsize(t *testing.T) {
// This is assumed to be backed by tmpfs and thus doesn't support xattr.
tmp := t.TempDir()
err := SetXFSExtsize(tmp)
if err == nil {
t.Fatal("did not get expected error")
}
t.Logf("got expected error: %v", err)
}
20 changes: 18 additions & 2 deletions test/e2e/storage/dax/dax.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand All @@ -135,6 +135,7 @@ func testDaxInPod(
config *storageframework.PerTestConfig,
withKataContainers bool,
daxSupported bool,
fstype string,
) {
expectDax := daxSupported
if withKataContainers {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 975ef5a

Please sign in to comment.