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

[Blue Swirl <blauwirbel@gmail.com>]

On 3/31/10, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/31/2010 02:38 PM, Blue Swirl wrote:
>
> > On 3/31/10, Michael S. Tsirkin<mst@redhat.com>  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<anthony@codemonkey.ws>  wrote:
> > >  >
> > >  >  >  On 03/31/2010 01:20 PM, Michael S. Tsirkin wrote:
> > >  >  >  >  From: David L Stevens<dlstevens@us.ibm.com>
> > >  >  >  >
> > >  >  >  >  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<dlstevens@us.ibm.com>
> > >  >  >  >  Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> > >  >  >  >  ---
> > >  >  >  >
> > >  >  >  >  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<blauwirbel@gmail.com>
> > >  >  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%.
> >
> >
>
>  BTW, this seems to do it.  I'll commit after some testing.

But this makes kvm_enabled() check dynamic and code that was
eliminated by compiler for !CONFIG_KVM will now be generated.

Wouldn't adding CONFIG_KVM to config-host.h also solve the problem?