Hi,
On 28-11-17 17:15, Larry Finger wrote:
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.
Hmm, I guess what is happening here is that the vbox provided vboxguest is not
loading
because the one built into the kernel is not loading, and then vboxvideo does
not load
because it depends on symbols from vboxguest. But the vboxvideo shipped with
the guest
additions has been changed to no longer depend on vboxguest quite a while ago,
so with
current guest additions this should not be an issue and functionality wise the 2
vboxguest drivers are equivalent.
"depends on" / "select" really are reserved for when drivers use symbols from
another
module and not to express weird interactions like this. There are many ways a
.config
can be made which userspace will not like, yet we still allow this. IMHO adding
a
depends on between 2 independent modules is simply just wrong.
For the next version I will add a text to the help part of the Kconfig advising
to also
enable the vboxvideo driver. That really is the best we can do IMHO.
Talking about the next version, yesterday I ran out of time while working on
this,
I hope to post a v3 addressing your review comments today.
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.
Regards,
Hans
