vboxadd.service unmounting vboxsf may return wrong error

[dry]

On Linux, the service (script) /opt/VBoxGuestAdditions-5.2.32/init/vboxadd does shared folder(s) unmounting with:

if ! umount -a -t vboxsf 2>/dev/null; then
  fail "Cannot unmount vboxsf folders"
fi

However umount may return an error if, for example, the same mount is available in more than one place, through binding. Then after first unmount of the same share the next one will be erroneous and umount may return error (on my Linux/Fedora it returns code 32).

I'm suggesting to slightly modify that script to avoid (possibly wrong) error exit (which can annoy your logs ), as below:

if ! umount -a -t vboxsf 2>/dev/null; then
  [ -n "$(findmnt -t vboxsf)" ] && fail "Cannot unmount vboxsf folders"
fi

[Frank Batschulat (Oracle)]

The same code is still in Trunk, the patch seems reasonable.

Any chance the submitter can provide me with a step-by-step reproducable test case how I can trigger this failing unmount behavior though "bindings"? Thanks!

[dry]

For the test case.

Before vboxsf share(s) mounting, bind root somewhere else : mount --bind / /var/local/bind/root. Do some stuff ... (optionally, part of init ) Now mount vboxsf shares , as with the vboxadd service

After such mount, you will have these shares at two places : original configured , and under the root bind. As e.g.:

dloads on /media/sf_dloads type vboxsf (rw,nodev,relatime)
dloads on /var/local/bind/root/media/sf_dloads type vboxsf (rw,nodev,relatime)

Then you can test the original command "umount -a -t vboxsf", and confirm it fails after first unmount of same share, returns non 0 (32 for me)

My guest : Linux Fedora 29, x86_64,

umount --version
umount from util-linux 2.32.1 (libmount 2.32.1: selinux, btrfs, assert, debug)

[dry]

Replying to fbatschu:

The same code is still in Trunk, the patch seems reasonable.

Any chance the submitter can provide me with a step-by-step reproducable test case how I can trigger this failing unmount behavior though "bindings"? Thanks!

For the test case.

Before vboxsf share(s) mounting, bind root somewhere else : mount --bind / /var/local/bind/root. Do some stuff ... (optionally, part of init ) Now mount vboxsf shares , as with the vboxadd service

After such mount, you will have these shares at two places : original configured , and under the root bind. As e.g.:

dloads on /media/sf_dloads type vboxsf (rw,nodev,relatime)
dloads on /var/local/bind/root/media/sf_dloads type vboxsf (rw,nodev,relatime)

Then you can test the original command "umount -a -t vboxsf", and confirm it fails after first unmount of same share, returns non 0 (32 for me)

My guest : Linux Fedora 29, x86_64,

umount --version
umount from util-linux 2.32.1 (libmount 2.32.1: selinux, btrfs, assert, debug)

[dry]

Btw, am I allowed to clone & push suggested patches/changes in some vbox repo ..?

[Frank Batschulat (Oracle)]

Replying to dry:

Btw, am I allowed to clone & push suggested patches/changes in some vbox repo ..?

Unfortunately, that is not possible, see the wiki:
https://www.virtualbox.org/wiki/Contributor_information

Also even for the one line fix/patch you have submitted, we'd want you to place your contribution under the appropriate terms as explained in the wiki above. For this small fix I'd suggest you declare your fix contribution under the terms of the MIT licence in this bug excplicitely, refer to:
https://www.virtualbox.org/wiki/MIT%20license

Thanks.

Also please mention whether or not you want to be named with a credit in the svn commit message when that fix integrates and what that name or email address should be.

[dry]

Replying to fbatschu:

... I'd suggest you declare your fix contribution under the terms of the MIT licence

in this bug excplicitely, refer to:
https://www.virtualbox.org/wiki/MIT%20license

Also please mention whether or not you want to be named with a credit in the svn commit message when that fix integrates and what that name or email address should be.

Yes, I declare it as contribution under MIT license. And Yes, I do appreciate to be named.

Thank you

[Frank Batschulat (Oracle)]

Replying to dry:

Replying to fbatschu:

... I'd suggest you declare your fix contribution under the terms of the MIT licence

in this bug excplicitely, refer to:
https://www.virtualbox.org/wiki/MIT%20license

Also please mention whether or not you want to be named with a credit in the svn commit message when that fix integrates and what that name or email address should be.

Yes, I declare it as contribution under MIT license. And Yes, I do appreciate to be named.

Thank you

Thanks a lot.

But please you need to let me know the Name you want to see mentioned in the commit contribution message. "dry" is hardly the one I suppose.

[Frank Batschulat (Oracle)]

