Re: [RFC v3 5/8] block: add BlockRAMRegistrar

[Hanna Reitz]

On 08.07.22 06:17, Stefan Hajnoczi wrote:
> Emulated devices and other BlockBackend users wishing to take advantage
> of blk_register_buf() all have the same repetitive job: register
> RAMBlocks with the BlockBackend using RAMBlockNotifier.
>
> Add a BlockRAMRegistrar API to do this. A later commit will use this
> from hw/block/virtio-blk.c.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   MAINTAINERS                          |  1 +
>   include/sysemu/block-ram-registrar.h | 30 +++++++++++++++++++++
>   block/block-ram-registrar.c          | 39 ++++++++++++++++++++++++++++
>   block/meson.build                    |  1 +
>   4 files changed, 71 insertions(+)
>   create mode 100644 include/sysemu/block-ram-registrar.h
>   create mode 100644 block/block-ram-registrar.c

What memory is handled in ram_list?  Is it everything?  If so, won’t 
devices have trouble registering all those buffer, especially if they 
happen to be fragmented in physical memory? (nvme_register_buf() seems 
to say it can run out of slots quite easily.)

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50f340d9ee..d16189449f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2490,6 +2490,7 @@ F: block*
>   F: block/
>   F: hw/block/
>   F: include/block/
> +F: include/sysemu/block-*.h
>   F: qemu-img*
>   F: docs/tools/qemu-img.rst
>   F: qemu-io*

Sneaky. ;)

> diff --git a/include/sysemu/block-ram-registrar.h 
> b/include/sysemu/block-ram-registrar.h
> new file mode 100644
> index 0000000000..09d63f64b2
> --- /dev/null
> +++ b/include/sysemu/block-ram-registrar.h
> @@ -0,0 +1,30 @@
> +/*
> + * BlockBackend RAM Registrar
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef BLOCK_RAM_REGISTRAR_H
> +#define BLOCK_RAM_REGISTRAR_H
> +
> +#include "exec/ramlist.h"
> +
> +/**
> + * struct BlockRAMRegistrar:
> + *
> + * Keeps RAMBlock memory registered with a BlockBackend using
> + * blk_register_buf() including hotplugged memory.
> + *
> + * Emulated devices or other BlockBackend users initialize a BlockRAMRegistrar
> + * with blk_ram_registrar_init() before submitting I/O requests with the
> + * BLK_REQ_REGISTERED_BUF flag set.

s/BLK/BDRV/, right?

> + */
> +typedef struct {
> +    BlockBackend *blk;
> +    RAMBlockNotifier notifier;
> +} BlockRAMRegistrar;
> +
> +void blk_ram_registrar_init(BlockRAMRegistrar *r, BlockBackend *blk);
> +void blk_ram_registrar_destroy(BlockRAMRegistrar *r);
> +
> +#endif /* BLOCK_RAM_REGISTRAR_H */