Let mkfs.ext3 use the image file directly instead of loopback device #481

wpoely86 · 2017-02-09T20:00:21Z

By avoiding the loopback device, we do not need root priviledges to create an
image. We give mkfs.ext3 an offset and image size.

This is NOT ready: it's only meant to show that it is possible.

@singularityware-admin

By avoiding the loopback device, we do not need root priviledges to create an image. We give mkfs.ext3 an offset and image size.

wpoely86 · 2017-02-09T20:05:19Z

The failing test is normal, I haven't adjusted the build system.

wpoely86 · 2017-02-09T20:11:32Z

OK, this only works with E2fsprogs 1.42.10 (May 18, 2014) and newer.

gmkurtzer · 2017-02-10T00:49:01Z

I would support making this an optional pathway.... Probably makes the most sense to check for this feature in configure.ac and then conditionalize the SUID generation. Let me know if you need a hand.

According to the man page of waitpid, you can only call WEXITSTATUS after you have checked WIFEXITED.

wpoely86 · 2017-02-13T15:33:23Z

Okay, it took some effort but it finally works. There was an error in the overlay causing the tests to fail that I also fixed.

* origin/lib-refactor: Added copy ability to run as non-root Added src/copy to gitignore Added copy functionality Adding debugging when forking Adding more tests Add create final mount directory when overlayfs is not enabled Support multiple allowed container owners (list) Return if binary is running suid (enabled?) Add ability to limit containers that can run according to owner Removed unused privleged mode code (for now...) Minor fix to make sure that singularity.conf is always updated Add selftest abilities... needs to be expanded upon Include MS_NODEV in image mount Merge contributing clause back into license as it should be

gmkurtzer · 2017-02-13T17:16:35Z

configure.ac

