[Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio

Dr. David Alan Gilbert (git) posted 2 patches 6 years, 3 months ago
Test docker-clang@ubuntu passed
Test s390x failed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190729162903.4489-1-dgilbert@redhat.com
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Gonglei <arei.gonglei@huawei.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/core/machine.c             | 23 +++--------------------
hw/display/virtio-gpu-pci.c   |  4 +---
hw/display/virtio-vga.c       |  4 +---
hw/virtio/virtio-crypto-pci.c |  4 +---
hw/virtio/virtio-input-pci.c  |  4 +---
hw/virtio/virtio-pci.c        | 26 ++++++++++----------------
hw/virtio/virtio-pci.h        | 31 ++++++-------------------------
include/hw/qdev-core.h        |  3 +++
qom/object.c                  |  3 +++
9 files changed, 29 insertions(+), 73 deletions(-)
[Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio
Posted by Dr. David Alan Gilbert (git) 6 years, 3 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Revert a couple of patches that break PCIe capabilities in virtio
devices. The 'optional' revert is just reverted to make the main
reversion trivial.

Symptom:
  Loss of PCIe capabilities in virtio devices hung off PCIe bridges

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


Dr. David Alan Gilbert (2):
  Revert "Revert "globals: Allow global properties to be optional""
  Revert "hw: report invalid disable-legacy|modern usage for
    virtio-1-only devs"

 hw/core/machine.c             | 23 +++--------------------
 hw/display/virtio-gpu-pci.c   |  4 +---
 hw/display/virtio-vga.c       |  4 +---
 hw/virtio/virtio-crypto-pci.c |  4 +---
 hw/virtio/virtio-input-pci.c  |  4 +---
 hw/virtio/virtio-pci.c        | 26 ++++++++++----------------
 hw/virtio/virtio-pci.h        | 31 ++++++-------------------------
 include/hw/qdev-core.h        |  3 +++
 qom/object.c                  |  3 +++
 9 files changed, 29 insertions(+), 73 deletions(-)

-- 
2.21.0


Re: [Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio
Posted by Cornelia Huck 6 years, 3 months ago
On Mon, 29 Jul 2019 17:29:01 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Revert a couple of patches that break PCIe capabilities in virtio
> devices. The 'optional' revert is just reverted to make the main
> reversion trivial.

Don't want to spoil the party here; but wasn't the optional stuff
removed because it was deemed to be a bad idea?

> 
> Symptom:
>   Loss of PCIe capabilities in virtio devices hung off PCIe bridges
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> 
> Dr. David Alan Gilbert (2):
>   Revert "Revert "globals: Allow global properties to be optional""
>   Revert "hw: report invalid disable-legacy|modern usage for
>     virtio-1-only devs"
> 
>  hw/core/machine.c             | 23 +++--------------------
>  hw/display/virtio-gpu-pci.c   |  4 +---
>  hw/display/virtio-vga.c       |  4 +---
>  hw/virtio/virtio-crypto-pci.c |  4 +---
>  hw/virtio/virtio-input-pci.c  |  4 +---
>  hw/virtio/virtio-pci.c        | 26 ++++++++++----------------
>  hw/virtio/virtio-pci.h        | 31 ++++++-------------------------
>  include/hw/qdev-core.h        |  3 +++
>  qom/object.c                  |  3 +++
>  9 files changed, 29 insertions(+), 73 deletions(-)
> 


Re: [Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio
Posted by Dr. David Alan Gilbert 6 years, 3 months ago
* Cornelia Huck (cohuck@redhat.com) wrote:
> On Mon, 29 Jul 2019 17:29:01 +0100
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Revert a couple of patches that break PCIe capabilities in virtio
> > devices. The 'optional' revert is just reverted to make the main
> > reversion trivial.
> 
> Don't want to spoil the party here; but wasn't the optional stuff
> removed because it was deemed to be a bad idea?

I'm perfectly happy to go either way with this; it maybe a bad idea
but it's harmless I think.

Dave

> > 
> > Symptom:
> >   Loss of PCIe capabilities in virtio devices hung off PCIe bridges
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > 
> > 
> > Dr. David Alan Gilbert (2):
> >   Revert "Revert "globals: Allow global properties to be optional""
> >   Revert "hw: report invalid disable-legacy|modern usage for
> >     virtio-1-only devs"
> > 
> >  hw/core/machine.c             | 23 +++--------------------
> >  hw/display/virtio-gpu-pci.c   |  4 +---
> >  hw/display/virtio-vga.c       |  4 +---
> >  hw/virtio/virtio-crypto-pci.c |  4 +---
> >  hw/virtio/virtio-input-pci.c  |  4 +---
> >  hw/virtio/virtio-pci.c        | 26 ++++++++++----------------
> >  hw/virtio/virtio-pci.h        | 31 ++++++-------------------------
> >  include/hw/qdev-core.h        |  3 +++
> >  qom/object.c                  |  3 +++
> >  9 files changed, 29 insertions(+), 73 deletions(-)
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio
Posted by Peter Maydell 6 years, 3 months ago
On Mon, 29 Jul 2019 at 17:36, Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Cornelia Huck (cohuck@redhat.com) wrote:
> > On Mon, 29 Jul 2019 17:29:01 +0100
> > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> >
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > >
> > > Revert a couple of patches that break PCIe capabilities in virtio
> > > devices. The 'optional' revert is just reverted to make the main
> > > reversion trivial.
> >
> > Don't want to spoil the party here; but wasn't the optional stuff
> > removed because it was deemed to be a bad idea?
>
> I'm perfectly happy to go either way with this; it maybe a bad idea
> but it's harmless I think.

It seems like the original commits were:
 * patch that does something
 * patch that removes no-longer used functionality (optional globals)

so it makes sense to me that if we want to revert the 'patch that
does something' we should first revert the patch that cleaned
up unused-functionality (because now we need it again). Is
that right?

If optional-globals are a bad idea then we should take another
run at this for 4.2, but as a "revert stuff for 4.1" strategy
it seems fine to me.

thanks
-- PMM

Re: [Qemu-devel] [For 4.1 PATCH v2 0/2] Reversions to fix PCIe in virtio
Posted by Daniel P. Berrangé 6 years, 3 months ago
On Mon, Jul 29, 2019 at 05:43:17PM +0100, Peter Maydell wrote:
> On Mon, 29 Jul 2019 at 17:36, Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Cornelia Huck (cohuck@redhat.com) wrote:
> > > On Mon, 29 Jul 2019 17:29:01 +0100
> > > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > >
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > >
> > > > Revert a couple of patches that break PCIe capabilities in virtio
> > > > devices. The 'optional' revert is just reverted to make the main
> > > > reversion trivial.
> > >
> > > Don't want to spoil the party here; but wasn't the optional stuff
> > > removed because it was deemed to be a bad idea?
> >
> > I'm perfectly happy to go either way with this; it maybe a bad idea
> > but it's harmless I think.
> 
> It seems like the original commits were:
>  * patch that does something
>  * patch that removes no-longer used functionality (optional globals)
> 
> so it makes sense to me that if we want to revert the 'patch that
> does something' we should first revert the patch that cleaned
> up unused-functionality (because now we need it again). Is
> that right?
> 
> If optional-globals are a bad idea then we should take another
> run at this for 4.2, but as a "revert stuff for 4.1" strategy
> it seems fine to me.

Functionally both approaches are supposed to be identical, but given
that we already found one last minute problem with the 2nd patch, the
full revert of both feels ever so slightly safer.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|