I can reproduce the bug and verify the fix:

/opt/VBoxGuestAdditions-6.0.10/init# ./vboxadd-service status
The VirtualBox Additions are currently running.

# mkdir -p /var/local/bin/root
# mount --bind / /var/local/bin/root
# df -lkah |grep root 
/dev/sda1 on /var/local/bin/root type ext4 (rw,relatime,errors=remount-ro)

### configured a shared folder for this guest (permannent, automount)

root@fbatschu-VirtualBox:/opt/VBoxGuestAdditions-6.0.10/init# ./vboxadd-service start
# df -lkah |grep vbox >> /home/fbatschu/test.txt
vbox            687G  471G  216G  69% /media/sf_dloads
vbox            687G  471G  216G  69% /var/local/bin/root/media/sf_dloads
# df -lkah |grep root >> /home/fbatschu/test.txt 
/dev/sda1        32G  7,1G   23G  24% /var/local/bin/root
vbox            687G  471G  216G  69% /var/local/bin/root/media/sf_dloads

### now do the umount manually the vboxadd script would be doing:

# umount -a -t vboxsf
umount: /media/sf_dloads: not mounted.
# echo $?
32
# mount |grep vbox
#
# mount |grep root
/dev/sda1 on /var/local/bin/root type ext4 (rw,relatime,errors=remount-ro)

### mount again and unmount using the script to see the script failing

# ./vboxadd-service start
vboxadd-service.sh: Starting VirtualBox Guest Addition service.
# mount|grep vbox
vbox on /media/sf_dloads type vboxsf (rw,nodev,relatime,iocharset=utf8,uid=0,gid=999,dmode=0770,fmode=0770,tag=VBoxAutomounter)
vbox on /var/local/bin/root/media/sf_dloads type vboxsf (rw,nodev,relatime,iocharset=utf8,uid=0,gid=999,dmode=0770,fmode=0770,tag=VBoxAutomounter)

### now cause the unmount with the vboxadd script:

# ./vboxadd stop
VirtualBox Guest Additions: Stopping.
Cannot unmount vboxsf folders
The log file /var/log/vboxadd-setup.log may contain further information.
# echo $?
1
# cat /var/log/vboxadd-setup.log
Cannot unmount vboxsf folders

### bug reproduced!

### running it again the succeeds:

t# ./vboxadd stop
VirtualBox Guest Additions: Stopping.
VirtualBox Guest Additions: You may need to restart your guest system to finish 
removing guest drivers.

### fix verification:

# diff vboxadd.orig vboxadd
530c530
<         fail "Cannot unmount vboxsf folders"
---
>         [ -n "$(findmnt -t vboxfs)" ] && fail "Cannot unmount vboxsf folders"

# findmnt -t vboxsf
TARGET                              SOURCE FSTYPE OPTIONS
/var/local/bin/root/media/sf_dloads vbox   vboxsf rw,nodev,relatime,iocharset=utf8,uid=0,gid=999,dmode=0770,fmode=0770,tag=VBox
/media/sf_dloads                    vbox   vboxsf rw,nodev,relatime,iocharset=utf8,uid=0,gid=999,dmode=0770,fmode=0770,tag=VBox
root@fbatschu-VirtualBox:/opt/VBoxGuestAdditions-6.0.10/init# 

# ./vboxadd stop
VirtualBox Guest Additions: Stopping.
VirtualBox Guest Additions: You may need to restart your guest system to finish 
removing guest drivers.
# findmnt -t vboxsf
# 

### fix verified.

[Frank Batschulat (Oracle)]

suggested fix:

VBox/Trunk fbatschu@hpbox trunk $ svn diff
Index: src/VBox/Additions/linux/installer/vboxadd.sh
===================================================================
--- src/VBox/Additions/linux/installer/vboxadd.sh	(revision 133049)
+++ src/VBox/Additions/linux/installer/vboxadd.sh	(working copy)
@@ -527,7 +527,9 @@
         ldconfig
     fi
     if ! umount -a -t vboxsf 2>/dev/null; then
-        fail "Cannot unmount vboxsf folders"
+	# Make sure we only fail, if there are truly no more vboxsf
+	# mounts in the system.
+	[ -n "$(findmnt -t vboxsf)" ] && fail "Cannot unmount vboxsf folders"
     fi
     test -n "${INSTALL_NO_MODULE_BUILDS}" ||
         info "You may need to restart your guest system to finish removing guest drivers."

[Frank Batschulat (Oracle)]

fixed in Trunk, Committed revision 133083.

[Frank Batschulat (Oracle)]

Backported to 6.0 branch, Committed revision 133107