-
Notifications
You must be signed in to change notification settings - Fork 145
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
Refuse to destroy a jail with mounted filesystems #667
Conversation
b56ec46
to
54e3cd2
Compare
``` # bastille create temp 14.0-RELEASE 192.168.1.10 lo1 ... # mkdir test # cp /usr/bin/less test/ # mkdir /usr/local/bastille/jails/temp/root/test # bastille mount temp $(realpath test) test [temp]: Added: /root/admin/bastille/test /usr/local/bastille/jails/temp/root/test nullfs ro 0 0 # /usr/local/bastille/jails/temp/root/test/less -f /dev/stdin & # bastille destroy force temp rdr-anchor not found in pf.conf [temp]: temp: removed umount: unmount of /usr/local/bastille/jails/temp/root/test failed: Device busy jail: temp: /sbin/umount -t nullfs /usr/local/bastille/jails/temp/root/test: failed Deleting Jail: temp. Jail has mounted filesystems: /usr/local/bastille/jails/temp/root/test ```
54e3cd2
to
6c32f7f
Compare
I think this should be considered a bug fix rather than an enhancement. A likely consequence of operating without this protection is the loss of files within the mounted file system. That's a very bad outcome and not something a user would expect or think to guard against externally. |
@gahr in your PR, the message comes after the dataset has been destroyed if it's on ZFS. If we are going to have that warning, it should come earlier so the user can check mounts and try to destroy again. |
Good point. I don't use ZFS myself so I can't easily test any changes in that area. Would you please improve my PR in that direction? |
Will this destroy the files that are mounted inside the jail? |
@yaazkal I have gone over the changes and moved the mount check to it's proper place. It is functioning as expected. How do I update the PR here? |
thanks, just commit the changes to the branch you created whe did the PR |
This PR can now be closed in favor of the above. |
Thank you! |
Fixes #662