On 11/28/2017 04:01 AM, Hans de Goede wrote:
Hi,
I did have two problems when I tried to build these commits and the one that
creates vboxsf.
The more serious one is that it is possible to build vboxguest without
vboxvideo. When that happens, a non-privileged user cannot start X. As I say
in the review, > I think that combination does not make sense and should not
be allowed.
vboxguest and vboxvideo are completely independent at least from the kernel pov,
I do not believe that making them depend on each other makes sense.
AFAIK a non-privileged user cannot start X without vboxvideo at all, independent
of vboxguest being build or not. Falling back to vesa modesetting always
requires
Xorg to be suid root, or the user to be privileged.
TL;DR: I can add a dependency between the 2, but I would rather not.
Keep in mind that at some point, the newest kernel will support vboxvideo and
vboxguest; however, any distribution package will still need to contain both
kernel modules so that older kernels will work. My test showed that loading an
in-kernel vboxguest with Oracle's vboxvideo fails *unless* you run as root,
which is not acceptable. When both are in the kernel, then Xorg starts for a
non-privaleged user. That is why I think you need either a "depends on
VBOXVIDEO" or a "selects VBOXVIDEO" in the Kconfig for vboxvideo. My preference
is for the latter.
When the system is booted, vboxsf is not loaded, and the shared folders are
not automounted. Of course, that issue is not germane to these patches, but
will be important when vboxsf is merged.
Hmm, I mount a couple of shares from rc.local (I don't use vbox' automount as I
want to specify a uid for the files) and as soon as mount.vboxsf gets executed
the vboxsf module gets auto-loaded as the module contains:
MODULE_ALIAS_FS("vboxsf");
AFAIK the communication of which volumes to automount is done through vboxguest,
anyways I will look into this before submitting vboxsf, in the worst case
we need to drop a modprobe.conf.d/vboxguest.conf file which has a postinst
vboxguest
which loads vboxsf.
Thanks. Adding something of this type will make the in-kernel version match the
Oracle documentation.
Larry
