Re: [Qemu-devel] [PATCH] ivshmem: Fix unplug of device "ivshmem-plain"

Re: [Qemu-devel] [PATCH] ivshmem: Fix unplug of device "ivshmem-plain"

[Paolo Bonzini]

On 26/09/2018 18:37, Markus Armbruster wrote:
> Commit 2aece63c8a "hostmem: detect host backend memory is being used
> properly" fixed "ivshmem-plain" to reject memory backends that are
> already in use, and to block their deletion while in use.  Two bugs
> escaped review:
> 
> * New ivshmem_plain_exit() fails to call ivshmem_exit().  This breaks
>   unplug.  Reproducer: migration after unplug still fails with
>   "Migration is disabled when using feature 'peer mode' in device
>   'ivshmem'".
> 
> * It failed to update legacy "ivshmem".  Harmless, because it creates
>   the memory backend itself, and nothing else should use it.
> 
> Fix by moving the two host_memory_backend_set_mapped() calls into
> ivshmem_common_realize() and ivshmem_exit(), guarded by s->hostmem.
> 
> Fixes: 2aece63c8a9d2c3a8ff41d2febc4cdeff2633331
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/misc/ivshmem.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 6febbabcaa..8cb17b9dd4 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -911,6 +911,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>          IVSHMEM_DPRINTF("using hostmem\n");
>  
>          s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem);
> +        host_memory_backend_set_mapped(s->hostmem, true);
>      } else {
>          Chardev *chr = qemu_chr_fe_get_driver(&s->server_chr);
>          assert(chr);
> @@ -993,6 +994,10 @@ static void ivshmem_exit(PCIDevice *dev)
>          vmstate_unregister_ram(s->ivshmem_bar2, DEVICE(dev));
>      }
>  
> +    if (s->hostmem) {
> +        host_memory_backend_set_mapped(s->hostmem, false);
> +    }
> +
>      if (s->peers) {
>          for (i = 0; i < s->nb_peers; i++) {
>              close_peer_eventfds(s, i);
> @@ -1101,14 +1106,6 @@ static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
>      }
>  
>      ivshmem_common_realize(dev, errp);
> -    host_memory_backend_set_mapped(s->hostmem, true);
> -}
> -
> -static void ivshmem_plain_exit(PCIDevice *pci_dev)
> -{
> -    IVShmemState *s = IVSHMEM_COMMON(pci_dev);
> -
> -    host_memory_backend_set_mapped(s->hostmem, false);
>  }
>  
>  static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
> @@ -1117,7 +1114,6 @@ static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
>      k->realize = ivshmem_plain_realize;
> -    k->exit = ivshmem_plain_exit;
>      dc->props = ivshmem_plain_properties;
>      dc->vmsd = &ivshmem_plain_vmsd;
>  }
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks for the fix!

Paolo