On Mon, Oct 13, 2014 at 02:29:20PM +0200, Alexander Graf wrote:
>
>
> On 13.10.14 12:42, Greg Kurz wrote:
> > On Mon, 13 Oct 2014 12:01:07 +0300
> > "Michael S. Tsirkin" <m...@redhat.com> wrote:
> >
> >> On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
> >>> On Mon, 6 Oct 2014 20:25:04 +0300
> >>> "Michael S. Tsirkin" <m...@redhat.com> wrote:
> >>>> [...]
> >>>>
> >>>> BTW I reverted that patch, and to fix migration, I'm thinking
> >>>> about applying the following patch on top of master.
> >>>>
> >>>
> >>> Michael,
> >>>
> >>> I could force the migration issue with a rhel65 guest thanks to the
> >>> following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
> >>>
> >>> @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque,
> >>> uint32_t addr, uint32_t val)
> >>> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> >>> hwaddr pa;
> >>>
> >>> + if ((vdev->status == (VIRTIO_CONFIG_S_ACKNOWLEDGE |
> >>> VIRTIO_CONFIG_S_DRIVER))
> >>> + && (!(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER))
> >>> + && getenv("MIG_BUG"))
> >>> + {
> >>> + fprintf(stderr, "\n\n\n\tMIGRATE !\n\n\n");
> >>> + qmp_migrate(getenv("MIG_BUG"), false, false, false, false, false,
> >>> + false, NULL);
> >>> + unsetenv("MIG_BUG");
> >>> + }
> >>> +
> >>> switch (addr) {
> >>> case VIRTIO_PCI_GUEST_FEATURES:
> >>> /* Guest does not negotiate properly? We have to assume
> >>> nothing. */
> >>>
> >>>
> >>> Indeed, the destination QEMU master hangs because bus master isn't set.
> >>>
> >>>> Would appreciate testing cross-version migration (2.1 to master)
> >>>> with this patch applied.
> >>>>
> >>>
> >>> I had first to to enable the property for pseries of course. I could then
> >>> migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
> >>> where we have DRIVER enabled and MASTER disabled, without experiencing
> >>> the hang.
> >>>
> >>> Your fix works as expected (just a remark below).
> >>>
> >>>>
> >>>>
> >>>> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> >>>> index 1cea157..8873b6d 100644
> >>>> --- a/hw/virtio/virtio-pci.h
> >>>> +++ b/hw/virtio/virtio-pci.h
> >>>> @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
> >>>> #define VIRTIO_PCI_BUS_CLASS(klass) \
> >>>> OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass,
> >>>> TYPE_VIRTIO_PCI_BUS)
> >>>>
> >>>> +/* Need to activate work-arounds for buggy guests at vmstate load. */
> >>>> +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT 0
> >>>> +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
> >>>> + (1 << VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
> >>>> +
> >>>> /* Performance improves when virtqueue kick processing is decoupled
> >>>> from the
> >>>> * vcpu thread using ioeventfd for some devices. */
> >>>> #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
> >>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>>> index bae023a..e07b6c4 100644
> >>>> --- a/include/hw/i386/pc.h
> >>>> +++ b/include/hw/i386/pc.h
> >>>> @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> >>>> uint64_t *);
> >>>> .driver = "intel-hda",\
> >>>> .property = "old_msi_addr",\
> >>>> .value = "on",\
> >>>> + },\
> >>>> + {\
> >>>> + .driver = "virtio-pci",\
> >>>> + .property = "virtio-pci-bus-master-bug-migration",\
> >>>> + .value = "on",\
> >>>> }
> >>>>
> >>>> #define PC_COMPAT_2_0 \
> >>>
> >>> FWIW, the issue does not occur with intel targets, at least not
> >>> in my test case (booting rhel6 on a virtio-blk disk). I see bus
> >>> master is set early (bios ?) and never unset...
> >>>
> >>> If you decide to apply for intel anyway, shouldn't the enablement be
> >>> in a separate patch ?
> >>>
> >>> Will you resend or would you like me to do it, along with the pseries
> >>> enablement part ? In this case, I would need your SoB for the present
> >>> patch.
> >>>
> >>> Thanks.
> >>>
> >>> --
> >>> Greg
> >>
> >> AFAIK pseries doesn't support cross-version migration, does it?
>
> We're trying to make sure we don't break cross-version migration for the
> pseries machines. If nothing else, at least as a learning exercise.
>
>
> Alex
I see.
In that case, I think we need a common header, where
we can add device compatibility defines.
With PC and PSERIES would then pull it in.
Let's not duplicate code.
I'll try to come up with a patch.
--
MST
