Re: [PATCH] virtio: move 'use-disabled-flag' property to hw_compat_4_2

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Stefano Garzarella (sgarzare@redhat.com) wrote:
> On Mon, Jan 18, 2021 at 04:03:12PM +0000, Dr. David Alan Gilbert wrote:
> > * Stefano Garzarella (sgarzare@redhat.com) wrote:
> > > Commit 9d7bd0826f introduced a new 'use-disabled-flag' property
> > > set to true by default.
> > > To allow the migration, we set this property to false in the hw_compat,
> > > but in the wrong place (hw_compat_4_1).
> > > 
> > > Since commit 9d7bd0826f was released with QEMU 5.0, we move
> > > 'use-disabled-flag' property to hw_compat_4_2, so 4.2 machine types
> > > will have the pre-patch behavior and the migration can work.
> > 
> > Be a little careful that fixing this probably causes a migration from
> > 5.2->6.0 to fail with this machine type;  so when we do these
> > type of fixes we often fix an old machine type between some pair of qemu
> > versions and then break it between a different set.
> 
> Good point!
> 
> I did some tests using the example below always using pc-q35-4.2 and it
> seems that works well:
> 
> - 5.2 -> 6.0    pass
> - 5.2 -> 4.2            FAIL
> - 6.0 -> 5.2    pass
> - 6.0 -> 4.2    pass
> - 4.2 -> 5.2    pass
> - 4.2 -> 6.0    pass
> 
> Should I run some more tests?

Apologies for the delay; I had to step back and understand a bit about
what was going on.

The problem here is that you're sending a 'disabled' subsection when
that option is true; your patch doesn't change the 4.1 machine type but
it does change the 4.2 machine type; and that makes the 4.2 machine type
not send it; so that means your patched version *will* work to existing
code (because it's a subsection anyway it doesn't break the stream
format when it's missing).


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Thanks,
> Stefano
> 
> > 
> > Dave
> > 
> > > The issue was discovered with vhost-vsock device and 4.2 machine
> > > type without running any kernel in the VM:
> > >     $ qemu-4.2 -M pc-q35-4.2,accel=kvm \
> > >         -device vhost-vsock-pci,guest-cid=4 \
> > >         -monitor stdio -incoming tcp:0:3333
> > > 
> > >     $ qemu-5.2 -M pc-q35-4.2,accel=kvm \
> > >         -device vhost-vsock-pci,guest-cid=3 \
> > >         -monitor stdio
> > >     (qemu) migrate -d tcp:0:3333
> > > 
> > >     # qemu-4.2 output
> > >     qemu-system-x86_64: Failed to load virtio-vhost_vsock:virtio
> > >     qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:03.0/virtio-vhost_vsock'
> > >     qemu-system-x86_64: load of migration failed: No such file or directory
> > > 
> > > Reported-by: Jing Zhao <jinzhao@redhat.com>
> > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1907255
> > > Fixes: 9d7bd0826f ("virtio-pci: disable vring processing when bus-mastering is disabled")
> > > Cc: mdroth@linux.vnet.ibm.com
> > > CC: qemu-stable@nongnu.org
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > >  hw/core/machine.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index de3b8f1b31..5d6163ab70 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -70,12 +70,12 @@ GlobalProperty hw_compat_4_2[] = {
> > >      { "qxl", "revision", "4" },
> > >      { "qxl-vga", "revision", "4" },
> > >      { "fw_cfg", "acpi-mr-restore", "false" },
> > > +    { "virtio-device", "use-disabled-flag", "false" },
> > >  };
> > >  const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
> > > 
> > >  GlobalProperty hw_compat_4_1[] = {
> > >      { "virtio-pci", "x-pcie-flr-init", "off" },
> > > -    { "virtio-device", "use-disabled-flag", "false" },
> > >  };
> > >  const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
> > > 
> > > --
> > > 2.26.2
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK