Skip to content

Commit

Permalink
Removed nobarrier mount flag from xfs driver
Browse files Browse the repository at this point in the history
nobarrier has been deprecated since kernel 4.10 and has no effect at
all.

[#163617086]
  • Loading branch information
yulianedyalkova committed Feb 19, 2019
1 parent 3c0fa49 commit 9dbb3c9
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 25 deletions.
2 changes: 1 addition & 1 deletion ci/scripts/test/utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ mount_storage() {

# Mount XFS
mkdir /mnt/xfs-${i}
mount -t xfs -o pquota,noatime,nobarrier /xfs_volume_${i} /mnt/xfs-${i}
mount -t xfs -o pquota,noatime /xfs_volume_${i} /mnt/xfs-${i}
chmod 777 -R /mnt/xfs-${i}
done
}
Expand Down
2 changes: 1 addition & 1 deletion hack/quick-setup
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ make_backing_store() {

mount_store() {
mkdir -p /var/lib/grootfs/xfs
mount -t xfs -o pquota,noatime,nobarrier /var/lib/grootfs.xfs.backing-store /var/lib/grootfs/xfs
mount -t xfs -o pquota,noatime /var/lib/grootfs.xfs.backing-store /var/lib/grootfs/xfs
chmod 777 /var/lib/grootfs/xfs
}

Expand Down
5 changes: 3 additions & 2 deletions store/filesystems/overlayxfs/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (d *Driver) ValidateFileSystem(logger lager.Logger, path string) error {
logger.Debug("starting")
defer logger.Debug("ending")

if err := filesystems.CheckFSPath(path, "xfs", "noatime", "nobarrier", "prjquota"); err != nil {
if err := filesystems.CheckFSPath(path, "xfs", "noatime", "prjquota"); err != nil {
return errorspkg.Wrap(err, "overlay-xfs filesystem validation")
}

Expand Down Expand Up @@ -421,7 +421,8 @@ func (d *Driver) formatFilesystem(logger lager.Logger, filesystemPath string) er
}

func (d *Driver) mountFilesystem(source, destination, option string) error {
allOpts := strings.Trim(fmt.Sprintf("%s,loop,pquota,noatime,nobarrier", option), ",")
allOpts := strings.Trim(fmt.Sprintf("%s,loop,pquota,noatime", option), ",")

cmd := exec.Command("mount", "-o", allOpts, "-t", "xfs", source, destination)
if output, err := cmd.CombinedOutput(); err != nil {
return errorspkg.Errorf("%s: %s", err, string(output))
Expand Down
49 changes: 28 additions & 21 deletions store/filesystems/overlayxfs/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ import (
. "github.com/st3v/glager"
)

const (
_ = iota
kb int64 = 1 << (10 * iota)
mb
gb
)

var _ = Describe("Driver", func() {
var (
storePath string
Expand Down Expand Up @@ -82,7 +89,7 @@ var _ = Describe("Driver", func() {
tempFile, err := ioutil.TempFile("", "xfs-filesystem")
Expect(err).NotTo(HaveOccurred())
fsFile = tempFile.Name()
Expect(os.Truncate(fsFile, 1024*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, gb)).To(Succeed())

storePath, err = ioutil.TempDir("", "store")
Expect(err).NotTo(HaveOccurred())
Expand All @@ -104,7 +111,7 @@ var _ = Describe("Driver", func() {
mountinfo, err := ioutil.ReadFile("/proc/self/mountinfo")
Expect(err).NotTo(HaveOccurred())

Expect(string(mountinfo)).To(MatchRegexp(fmt.Sprintf("%s[^\n]*noatime[^\n]*nobarrier[^\n]*prjquota", storePath)))
Expect(string(mountinfo)).To(MatchRegexp(fmt.Sprintf("%s[^\n]*noatime[^\n]*prjquota", storePath)))
})

Context("when creating the filesystem fails", func() {
Expand All @@ -117,7 +124,7 @@ var _ = Describe("Driver", func() {
Context("when the filesystem is already formatted", func() {
BeforeEach(func() {
cmd := exec.Command("mkfs.xfs", "-f", fsFile)
Expect(os.Truncate(fsFile, 200*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, 200*mb)).To(Succeed())
Expect(cmd.Run()).To(Succeed())
})

Expand All @@ -128,10 +135,10 @@ var _ = Describe("Driver", func() {

Context("when the store is already mounted", func() {
BeforeEach(func() {
Expect(os.Truncate(fsFile, 200*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, 200*mb)).To(Succeed())
cmd := exec.Command("mkfs.xfs", "-f", fsFile)
Expect(cmd.Run()).To(Succeed())
cmd = exec.Command("mount", "-o", "loop,pquota,noatime,nobarrier", "-t", "xfs", fsFile, storePath)
cmd = exec.Command("mount", "-o", "loop,pquota,noatime", "-t", "xfs", fsFile, storePath)
Expect(cmd.Run()).To(Succeed())
})

Expand All @@ -155,7 +162,7 @@ var _ = Describe("Driver", func() {
tempFile, err := ioutil.TempFile("", "xfs-filesystem")
Expect(err).NotTo(HaveOccurred())
fsFile = tempFile.Name()
Expect(os.Truncate(fsFile, 1024*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, gb)).To(Succeed())
cmd := exec.Command("mkfs.xfs", "-f", fsFile)
Expect(cmd.Run()).To(Succeed())

Expand All @@ -179,15 +186,15 @@ var _ = Describe("Driver", func() {
mountinfo, err := ioutil.ReadFile("/proc/self/mountinfo")
Expect(err).NotTo(HaveOccurred())

Expect(string(mountinfo)).To(MatchRegexp(fmt.Sprintf("%s[^\n]*noatime[^\n]*nobarrier[^\n]*prjquota", storePath)))
Expect(string(mountinfo)).To(MatchRegexp(fmt.Sprintf("%s[^\n]*noatime[^\n]*prjquota", storePath)))
})

Context("when the store is already mounted", func() {
BeforeEach(func() {
Expect(os.Truncate(fsFile, 200*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, 200*mb)).To(Succeed())
cmd := exec.Command("mkfs.xfs", "-f", fsFile)
Expect(cmd.Run()).To(Succeed())
cmd = exec.Command("mount", "-o", "loop,pquota,noatime,nobarrier", "-t", "xfs", fsFile, storePath)
cmd = exec.Command("mount", "-o", "loop,pquota,noatime", "-t", "xfs", fsFile, storePath)
Expect(cmd.Run()).To(Succeed())
})

Expand All @@ -211,7 +218,7 @@ var _ = Describe("Driver", func() {
tempFile, err := ioutil.TempFile("", "xfs-filesystem")
Expect(err).NotTo(HaveOccurred())
fsFile = tempFile.Name()
Expect(os.Truncate(fsFile, 1024*1024*1024)).To(Succeed())
Expect(os.Truncate(fsFile, gb)).To(Succeed())

deinitStorePath, err = ioutil.TempDir("", "store")
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -451,7 +458,7 @@ var _ = Describe("Driver", func() {

Context("when disk limit is > 0", func() {
BeforeEach(func() {
spec.DiskLimit = 1024 * 1024 * 10
spec.DiskLimit = 10 * mb
Expect(ioutil.WriteFile(filepath.Join(storePath, store.MetaDirName, fmt.Sprintf("volume-%s", layer1ID)), []byte(`{"Size": 3145728}`), 0644)).To(Succeed())
})

Expand Down Expand Up @@ -526,7 +533,7 @@ var _ = Describe("Driver", func() {
_, err := driver.CreateImage(logger, spec)
Expect(err).ToNot(HaveOccurred())

ensureQuotaMatches(filepath.Join(spec.ImagePath, "image_quota"), 1024*1024*10-3145728)
ensureQuotaMatches(filepath.Join(spec.ImagePath, "image_quota"), 10*mb-3145728)
})

Context("when the DiskLimit is smaller than VolumeSize", func() {
Expand All @@ -539,12 +546,12 @@ var _ = Describe("Driver", func() {

Context("when the DiskLimit is less than the minimum quota (1024*256 bytes) after accounting for the VolumeSize", func() {
BeforeEach(func() {
volumeSize := int64(1024 * 128)
volumeSize := int64(128 * kb)
layerID := randVolumeID()
_ = createVolume(storePath, driver, "parent-id", layerID, volumeSize)

spec.BaseVolumeIDs = []string{layerID}
spec.DiskLimit = volumeSize + (1024 * 128)
spec.DiskLimit = volumeSize + (128 * kb)
})

It("enforces the minimum required quota in the image", func() {
Expand Down Expand Up @@ -573,7 +580,7 @@ var _ = Describe("Driver", func() {

Context("when the DiskLimit is smaller than VolumeSize", func() {
It("succeeds", func() {
spec.DiskLimit = 1024 * 1024 * 3
spec.DiskLimit = 3 * mb
_, err := driver.CreateImage(logger, spec)
Expect(err).ToNot(HaveOccurred())
})
Expand Down Expand Up @@ -601,7 +608,7 @@ var _ = Describe("Driver", func() {
_, err := driver.CreateImage(logger, spec)
Expect(err).ToNot(HaveOccurred())

ensureQuotaMatches(filepath.Join(spec.ImagePath, "image_quota"), 1024*1024*10)
ensureQuotaMatches(filepath.Join(spec.ImagePath, "image_quota"), 10*mb)
})
})

Expand Down Expand Up @@ -755,7 +762,7 @@ var _ = Describe("Driver", func() {
createVolume(storePath, driver, "parent-id", volumeID, 3000000)

spec.BaseVolumeIDs = []string{volumeID}
spec.DiskLimit = 10 * 1024 * 1024
spec.DiskLimit = 10 * mb
_, err := driver.CreateImage(logger, spec)
Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -997,7 +1004,7 @@ var _ = Describe("Driver", func() {
volumePath, err = driver.CreateVolume(logger, "", volumeID)
Expect(err).NotTo(HaveOccurred())

Expect(driver.WriteVolumeMeta(logger, volumeID, base_image_puller.VolumeMeta{Size: 1024})).To(Succeed())
Expect(driver.WriteVolumeMeta(logger, volumeID, base_image_puller.VolumeMeta{Size: kb})).To(Succeed())
})

It("Renames the volume directory", func() {
Expand Down Expand Up @@ -1274,7 +1281,7 @@ var _ = Describe("Driver", func() {

Describe("WriteVolumeMeta", func() {
It("creates the correct metadata file", func() {
err := driver.WriteVolumeMeta(logger, "1234", base_image_puller.VolumeMeta{Size: 1024})
err := driver.WriteVolumeMeta(logger, "1234", base_image_puller.VolumeMeta{Size: kb})
Expect(err).NotTo(HaveOccurred())

metaFilePath := filepath.Join(storePath, store.MetaDirName, "volume-1234")
Expand All @@ -1284,7 +1291,7 @@ var _ = Describe("Driver", func() {
var meta base_image_puller.VolumeMeta

Expect(json.NewDecoder(metaFile).Decode(&meta)).To(Succeed())
Expect(meta).To(Equal(base_image_puller.VolumeMeta{Size: 1024}))
Expect(meta).To(Equal(base_image_puller.VolumeMeta{Size: kb}))
})
})

Expand Down Expand Up @@ -1404,7 +1411,7 @@ func createVolume(storePath string, driver *overlayxfs.Driver, parentID, id stri
return path
}

func ensureQuotaMatches(fileName string, expectedQuota int) {
func ensureQuotaMatches(fileName string, expectedQuota int64) {
Expect(fileName).To(BeAnExistingFile())

contents, err := ioutil.ReadFile(fileName)
Expand Down

0 comments on commit 9dbb3c9

Please sign in to comment.