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

lxc-test-unpriv: fix the overlayfs mount error #1908

Merged
merged 1 commit into from Nov 9, 2017

Conversation

4 participants
@Cypresslin

Cypresslin commented Nov 8, 2017

This patch fixes the missing workdir issue for the overlayfs mount command in
the lxc-test-unpriv test.

Bug link: https://bugs.launchpad.net/bugs/1730915

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

Po-Hsu Lin
lxc-test-unpriv: fix the overlayfs mount error
This patch fixes the missing workdir issue for the overlayfs mount command in
the lxc-test-unpriv test.

Bug link: https://bugs.launchpad.net/bugs/1730915

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
@lxc-jenkins

This comment has been minimized.

Show comment
Hide comment
@lxc-jenkins

lxc-jenkins Nov 8, 2017

This pull request didn't trigger Jenkins as its author isn't in the whitelist.

An organization member must perform one of the following:

  • To have this branch tested by Jenkins, use the "ok to test" command.
  • To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

lxc-jenkins commented Nov 8, 2017

This pull request didn't trigger Jenkins as its author isn't in the whitelist.

An organization member must perform one of the following:

  • To have this branch tested by Jenkins, use the "ok to test" command.
  • To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

@brauner

This comment has been minimized.

Show comment
Hide comment
@brauner

brauner Nov 8, 2017

Member

jenkins: test this please

Member

brauner commented Nov 8, 2017

jenkins: test this please

mkdir "${MOUNTDIR}/lowerdir" "${MOUNTDIR}/upperdir" "${MOUNTDIR}/overlayfs"
mount -t overlayfs -o lowerdir="${MOUNTDIR}/lowerdir",upperdir="${MOUNTDIR}/upperdir" none "${MOUNTDIR}/overlayfs"
mkdir ${MOUNTDIR}/{lowerdir,upperdir,workdir,overlayfs}
mount -t overlayfs -o lowerdir="${MOUNTDIR}/lowerdir",upperdir="${MOUNTDIR}/upperdir",workdir="${MOUNTDIR}/workdir" none "${MOUNTDIR}/overlayfs"

This comment has been minimized.

@brauner

brauner Nov 8, 2017

Member

Thanks for the patch. One nit though. What happens on older kernels whose overlay module does not support the workdir option? Should the logic be:

  • try with workdir option first and if that fails try without the workdir option?
@brauner

brauner Nov 8, 2017

Member

Thanks for the patch. One nit though. What happens on older kernels whose overlay module does not support the workdir option? Should the logic be:

  • try with workdir option first and if that fails try without the workdir option?

This comment has been minimized.

@stgraber

stgraber Nov 8, 2017

Member

Yeah, looks like this is going to break older kernels, just a reminder that LXC 2.0.x still supports kernels from 2.6.32 onwards so this does need fixing as the ultimate target for this fix is 2.0.x

@stgraber

stgraber Nov 8, 2017

Member

Yeah, looks like this is going to break older kernels, just a reminder that LXC 2.0.x still supports kernels from 2.6.32 onwards so this does need fixing as the ultimate target for this fix is 2.0.x

This comment has been minimized.

@Cypresslin

Cypresslin Nov 9, 2017

Hi, thanks for catching this.

To this test itself, the script will skip this overlayfs test part on older kernels (tested on Trusty 3.13, which needs the overlayfs module) with an if statement:

if modprobe -q overlayfs; then

So I think as long as this module check criteria is valid, this change should be fine?

@Cypresslin

Cypresslin Nov 9, 2017

Hi, thanks for catching this.

To this test itself, the script will skip this overlayfs test part on older kernels (tested on Trusty 3.13, which needs the overlayfs module) with an if statement:

if modprobe -q overlayfs; then

So I think as long as this module check criteria is valid, this change should be fine?

This comment has been minimized.

@brauner

brauner Nov 9, 2017

Member

Ok, cool.

@brauner

brauner Nov 9, 2017

Member

Ok, cool.

@brauner brauner merged commit d4624e9 into lxc:master Nov 9, 2017

4 checks passed

Branch target Branch target is correct
Details
Signed-off-by All commits signed-off
Details
Testsuite Testsuite passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment