kvm-all: Partially reverts 4fe6d78b2e to remove the cleanup call

[Qemu-devel] [PATCH v2] kvm-all: Partially reverts 4fe6d78b2e to remove the cleanup call

Posted by Jose Ricardo Ziviani 6 years, 3 months ago

This commit partially reverts the commit 4fe6d78b2e because of issues
reported in the virtio.

Examples:

$ qemu-system-ppc64 -cpu POWER8 -nographic -vga none -m 4G \
  -M pseries,accel=kvm -netdev type=user,id=net0 \
  -device virtio-net-pci,netdev=net0 -drive file=../disk.qcow2,if=virtio

Populating /vdevice/nvram@71000001
Populating /vdevice/v-scsi@71000002
       SCSI: Looking for devices
          8200000000000000 CD-ROM   : "QEMU     QEMU CD-ROM      2.5+"
Populating /pci@800000020000000
                     00 0000 (D) : 1af4 1000    virtio [ net ]
Aborted

$ qemu-system-x86_64 -m 4G -enable-kvm -drive file=util.qcow2,if=virtio

Running QEMU with GTK 2.x is deprecated, and will be removed
in a future release. Please switch to GTK 3.x instead
[1]    5282 abort

Reference http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05457.html

Reported-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
 accel/kvm/kvm-all.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 071f4f57c0..f290f487a5 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -812,10 +812,6 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
     if (r < 0) {
         abort();
     }
-
-    if (e->cleanup) {
-        e->cleanup(e);
-    }
 }
 
 static void kvm_io_ioeventfd_add(MemoryListener *listener,
-- 
2.14.3

Re: [Qemu-devel] [PATCH v2] kvm-all: Partially reverts 4fe6d78b2e to remove the cleanup call

Posted by Daniel Henrique Barboza 6 years, 3 months ago

On 01/23/2018 03:54 PM, Jose Ricardo Ziviani wrote:
> This commit partially reverts the commit 4fe6d78b2e because of issues
> reported in the virtio.
>
> Examples:
>
> $ qemu-system-ppc64 -cpu POWER8 -nographic -vga none -m 4G \
>    -M pseries,accel=kvm -netdev type=user,id=net0 \
>    -device virtio-net-pci,netdev=net0 -drive file=../disk.qcow2,if=virtio
>
> Populating /vdevice/nvram@71000001
> Populating /vdevice/v-scsi@71000002
>         SCSI: Looking for devices
>            8200000000000000 CD-ROM   : "QEMU     QEMU CD-ROM      2.5+"
> Populating /pci@800000020000000
>                       00 0000 (D) : 1af4 1000    virtio [ net ]
> Aborted
>
> $ qemu-system-x86_64 -m 4G -enable-kvm -drive file=util.qcow2,if=virtio
>
> Running QEMU with GTK 2.x is deprecated, and will be removed
> in a future release. Please switch to GTK 3.x instead
> [1]    5282 abort
>
> Reference http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05457.html
>
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---

This patch also fixed a migration problem I was investigating today. 
Turns out 4fe6d78b2e
is also to be blamed for this P8 -> P8 migration being broken in current 
master:


$ sudo ./qemu-system-ppc64 --enable-kvm --nographic -vga none -machine 
pseries -m 4G,slots=32,maxmem=32G -smp 1,maxcpus=32 -device 
virtio-blk-pci,drive=rootdisk -drive 
file=/home/danielhb/vm_imgs/f26.qcow2,if=none,cache=none,format=qcow2,id=rootdisk 
-incoming tcp:0:4444
qemu-system-ppc64: load of migration failed: Input/output error


Reverting 4fe6d78b2e or applying this patch fixes this migration issue too.


Reviewed-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Tested-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>


>   accel/kvm/kvm-all.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 071f4f57c0..f290f487a5 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -812,10 +812,6 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
>       if (r < 0) {
>           abort();
>       }
> -
> -    if (e->cleanup) {
> -        e->cleanup(e);
> -    }
>   }
>
>   static void kvm_io_ioeventfd_add(MemoryListener *listener,

Re: [Qemu-devel] [PATCH v2] kvm-all: Partially reverts 4fe6d78b2e to remove the cleanup call

Posted by Paolo Bonzini 6 years, 3 months ago

On 23/01/2018 18:54, Jose Ricardo Ziviani wrote:
> This commit partially reverts the commit 4fe6d78b2e because of issues
> reported in the virtio.
> 
> Examples:
> 
> $ qemu-system-ppc64 -cpu POWER8 -nographic -vga none -m 4G \
>   -M pseries,accel=kvm -netdev type=user,id=net0 \
>   -device virtio-net-pci,netdev=net0 -drive file=../disk.qcow2,if=virtio
> 
> Populating /vdevice/nvram@71000001
> Populating /vdevice/v-scsi@71000002
>        SCSI: Looking for devices
>           8200000000000000 CD-ROM   : "QEMU     QEMU CD-ROM      2.5+"
> Populating /pci@800000020000000
>                      00 0000 (D) : 1af4 1000    virtio [ net ]
> Aborted
> 
> $ qemu-system-x86_64 -m 4G -enable-kvm -drive file=util.qcow2,if=virtio
> 
> Running QEMU with GTK 2.x is deprecated, and will be removed
> in a future release. Please switch to GTK 3.x instead
> [1]    5282 abort
> 
> Reference http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05457.html
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
>  accel/kvm/kvm-all.c | 4 ----
>  1 file changed, 4 deletions(-)

Michael, please send a revert for all three of Gal's patches.

Thanks,

Paolo

> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 071f4f57c0..f290f487a5 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -812,10 +812,6 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
>      if (r < 0) {
>          abort();
>      }
> -
> -    if (e->cleanup) {
> -        e->cleanup(e);
> -    }
>  }
>  
>  static void kvm_io_ioeventfd_add(MemoryListener *listener,
>

Re: [Qemu-devel] [PATCH v2] kvm-all: Partially reverts 4fe6d78b2e to remove the cleanup call

Posted by Michael S. Tsirkin 6 years, 3 months ago

On Wed, Jan 24, 2018 at 10:40:14AM +0100, Paolo Bonzini wrote:
> On 23/01/2018 18:54, Jose Ricardo Ziviani wrote:
> > This commit partially reverts the commit 4fe6d78b2e because of issues
> > reported in the virtio.
> > 
> > Examples:
> > 
> > $ qemu-system-ppc64 -cpu POWER8 -nographic -vga none -m 4G \
> >   -M pseries,accel=kvm -netdev type=user,id=net0 \
> >   -device virtio-net-pci,netdev=net0 -drive file=../disk.qcow2,if=virtio
> > 
> > Populating /vdevice/nvram@71000001
> > Populating /vdevice/v-scsi@71000002
> >        SCSI: Looking for devices
> >           8200000000000000 CD-ROM   : "QEMU     QEMU CD-ROM      2.5+"
> > Populating /pci@800000020000000
> >                      00 0000 (D) : 1af4 1000    virtio [ net ]
> > Aborted
> > 
> > $ qemu-system-x86_64 -m 4G -enable-kvm -drive file=util.qcow2,if=virtio
> > 
> > Running QEMU with GTK 2.x is deprecated, and will be removed
> > in a future release. Please switch to GTK 3.x instead
> > [1]    5282 abort
> > 
> > Reference http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05457.html
> > 
> > Reported-by: Anton Blanchard <anton@samba.org>
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> >  accel/kvm/kvm-all.c | 4 ----
> >  1 file changed, 4 deletions(-)
> 
> Michael, please send a revert for all three of Gal's patches.
> 
> Thanks,
> 
> Paolo

Yes, I've done exactly that.
Thanks for the suggestion.

> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 071f4f57c0..f290f487a5 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -812,10 +812,6 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
> >      if (r < 0) {
> >          abort();
> >      }
> > -
> > -    if (e->cleanup) {
> > -        e->cleanup(e);
> > -    }
> >  }
> >  
> >  static void kvm_io_ioeventfd_add(MemoryListener *listener,
> >