Re: [Qemu-devel] Re: [PATCH] vhost: fix features ack

[Blue Swirl]

On 3/31/10, Michael S. Tsirkin <address@hidden> wrote:
> On Wed, Mar 31, 2010 at 03:38:05PM -0300, Luiz Capitulino wrote:
>  > On Wed, 31 Mar 2010 13:26:23 -0500
>  > Anthony Liguori <address@hidden> wrote:
>  >
>  > > On 03/31/2010 01:20 PM, Michael S. Tsirkin wrote:
>  > > > From: David L Stevens<address@hidden>
>  > > >
>  > > > vhost driver in qemu didn't ack features, and this happens
>  > > > to work because we don't really require any features. However,
>  > > > it's better not to rely on this. This patch passes features to
>  > > > vhost as guest acks them.
>  > > >
>  > > > Signed-off-by: David L Stevens<address@hidden>
>  > > > Signed-off-by: Michael S. Tsirkin<address@hidden>
>  > > > ---
>  > > >
>  > > > Anthony, here's a fixup patch to address an issue in vhost
>  > > > patches. Incidentially, what's the status of the vhost patchset?
>  > > >
>  > >
>  > > http://repo.or.cz/w/qemu/aliguori-queue.git vhost
>  > >
>  > > Is what I'm currently testing.  With vhost disabled,  the following seg
>  > > faults:
>  > >
>  > > qemu-system-x86_64 -hda ~/images/linux.img -net tap -net
>  > > nic,model=virtio -enable-kvm
>  > >
>  > > But not when using TCG.  I'm not sure that it's your patches at fault
>  > > and I'm attempting to bisect now to figure that out.
>  >
>  >  Probably this is the same segfault I'm getting right now in master,
>  > bisect says it's:
>  >
>  > """
>  > commit ad96090a01d848df67d70c5259ed8aa321fa8716
>  > Author: Blue Swirl <address@hidden>
>  > Date:   Mon Mar 29 19:23:52 2010 +0000
>  >
>  >     Refactor target specific handling, compile vl.c only once
>  > """
>
>  Why are the compile once patches helpful? They seem to introduce
>  churn and bugs, they actively make it harder to extend qemu as you can't use
>  target-specific code in code that is compiled once, they might have
>  performance penalty - and what do we gain? Any given user is unlikely to
>  need to build on more than one target, distros have enough computing
>  power to build in parallel.

As has been explained many times, knowledge about CPU specific
features has no place in devices. Actively removing CPU specifics from
devices is good but preventing new bad code to be committed is better.

I don't have distro computing powers (unless some distro's compute
center only has two low power machines) and I guess some other
developers don't have either. All developers and patch submitters are
expected to compile all targets. This patch set has decreased the
number of files compiled by about 20%.

>  Maybe it makes sense to revert the compile once patches, and discuss
>  these issues before re-commit?

Maybe I'll try to remember this as your favourite problem solving
approach if I happen to dislike the changes your patches may cause in
the future. ;-)