Re: [open-vm-tools-devel] Getting the kernel modules into Linux mainline

[Adar D. <ad...@vm...>]

Chris,

I'm sorry that none of us replied to your mail, but better 3 months late than never, right?

Thanks very much for taking the time to go through the vmblock code and cleaning it up. As a developer I can imagine how annoying it must be to translate a non-trivial piece of code from one coding style to another, so I very much appreciate your perseverance in the matter.

Before I get to your patch, let's discuss the more interesting issue of vmblock's future. As you surmised, vmblock-fuse was written to replace vmblock. This was done primarily because the informal feedback we received from some kernel developers suggested that the design of vmblock is a non-starter for the kernel. See http://bugzilla.redhat.com/show_bug.cgi?id=294341 for Christoph Hellwig's comments and http://sourceforge.net/mailarchive/message.php?msg_name=46E6FDEC.3090700%40codemonkey.ws for Anthony Liguori's comments. Unfortunately, vmblock-fuse isn't quite ready for prime-time, but until its code is part of open-vm-tools it's difficult to describe exactly why (the open-sourcing process is taking longer than I expected). I suggest that you table the work on vmblock for the time being, and once vmblock-fuse is part of open-vm-tools we can have a more productive discussion about the future of vmblock. How does that sound?

If you do want to continue preparing VMware module code for upstream inclusion, first of all, thanks, and secondly, I recommend vmmemctl. It's a very simple module (especially once the compat glue is removed), it hasn't changed in a long time, and as Linux has recently gained kvm and xen balloon modules, I imagine the VMware one will be fairly non-controversial. Another alternative is the vmxnet module, which is a straightforward paravirtualized network card. The one issue there might be the morphing "feature", which may require us to merge the vmxnet code into the existing upstream pcnet32 module. If you're interested in this, let me know and I'll provide more details.

Since we're on the subject, I also want to say a few words about upstreaming VMware's kernel modules. In general we're getting more and more comfortable about upstreaming as some of us have been socializing the idea within the company. I think the remaining obstacle is this idea that once our code is upstream, we are no longer acting as the distributor, which means we can no longer control its destiny. We've shipped out-of-tree modules for years now and have grown accustomed to the control that this gives us; among other things, we can do things like release updated modules simultaneously with our products, or make whatever changes we want regardless of how locked down the affected distros might be (for example, we can add a new feature to a module intended for a distro that's been declared end-of-life by its vendor). It's very difficult to cede this control once you've enjoyed it for many product releases, which is partly why progress has been slow. To mitigate this problem, we're trying to understand whether, after upstreaming, we can gain some control back by working more closely with distros so that new features or changes that are important to us can be backported into existing distro releases in time for, say, a VMware product release. I don't have any details to share yet, but I'm sure either Ragavan or myself will say more about this in the near future.

Overall I think your vmblock patch is a solid improvement. I didn't look so much at the functionality as I imagine it's the same as before, minus compat glue. If there's something in particular you'd like me to pay closer attention to, let me know. Since you're working on vmblock, I've attached two unit test programs that you might find useful. vmblocktest.c is a general stress test we've used on all of our vmblock implementations (fuse, Linux, FreeBSD, and Solaris). manual-blocker.c was written by Daniel and used for testing vmblock-fuse. You should be able to build both with a minimum of fuss, but let me know if you have issues there.

Here are my (cosmetic) comments:
- The PURPOSE and TESTING portions are especially useful; we're suffering from a dearth of documentation.
- The overloaded interface you mention in the test code was also discussed by Dan Benamy and Miklos at the time that Dan was writing vmblock-fuse. In his implementation he's opted for a new interface where a character representing the command and the path are concatenated and sent to the driver via a single write(2). You can see view the thread at http://article.gmane.org/gmane.comp.file-systems.fuse.devel/6533, if you're interested.
- In Kconfig, some of the capitalization of "VMware" is inconsistent; could you fix that?
- Also in Kconfig, the driver should probably be referred to as a "blocking" driver instead of a "block" driver; that should alleviate any confusion regarding vmblock and the block layer.
- In Makefile, the indentation for the backticks isn't consistent. I'm not sure if you care about that.

Again, thanks for helping us out with this effort!