* Maxime Coquelin (maxime.coque...@redhat.com) wrote:
>
>
> On 12/14/2016 09:59 AM, Cornelia Huck wrote:
> > On Wed, 14 Dec 2016 08:41:55 +0000
> > Stefan Hajnoczi <stefa...@gmail.com> wrote:
> >
> > > On Wed, Dec 14, 2016 at 8:28 AM, Maxime Coquelin
> > > <maxime.coque...@redhat.com> wrote:
> > > >
> > > >
> > > > On 12/14/2016 08:44 AM, Cornelia Huck wrote:
> > > > > >
> > > > > > > 14:44 < stefanha> Not sure if anyone can think of a nicer
> > > > > > > solution.
> > > > > > > 14:45 < stefanha> But we're going to have to keep lying to the
> > > > > > > guest if
> > > > > > > we want to preserve migration compatibility
> > > > > > > 14:45 < stefanha> The key change in behavior with the patch you
> > > > > > > identified is:
> > > > > > > 14:46 < stefanha> if (!virtio_has_feature(vdev->host_features,
> > > > > > > VIRTIO_F_VERSION_1)) {
> > > > > > > 14:46 < stefanha> virtio_pci_disable_modern(proxy);
> > > > > > > 14:46 < stefanha> Previously it didn't care about
> > > > > > > vdev->host_features.
> > > > > > > It simply allowed VERSION_1 when proxy's disable_modern boolean
> > > > > > > was false.
> > > > > > > 14:47 < mdroth> stefanha: ok, that explains why it seems to work
> > > > > > > with
> > > > > > > disable-modern=true
> > > > > > > 14:48 < stefanha> mdroth: Your Ubuntu kernel old but 14.04 LTS is
> > > > > > > definitely still around so I don't think we can ship QEMU 2.8
> > > > > > > like this.
> > > > > > > 14:49 < stefanha> mdroth: Let's summarize it on the mailing list
> > > > > > > and
> > > > > > > see what Michael Tsirkin and Maxime Coquelin think.
> > > > > > > 14:49 < mdroth> stefanha: i suppose a potential workaround would
> > > > > > > be to
> > > > > > > tell users to set disable-modern= to match their vhost
> > > > > > > capabilities, but
> > > > > > > it's hard for them to apply that retroactively if they're looking
> > > > > > > to migrate
> > > > >
> > > > > Another thought: Maybe this bug only surfaced right now because older
> > > > > qemus defaulted virtio-pci to legacy?
> > > > >
> > > > > (I think modern virtio-pci with old vhost resulted in a config that
> > > > > was
> > > > > rejected at least by Linux guests. Because pci defaulted to legacy, we
> > > > > only had the post-plugged workaround for ccw before.)
> > > >
> > > >
> > > > Yes, for PCI with old vhost, modern enabled and recent kernel on guest,
> > > > we get this failure at virtio-pci probe time:
> > > >
> > > > virtio_net virtio0: virtio: device uses modern interface but does not
> > > > have
> > > > VIRTIO_F_VERSION_1.
> > >
> > > Is this error a regression in QEMU 2.8?
> >
> > I think it pokes up because modern virtio-pci is now by default on. It
> > was broken before if the user wanted a modern virtio-pci device
> > explicitly.
> >
> > (ccw defaulted to virtio 1.0 much earlier, so we had the post-plugged
> > solution that this patch replaced and which is basically the same for
> > ccw.)
> >
> > >
> > > It's better to ship with an existing issue still open than with a new
> > > regression. We must not break existing users' setups.
> > >
> > > A solution for the next QEMU version is to use a flag in the machine
> > > type version telling virtio whether or not allow devices (e.g.
> > > vhost-net) to influence the host feature bits. Old machine types will
> > > say no, new machine types will say yes.
> > >
> > > In the meantime I would revert your patch for QEMU 2.8.
> > >
> > > Maxime, Cornelia, Michael: Do you agree?
> > >
> > > Stefan
> >
> > Reverting the patch should be fine for ccw. What about the virtio-pci
> > with old vhost mess, though? Defaulting to modern would mean that users
> > get unusable devices in that setup.
>
> Just did some tests, and can confirm that reverting the patch would
> re-introduce initial bug, which is breaking virtio-pci when host does
> not support VERSION_1.
Can you modify it so it only fixes the problem on new machine types?
Either that or *fail* if you attempt to bring up a virtio interface
using version 1 with a kernel that doesn't support it (with a note
saying that you could use an option to disable it).
Dave
> Note that this problem is present in v2.7.0 since:
>
> commit 9a4c0e220d8a4f82b5665d0ee95ef94d8e1509d5
> Author: Marcel Apfelbaum <mar...@redhat.com>
> Date: Wed Jul 20 18:28:21 2016 +0300
>
> hw/virtio-pci: fix virtio behaviour
>
> Enable transitional virtio devices by default.
> Enable virtio-1.0 for devices plugged into
> PCIe ports (Root ports or Downstream ports).
>
> Using the virtio-1 mode will remove the limitation
> of the number of devices that can be attached to a machine
> by removing the need for the IO BAR.
>
> Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com>
>
>
> Maybe better to implement the workaround you proposed Stefan?
>
> Thanks,
> Maxime
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
