Re: [PATCH v2 2/4] virtio-mem: Skip most of virtio_mem_unplug_all() without plugged memory

[Juan Quintela <quintela@redhat.com>]

David Hildenbrand <david@redhat.com> wrote:
> Already when starting QEMU we perform one system reset that ends up
> triggering virtio_mem_unplug_all() with no actual memory plugged yet.
> That, in turn will trigger ram_block_discard_range() and perform some
> other actions that are not required in that case.
>
> Let's optimize virtio_mem_unplug_all() for the case that no memory is
> plugged. This will be beneficial for x-ignore-shared support as well.
>
> Tested-by: Mario Casquero <mcasquer@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

It works, so ...

Reviewed-by: Juan Quintela <quintela@redhat.com>

>      RAMBlock *rb = vmem->memdev->mr.ram_block;
>  
> -    if (virtio_mem_is_busy()) {
> -        return -EBUSY;
> -    }
> -
> -    if (ram_block_discard_range(rb, 0, qemu_ram_get_used_length(rb))) {
> -        return -EBUSY;
> -    }
> -    virtio_mem_notify_unplug_all(vmem);
> -
> -    bitmap_clear(vmem->bitmap, 0, vmem->bitmap_size);
>      if (vmem->size) {
> +        if (virtio_mem_is_busy()) {
> +            return -EBUSY;

I see that the only way that virtio_men_is_busy() is true is if we are
in the middle of a migration.  In the case that vmem is 0, we don't
care.  So we are good.



> +        }
> +        if (ram_block_discard_range(rb, 0, qemu_ram_get_used_length(rb))) {
> +            return -EBUSY;
> +        }

Nothing to discard, so also good.

> +        virtio_mem_notify_unplug_all(vmem);

Nothing to notify, so also good.

> +        bitmap_clear(vmem->bitmap, 0, vmem->bitmap_size);
>          vmem->size = 0;
>          notifier_list_notify(&vmem->size_change_notifiers, &vmem->size);
>      }
> +
>      trace_virtio_mem_unplugged_all();
>      virtio_mem_resize_usable_region(vmem, vmem->requested_size, true);
>      return 0;

Once that we are here.  Do you remember _why_ do we allow virtio-mem
plug/unplug in the middle of a migration.

We forbid to plug/unplug everything else.  Why do we need to plug/unplug
virtio-mem during migration?

Thanks, Juan.