Will do for all comments - steve
On 7/8/2021 8:01 AM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jul 7, 2021 at 9:35 PM Steve Sistare <steven.sist...@oracle.com
> <mailto:steven.sist...@oracle.com>> wrote:
>
> Add a function that returns true if any ram_list block represents
> volatile memory.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com
> <mailto:steven.sist...@oracle.com>>
> ---
> include/exec/memory.h | 8 ++++++++
> softmmu/memory.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index b116f7c..7ad63f8 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2649,6 +2649,14 @@ bool ram_block_discard_is_disabled(void);
> */
> bool ram_block_discard_is_required(void);
>
> +/**
> + * qemu_ram_volatile: return true if any memory regions are writable and
> not
> + * backed by shared memory.
> + *
> + * @errp: returned error message identifying the bad region.
> + */
> +bool qemu_ram_volatile(Error **errp);
>
>
> Usually, bool-value functions with an error return true on success. If it
> deviates from the recommendation, it should at least be documented.
>
> Also, we have a preference for using _is_ in the function name for such tests.
>
> +
> #endif
>
> #endif
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index f016151..e9536bc 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -2714,6 +2714,36 @@ void memory_global_dirty_log_stop(void)
> memory_global_dirty_log_do_stop();
> }
>
> +/*
> + * Return true if any memory regions are writable and not backed by
> shared
> + * memory.
> + */
>
>
> Let's not duplicate API comments.
>
> +bool qemu_ram_volatile(Error **errp)
> +{
> + RAMBlock *block;
> + MemoryRegion *mr;
> + bool ret = false;
> +
> + rcu_read_lock();
>
>
> RCU_READ_LOCK_GUARD()
>
>
> + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>
>
> RAMBLOCK_FOREACH() should do.
>
> Or rather use the qemu_ram_foreach_block() helper.
>
>
> + mr = block->mr;
> + if (mr &&
> + memory_region_is_ram(mr) &&
> + !memory_region_is_ram_device(mr) &&
> + !memory_region_is_rom(mr) &&
> + (block->fd == -1 || !qemu_ram_is_shared(block))) {
> +
> + error_setg(errp, "Memory region %s is volatile",
> + memory_region_name(mr));
> + ret = true;
> + break;
> + }
> + }
> +
> + rcu_read_unlock();
> + return ret;
> +}
> +
> static void listener_add_address_space(MemoryListener *listener,
> AddressSpace *as)
> {
> --
> 1.8.3.1
>
>
>
>
> --
> Marc-André Lureau
