Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
snap: auto mount block devices and import assertions #2047
Conversation
mvo5
added some commits
Sep 30, 2016
| + "github.com/snapcore/snapd/osutil" | ||
| +) | ||
| + | ||
| +const autoImportsName = "auto-imports.asserts" |
pedronis
Sep 30, 2016
Contributor
snap download uses ".assertions" not ".asserts" as extensions for a file full of assertions, it would be good if we were consistent (I prefer the longer extension personally but consistency is more important, also don't know how much people are relying on snap download assertion bits, I know I mentioned it to some people)
niemeyer
Sep 30, 2016
Contributor
I don't like cutting words in general either, but at the same time I find "assertions" slightly too long a word for a file extension. The pair:
spotify_10.0_amd64.snap
spotify_10.0_amd64.asserts
Feels slightly nicer than
spotify_10.0_amd64.snap
spotify_10.0_amd64.assertions
Very subjective, though, arguably.
niemeyer
Sep 30, 2016
Contributor
Or perhaps even:
spotify_10.0_amd64.snap
spotify_10.0_amd64.assert
?
pedronis
Sep 30, 2016
Contributor
I think at the moment is actually "spotify_10.0_amd64.snap.assertions"
anyway I think I prefer ".assert" over ".asserts" if we want to go short
mvo5
referenced this pull request
Oct 2, 2016
Closed
snap: auto-import assertions from block devices #2044
mvo5
added some commits
Oct 2, 2016
| + "github.com/snapcore/snapd/osutil" | ||
| +) | ||
| + | ||
| +const autoImportsName = "auto-imports.assert" |
| + } | ||
| + // not using syscall.Mount() because we don't know the fs type in | ||
| + // advance | ||
| + cmd := exec.Command("mount", "-o", "ro", "--make-private", deviceName, tmpMountTarget) |
zyga
Oct 3, 2016
Contributor
Can you please document why you use --make-private here?
I'm not 100% sure with regular mounts but with bind mounts you do have to do the mount in one operation and then the --make-private with another operation or the results are actually different. Perhaps mount handles this internally (I work more with regular syscall).
niemeyer
Oct 3, 2016
Contributor
As I understand it the point of --make-private is that we don't want anyone else to see this operation. We'll mount, have a look, and unmount, privately.
| + cmd := exec.Command("mount", "-o", "ro", "--make-private", deviceName, tmpMountTarget) | ||
| + if output, err := cmd.CombinedOutput(); err != nil { | ||
| + msg := fmt.Sprintf("cannot mount %q: %s", deviceName, osutil.OutputErr(output, err)) | ||
| + logger.Panicf(msg) |
zyga
Oct 3, 2016
Contributor
Are you sure we want to panic? There can be any number of valid reasons for which this can fail.
| @@ -0,0 +1,3 @@ | ||
| +# probe for assertions, must run before udisks2 | ||
| +ACTION=="add", SUBSYSTEM=="block" \ | ||
| + RUN+="/usr/bin/unshare -m /usr/bin/snap auto-import /dev/%k" |
zyga
Oct 3, 2016
Contributor
You can perhaps add --propagation private here. It is default but this may change later and it would be very explicit as to what we are doing.
| + "github.com/snapcore/snapd/osutil" | ||
| +) | ||
| + | ||
| +const autoImportsName = "auto-imports.assert" |
niemeyer
Oct 3, 2016
Contributor
Oops.. minor typo here (filename is auto-import.assert).
Will fix after merging.
mvo5 commentedSep 30, 2016
Based on https://github.com/snapcore/snapd/pull/2044/files
This works by putting the udev rule for snapd.autoimport.rules before the udisks2 rules. This way we can mount/peek/umount before the udisks2 mounts the device. And if no udisks2 is installed we still DTRT.