Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

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: #1058
(cherry picked from commit 4f1a2d8)
  • Loading branch information
pohly committed Jan 12, 2022
1 parent eaac130 commit fb0e4f6
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 13 deletions.
15 changes: 4 additions & 11 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 @@ -290,12 +291,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())
}
}
Expand Down Expand Up @@ -581,11 +577,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())
}
}
Expand Down
56 changes: 56 additions & 0 deletions pkg/xfs/xfs.go
Original file line number Diff line number Diff line change
@@ -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
}
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_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)
}
19 changes: 17 additions & 2 deletions test/e2e/storage/dax/dax.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,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)
testDaxInPod(f, l.root, l.resource.Pattern.VolMode, l.resource.VolSource, l.config, withKataContainers, l.resource.Pattern.FsType)
})
}

Expand All @@ -140,6 +140,7 @@ func testDaxInPod(
source *v1.VolumeSource,
config *storageframework.PerTestConfig,
withKataContainers bool,
fstype string,
) {
expectDax := true
if withKataContainers {
Expand All @@ -156,11 +157,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 @@ -341,6 +350,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 @@ -356,6 +366,11 @@ func testDax(
if expectDax {
By("checking volume for DAX support")
pmempod.RunInPod(f, root, []string{daxCheckBinary}, "lsblk; mount | grep /mnt; /tmp/"+path.Base(daxCheckBinary)+" /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")
pmempod.RunInPod(f, root, []string{daxCheckBinary}, "lsblk; mount | grep /mnt; /tmp/"+path.Base(daxCheckBinary)+" /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 fb0e4f6

Please sign in to comment.