Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1720120 > unrolled thread

[RFC v2 0/2] Add Virtual Box vboxguest and vboxsf guest drivers to the mainline kernel

Back to article view | Back to linux.kernel

Contents

  [RFC v2 0/2] Add Virtual Box vboxguest and vboxsf guest drivers to the mainline kernel Hans de Goede <hdegoede@redhat.com> - 2017-08-25 16:40 +0200
    Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Christoph Hellwig <hch@infradead.org> - 2017-08-25 17:00 +0200
      Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Hans de Goede <hdegoede@redhat.com> - 2017-08-25 17:10 +0200
        Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Christoph Hellwig <hch@infradead.org> - 2017-08-25 17:10 +0200
          Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Christoph Hellwig <hch@infradead.org> - 2017-08-25 17:20 +0200
          Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Hans de Goede <hdegoede@redhat.com> - 2017-08-25 17:20 +0200
        Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Christoph Hellwig <hch@infradead.org> - 2017-08-25 17:10 +0200
          Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest  integration Hans de Goede <hdegoede@redhat.com> - 2017-08-25 17:10 +0200
    Re: [RFC v2 2/2] fs: Add VirtualBox guest shared folder (vboxsf)  support Christoph Hellwig <hch@infradead.org> - 2017-08-25 17:10 +0200

#1720120 — [RFC v2 0/2] Add Virtual Box vboxguest and vboxsf guest drivers to the mainline kernel

Hi All,

Here is a 2nd RFC version of my cleaned up version of the
VirtualBox vboxguest and vboxsf guest drivers.

This time my cleanup is complete and the patches are ready for a
FULL review now.

This is still an RFC because VirtualBox upstream is considering making
some final ioctl ABI changes for the vboxguest chardev now that they
still can. As soon as I get the go ahead from them that the ioctl ABI
is stable I will send a non RFC version.

Here is (part of) the RFC v1 cover-letter which is still relevant:

I've removed all depenencies on vbox's OO-independent runtime and
the runtime itself, reducing the vboxguest driver from 100000+ lines
of code to aprox. 6500 and the vboxsf driver from 60000 lines to 4500.
This reduces the non debug vboxguest.ko size from 450kB to less then 100 kB. 
I've also cleaned up various other warts such as doing hardware init in
module_init rather then in a pci_probe callback.

The vboxguest driver introduces a new userspace API + ABI in the form
of ioctls on a character device. VirtualBox upstream not willing to
commit to keeping this ABI stable was one of the things which has
kept this driver driver out of mainline sofar. I've been talking to
VirtualBox upstream about mainlining the guest drivers and VirtualBox
upstream has agreed to consider the userspace ABI stable and only
extend it in a backwards compatible manner.

Regards,

Hans

[toc] | [next] | [standalone]

#1720149 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

Please kill all the weird typedefs and camel case first, and then
we can do a real review.

[toc] | [prev] | [next] | [standalone]

#1720165 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

Hi,

On 25-08-17 16:58, Christoph Hellwig wrote:
> Please kill all the weird typedefs and camel case first, and then
> we can do a real review.

I've already removed all the typedefs and camel-case outside of
include/uapi/linux

The ones in include/uapi/linux are preserved to preserve API
compatibility with the existing VirtualBox userspace tools.

I was planning to mention this in the coverletter, but I
forgot, sorry.

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1720166 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

On Fri, Aug 25, 2017 at 05:06:52PM +0200, Hans de Goede wrote:
> I'm afraid that the userspace API is more or less a take it
> or leave it kinda deal, we can do small cleanups, but an
> entire re-design is out of the question. The vboxguest code
> has a long (too long) out of tree history and the same ioctl
> interface is used by VirtualBox upstream on Windows, Mac OS X,
> the BSDs and Solaris, so we cannot just go and break it to
> out hart's content.

Then hard out NAK for the code.

[toc] | [prev] | [next] | [standalone]

#1720177 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

