github / libprojfs

Linux projected filesystem library
GNU Lesser General Public License v2.1
93 stars 14 forks source link

test that we actually lock dirs on projection #51

Closed kivikakk closed 5 years ago

kivikakk commented 5 years ago

Fixes #49.

kivikakk commented 5 years ago

Applying the following diff produces the following output:

diff --git a/lib/projfs.c b/lib/projfs.c
index 914bf52..6d3d030 100644
--- a/lib/projfs.c
+++ b/lib/projfs.c
@@ -196,7 +196,8 @@ static int get_path_userdata(struct node_userdata *user,
        wait_ms = PROJ_WAIT_MSEC;

 retry_flock:
-       err = flock(user->fd, LOCK_EX | LOCK_NB);
+       // err = flock(user->fd, LOCK_EX | LOCK_NB);
+       err = 0;
        if (err == -1) {
                if (errno == EWOULDBLOCK && wait_ms > 0) {
                        /* sleep 100ms, retry */
$ ./t205-event-locking.t -x -i
expecting success:
        projfs_run_twice ls target

+ projfs_run_twice ls target
+ pidA=45704
+ + ls target
pidB=45705
+ ret=0
+ wait 45704
+ ls target
ls: cannot access 'target': File exists
+ wait 45705
+ ret=1
+ return 1
error: last command exited with $?=1
not ok 1 - test concurrent access does not trigger failure
#
#               projfs_run_twice ls target
#
$

The provider attempts to create the lockfile when one already exists, yielding EEXIST.

chrisd8088 commented 5 years ago

Hey @kivikakk, thank you, first of all! I love the run-twice tests and supporting code! 👍 👍

I only have a couple of minor suggestions and one request, and then please merge away!

I was a bit slow responding to this PR because it inspired all kinds of ideas for me, which I pursued for a while in test-locking-errs, principally with the goal of triggering a "real" timeout from flock(). I ended up following a long-simmering plan to start getting away from using shell commands (like mkdir, touch, etc.) because their output may vary between versions, as we discovered with stat.

But on late reflection I think I'm not able right now to decide what's good or not in the stuff I wrote, and I'd like to come back to it refreshed next week and cherry-pick or rework some of it. Apologies, and have a great weekend!

Chris.

kivikakk commented 5 years ago

I ended up following a long-simmering plan to start getting away from using shell commands (like mkdir, touch, etc.) because their output may vary between versions, as we discovered with stat.

I think this could be really good! It'd be nice to be able to issue precise syscalls rather than depending on system utilities to do what we hope they do, with varying output formats.

Thank you for your review! I'll merge this one when CI goes green.