@@ -274,12 +274,25 @@ AC_ARG_ENABLE([suid],

 AS_IF([test "x$enable_suid" != "xno"], [
    AC_MSG_RESULT([yes])
-    BUILD_SUID="action-suid create-suid copy-suid expand-suid export-suid import-suid mount-suid"
+    BUILD_SUID="action-suid copy-suid expand-suid export-suid import-suid mount-suid"


Can you confirm that if the below check for new enough mkfs.ext3 fails, we need to add create-suid back into the BUILD_SUID list.

That happens on Line 291: it $BUILD_SUID is non-empty, we add create-suid to the list:

AS_IF([test -n "$BUILD_SUID"], [BUILD_SUID="$BUILD_SUID create-suid"], [])

You can check by looking at the build on Travis: it's uses an old version of mkfs.ext3 so it needs SUID rights to work.

Ahh, right you are. Sorry for missing that!

wpoely86 · 2017-02-13T19:08:34Z

Btw, @gmkurtzer is there a specific reason to choose ext3 over ext4?

gmkurtzer · 2017-02-13T19:12:33Z

Yep, ext4 versions are not as stable as ext3 (stable in the sense that an ext4 system created on latest Fedora won't work with RHEL5/6).

gmkurtzer · 2017-03-09T22:32:25Z

src/create.c

+    mkfs_cmd[3] = strjoin("offset=", int2str(strlength(LAUNCH_STRING, 1024)));
+    mkfs_cmd[4] = strdup(image.path);
+    // pass the correct size of the file in KiB
+    mkfs_cmd[5] = int2str((size*1024*1024-strlength(LAUNCH_STRING, 1024))/1024);


Is this necessary?

If you don't pass it, you get (even with -q):

Warning: offset specified without an explicit file system size. Creating a file system with 131072 blocks but this might not be what you want.

So I think it's safer to add it.

* origin/lib-refactor: (125 commits) Minor changes for apptainer#508 Fix tests to match ordering Fix double dots Implement read-only bind mounts to close apptainer#451 Fixing the API for the changes I made. ooopsie. Add support for %environment (apptainer#504) Minor environment file updates! remove tmpfile Don't do recursive chmod here Fix permissions to remove files properly re-add chmod to make things writable again fix import to properly error out we should not call add for user import, all calls go to import.py removing script that doesn't exist anymore updating docker/python functions to not need ADD Fix import.exec for new python interface Updates for new python interface tweaking variable name to SINGULARITY_CONTENTS instead of layerfile Updates for newest python API adding new python files ...

Currently `singularity -d export image.iso > image.tar` will not work as `-d` makes singularity print to stdout

wpoely86 · 2017-03-11T19:40:05Z

@gmkurtzer I just noticed that:

singularity -d export image.iso > image.tar

doesn't work. The first debug output of singularity goes to stdout instead of stderr. I think the only clean way of solving this is not to use the redirect and do something like:

singularity -d export image.iso image.tar

* origin/development: (67 commits) Clean environment when using -C/--containall flags Add -e/--cleanenv runtime action option (ref: apptainer#445) Moved test Cleaned up tests and added more docker tests Fix shell debug/verbose messages to goto STDERR (ref apptainer#481) fix: removed tabs fix: removed driver if added test fix syntax Testing a wider print format Minor fix for CI Clean redundant messages of label additions Support optional tests Make container writable for removal test Added config ownership test Fix comments and empty lines in the %file sections (and add tests) Finishing up the new test code Fixes for runscript and environment overriding what is in the sections and python Test updates... WIP Fix %labels parsing during bootstrap ...

wpoely86 · 2017-03-20T08:22:23Z

@gmkurtzer I've changed the target to the development branch.

gmkurtzer · 2017-03-20T16:01:13Z

Thanks for the merge with the latest "development" branch code.

So I am now on the fence on this PR as the 2.3 release will be able to create images without sudo already without this:

[gmk@centos7-x64 ~]$ whoami
gmk
[gmk@centos7-x64 ~]$ singularity create /tmp/gmk-test.img
Initializing Singularity image subsystem
Opening image file: gmk-test.img
Creating 768MiB image
Binding image to loop
Creating file system within image
Image is done: /tmp/gmk-test.img

So the main value add is that we can do it without a SUID pathway, but it also creates an alternate pathway that can end up making things more complicated to debug and maintain.

Considering we can do this now, do you still see a strong need for this PR?

wpoely86 · 2017-03-20T21:05:39Z

Strong case, probably not. It's nice to not need root nor suid for a simple thing as creating an image but for the end-user this makes no difference.

I do think the overhead of the extra path is minimal: it's all contained in a single file as ifdefs.

gmkurtzer · 2017-10-04T00:52:15Z

As of the 2.4 beta release, this is no longer necessary. We don't use the loop device anymore to create images. Thanks!

Let mkfs.ext3 use the image file directly instead of loopback

7d3d6b0

By avoiding the loopback device, we do not need root priviledges to create an image. We give mkfs.ext3 an offset and image size.

wpoely86 added 9 commits February 13, 2017 11:43

Use offset parameter of mkfs.ext3 if it is supported

e123130

Drop output of dd in configure

74a394a

Travis: show output of failed tests

1218254

Check exit status of mkfs.ext3 in create

977b81d

Only use WEXITSTATUS after checking WIFEXITED

4c56628

According to the man page of waitpid, you can only call WEXITSTATUS after you have checked WIFEXITED.

Make sure final mount point exists

0c29d2a

Always enable --debug when running make test

260e5fc

Add forgotten changes from last commit

6823351

Correct for dropped privileges

a97f774

gmkurtzer reviewed Feb 13, 2017

View reviewed changes

gmkurtzer reviewed Mar 9, 2017

View reviewed changes

wpoely86 added 2 commits March 11, 2017 14:08

Fix two errors because of merging upstream lib-refactor

dd72de4

Currently `singularity -d export image.iso > image.tar` will not work as `-d` makes singularity print to stdout

gmkurtzer added a commit that referenced this pull request Mar 20, 2017

Fix shell debug/verbose messages to goto STDERR (ref #481)

0d60d41

wpoely86 changed the base branch from lib-refactor to development March 20, 2017 08:05

Restore permission

3b25abc

gmkurtzer closed this Oct 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let mkfs.ext3 use the image file directly instead of loopback device #481

Let mkfs.ext3 use the image file directly instead of loopback device #481

wpoely86 commented Feb 9, 2017

wpoely86 commented Feb 9, 2017

wpoely86 commented Feb 9, 2017

gmkurtzer commented Feb 10, 2017

wpoely86 commented Feb 13, 2017

gmkurtzer Feb 13, 2017

wpoely86 Feb 13, 2017

wpoely86 Feb 13, 2017

gmkurtzer Feb 13, 2017

wpoely86 commented Feb 13, 2017

gmkurtzer commented Feb 13, 2017

gmkurtzer Mar 9, 2017

wpoely86 Mar 11, 2017

wpoely86 commented Mar 11, 2017

wpoely86 commented Mar 20, 2017

gmkurtzer commented Mar 20, 2017

wpoely86 commented Mar 20, 2017

gmkurtzer commented Oct 4, 2017

Let mkfs.ext3 use the image file directly instead of loopback device #481

Let mkfs.ext3 use the image file directly instead of loopback device #481

Conversation

wpoely86 commented Feb 9, 2017

wpoely86 commented Feb 9, 2017

wpoely86 commented Feb 9, 2017

gmkurtzer commented Feb 10, 2017

wpoely86 commented Feb 13, 2017

gmkurtzer Feb 13, 2017

Choose a reason for hiding this comment

wpoely86 Feb 13, 2017

Choose a reason for hiding this comment

wpoely86 Feb 13, 2017

Choose a reason for hiding this comment

gmkurtzer Feb 13, 2017

Choose a reason for hiding this comment

wpoely86 commented Feb 13, 2017

gmkurtzer commented Feb 13, 2017

gmkurtzer Mar 9, 2017

Choose a reason for hiding this comment

wpoely86 Mar 11, 2017

Choose a reason for hiding this comment

wpoely86 commented Mar 11, 2017

wpoely86 commented Mar 20, 2017

gmkurtzer commented Mar 20, 2017

wpoely86 commented Mar 20, 2017

gmkurtzer commented Oct 4, 2017