On Fri, Aug 25, 2017 at 05:13:35PM +0200, Hans de Goede wrote:
> You know I've been working on this for almost 2 fulltime months

I don't, and I don't care to be honest.  Figuring out what do
do about the userspace API/ABI should be your first priority,
and you should have discussed this on the linux-api list where
I didn't see anything about this driver.

We have a general stance to fix userspace APIs/ABIs before inclusion,
and in the few cases where we did it turned into a giant trainwreck.
The prime example is the Android binder, which has been a giant mess.
And that's something a lot more important than a weird hobbyist
hypersvior.

[toc] | [prev] | [next] | [standalone]

#1720188 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

Hi,

On 25-08-17 17:09, Christoph Hellwig wrote:
> On Fri, Aug 25, 2017 at 05:06:52PM +0200, Hans de Goede wrote:
>> I'm afraid that the userspace API is more or less a take it
>> or leave it kinda deal, we can do small cleanups, but an
>> entire re-design is out of the question. The vboxguest code
>> has a long (too long) out of tree history and the same ioctl
>> interface is used by VirtualBox upstream on Windows, Mac OS X,
>> the BSDs and Solaris, so we cannot just go and break it to
>> out hart's content.
> 
> Then hard out NAK for the code.

You know I've been working on this for almost 2 fulltime months
and this has been discussed in several places already long
before you jumped on this thread and so far everyone liked
the idea of getting this merged and was ok with living with
the existing ioctl API (assuming no security issues / other
nasty warts are found during review).

So although your input on this is appreciated some pragmatism
would also be much appreciated. Remember perfect is the enemy
of good.

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1720170 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

On Fri, Aug 25, 2017 at 05:00:31PM +0200, Hans de Goede wrote:
> I've already removed all the typedefs and camel-case outside of
> include/uapi/linux
> 
> The ones in include/uapi/linux are preserved to preserve API
> compatibility with the existing VirtualBox userspace tools.

When merging new drivers we need to fix up the user abis anyway,
we don't want to be stuck with mistakes from the past.  And the
APIs look pretty bad, so there will be a lot work required after
this basic cleanup anyway.

[toc] | [prev] | [next] | [standalone]

#1720172 — Re: [RFC v2 1/2] virt: Add vboxguest driver for Virtual Box Guest integration

Hi,

On 25-08-17 17:03, Christoph Hellwig wrote:
> On Fri, Aug 25, 2017 at 05:00:31PM +0200, Hans de Goede wrote:
>> I've already removed all the typedefs and camel-case outside of
>> include/uapi/linux
>>
>> The ones in include/uapi/linux are preserved to preserve API
>> compatibility with the existing VirtualBox userspace tools.
> 
> When merging new drivers we need to fix up the user abis anyway,
> we don't want to be stuck with mistakes from the past.  And the
> APIs look pretty bad, so there will be a lot work required after
> this basic cleanup anyway.

I'm afraid that the userspace API is more or less a take it
or leave it kinda deal, we can do small cleanups, but an
entire re-design is out of the question. The vboxguest code
has a long (too long) out of tree history and the same ioctl
interface is used by VirtualBox upstream on Windows, Mac OS X,
the BSDs and Solaris, so we cannot just go and break it to
out hart's content.

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1720169 — Re: [RFC v2 2/2] fs: Add VirtualBox guest shared folder (vboxsf) support

Please merge the files into a a lot fewer files - basically
one .c file and a header for the protocol probably is enough.

Also kill all kinds of weird tiny wrappers that just obsfucate the code,
and make sure linux-fsdevel is on Cc for your next submission.

Also don't use i_private, instead defined alloc_inode and destroy_inode
methods and use container_of.

> +/**
> + * @note Anyone wishing to make changes here might wish to take a look at
> + *  http://www.atnf.csiro.au/people/rgooch/linux/vfs.txt
> + * which seems to be the closest there is to official documentation on
> + * writing filesystem drivers for Linux.
> + */

And drop comments like this which will create a lot more confusion than
actually helping.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web