Skip to content
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

[merged] Cleanup temporary mount point upon failure #163

Closed
wants to merge 1 commit into from

Conversation

rhvgoyal
Copy link
Collaborator

@rhvgoyal rhvgoyal commented Nov 1, 2016

Fixes #162

platform_supports_deferred_deletion() mounts tempdir and then can exit
due to error conditions and never unmount that tempdir. Do an unmount
during exit to do the cleanup.

Also make a error check little early. First do the check and then
create pipes and mount point.

Signed-off-by: Vivek Goyal [email protected]


mkfifo $PIPE1
mkfifo $PIPE2
mount -o bind $TEMPDIR $TEMPDIR
unshare -m /usr/lib/docker-storage-setup/dss-child-read-write $PIPE1 $PIPE2 &
Copy link
Member

Choose a reason for hiding this comment

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

We could rely on the kernel to umount if we did the mount inside the unshare, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. We should not have to do mounting in host mount namespace. We should be able to create new mount namespace, do the mount and then signal to parent that try to remove directory from host mount namespace.

Only extra overhead will be that we will have to pass the path to the tmp directory to child process. That should be fine.

Will try it.

Do the bind mount in child mount namespace. That way it will automaitcally
get cleaned up as child exits.

Also create pipes after making sure child process script exists.

Signed-off-by: Vivek Goyal <[email protected]>
@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Nov 2, 2016

@cgwalters PTAL. Now I am creating bind mount inside child process mount namespace.

@rhatdan
Copy link
Member

rhatdan commented Nov 2, 2016

LGTM

@cgwalters
Copy link
Member

@rh-atomic-bot r+ 697d6ff

@rh-atomic-bot
Copy link

⌛ Testing commit 697d6ff with merge ba0dcf3...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing ba0dcf3 to master...

@rh-atomic-bot rh-atomic-bot changed the title Cleanup temporary mount point upon failure [merged] Cleanup temporary mount point upon failure Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants