Skip to content
This repository was archived by the owner on Aug 14, 2020. It is now read-only.

ace/validator: fix mountpoint checks #467

Merged
merged 1 commit into from
Aug 8, 2015

Conversation

iaguis
Copy link
Member

@iaguis iaguis commented Aug 3, 2015

This includes:

  • A proper check for Linux. The previous one was just checking if the
    devices were the same, this is not true for bind-mounts within the
    same device
  • A check for FreeBSD courtesy of Maciej Pasternacki
    [email protected]. Jails seem to have f_fsid set to zero so the
    original check didn't work. Now we check r_fstypename, f_mntfromname
    and f_mntonname too.
  • As a fallback (for other systems) we do the same as the original code:
    just check the f_fsid.

Fixes #464
cc @mpasternacki

@iaguis iaguis force-pushed the iaguis/check-mount branch 2 times, most recently from b1c22f1 to c1a1116 Compare August 3, 2015 16:40
@iaguis iaguis mentioned this pull request Aug 4, 2015
7 tasks
@iaguis iaguis force-pushed the iaguis/check-mount branch from c1a1116 to 69e5009 Compare August 5, 2015 08:03
@mpasternacki
Copy link
Contributor

After adding the missing import "fmt" in os_shared.go, works perfectly for me. 👍

@iaguis
Copy link
Member Author

iaguis commented Aug 6, 2015

Ooops, thanks for catching that!

@iaguis iaguis force-pushed the iaguis/check-mount branch from 69e5009 to afaa6ae Compare August 6, 2015 11:11
mpasternacki added a commit to 3ofcoins/jetpack that referenced this pull request Aug 6, 2015
root string
mountPoint string
mountOptions string
optionalFields string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to proc(5):

(7) optional fields: zero or more fields of the form "tag[:value]".

Does the Scanf work if there are several fields? It would be good to add some unit tests about that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine because we only care about (5) and (6) but for that same reason we can only parse the first 6 fields (which are fixed) and avoid possible problems. I'll update the PR with that and some unit tests. Thanks!

@alban
Copy link
Member

alban commented Aug 7, 2015

Except the issue with parsing optionalFields, it looks good.

This includes:

- A proper check for Linux. The previous one was just checking if the
  devices were the same, this is not true for bind-mounts within the
  same device

- A check for FreeBSD courtesy of Maciej Pasternacki
  <[email protected]>. Jails seem to have f_fsid set to zero so the
  original check didn't work. Now we check f_fstypename, f_mntfromname
  and f_mntonname too.

- As a fallback (for other systems) we do the same as the original code:
  just check the f_fsid.
@iaguis iaguis force-pushed the iaguis/check-mount branch from afaa6ae to 8962271 Compare August 7, 2015 14:20
@iaguis
Copy link
Member Author

iaguis commented Aug 7, 2015

Updated with only parsing the first 6 fields and added a test

@jonboulle
Copy link
Contributor

👍 thanks

jonboulle added a commit that referenced this pull request Aug 8, 2015
ace/validator: fix mountpoint checks
@jonboulle jonboulle merged commit e2073b6 into appc:master Aug 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants