Re: [RFC] Unify KVM kernel-space and user-space code into a singleproject

From: Avi Kivity
Date: Sun Mar 21 2010 - 16:23:08 EST


On 03/21/2010 09:06 PM, Ingo Molnar wrote:
* Avi Kivity<avi@xxxxxxxxxx> wrote:

[...] Second, from my point of view all contributors are volunteers
(perhaps their employer volunteered them, but there's no difference from
my perspective). Asking them to repaint my apartment as a condition to
get a patch applied is abuse. If a patch is good, it gets applied.
This is one of the weirdest arguments i've seen in this thread. Almost all
the time do we make contributions conditional on the general shape of the
project. Developers dont get to do just the fun stuff.
So, do you think a reply to a patch along the lines of

NAK. Improving scalability is pointless while we don't have a decent GUI.
I'll review you RCU patches
_after_ you've contributed a usable GUI.

?
What does this have to do with RCU?

The example was rcuifying kvm which took place a bit ago. Sorry, it wasn't clear.

I'm talking about KVM, which is a Linux kernel feature that is useless without
a proper, KVM-specific app making use of it.

RCU is a general kernel performance feature that works across the board. It
helps KVM indirectly, and it helps many other kernel subsystems as well. It
needs no user-space tool to be useful.

Correct. So should I tell someone that has sent a patch that rcu-ified kvm in order to scale it, that I won't accept the patch unless they do some usability userspace work? say, implementing an eject button. That's what I understood you to mean.

KVM on the other hand is useless without a user-space tool.

[ Theoretically you might have a fair point if it were a critical feature of
RCU for it to have a GUI, and if the main tool that made use of it sucked.
But it isnt and you should know that. ]

Had you suggested the following 'NAK', applied to a different, relevant
subsystem:

| NAK. Improving scalability is pointless while we don't have a usable
| tool. I'll review you perf patches _after_ you've contributed a usable
| tool.

That might hold, but the tool is usable at least for some people - it runs in production. The people running it won't benefit from an eject button or any usability improvement since they run it through a centralized management tool that hides everything. They will benefit from the scalability patches. Should I still make those patches conditional on scalability work that is of no interest to the submitter?

This is a basic quid pro quo: new features introduce risks and create
additional workload not just to the originating developer but on the rest
of the community as well. You should check how Linus has pulled new
features in the past 15 years: he very much requires the existing code to
first be top-notch before he accepts new features for a given area of
functionality.
For a given area, yes. [...]
That is my precise point.

KVM is a specific subsystem or "area" that makes no sense without the
user-space tooling it relates to. You seem to argue that you have no 'right'
to insist on good quality of that tooling - and IMO you are fundamentally
wrong with that.

kvm contains many sub-areas. I'm not going to tie unrelated things together like the GUI and sclability, configuration file format and emulator correctness, nested virtualization and qcow2 asynchronity, or other crazy combinations. People either leave en mass or become frustrated if they can't. I do reject patches touching a sub-area that I think need to be done in userspace, for example.

That's not to say kvm development is random. We have a weekly conference call where regular contributors and maintainers of both qemu and kvm participate and where we decide where to focus. Sadly the issue of a qemu GUI is not raised often. Perhaps you can participate and voice your concerns.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/