Re: [PATCH V7 01/29] memory: qemu_check_ram_volatile

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Steve Sistare (steven.sistare@oracle.com) wrote:
> Add a function that returns an error if any ram_list block represents
> volatile memory.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  include/exec/memory.h |  8 ++++++++
>  softmmu/memory.c      | 26 ++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 20f1b27..137f5f3 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2981,6 +2981,14 @@ bool ram_block_discard_is_disabled(void);
>   */
>  bool ram_block_discard_is_required(void);
>  
> +/**
> + * qemu_ram_check_volatile: return 1 if any memory regions are writable and not
> + * backed by shared memory, else return 0.
> + *
> + * @errp: returned error message identifying the first volatile region found.
> + */
> +int qemu_check_ram_volatile(Error **errp);
> +
>  #endif
>  
>  #endif
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 7340e19..30b2f68 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -2837,6 +2837,32 @@ void memory_global_dirty_log_stop(unsigned int flags)
>      memory_global_dirty_log_do_stop(flags);
>  }
>  
> +static int check_volatile(RAMBlock *rb, void *opaque)
> +{
> +    MemoryRegion *mr = rb->mr;
> +
> +    if (mr &&
> +        memory_region_is_ram(mr) &&
> +        !memory_region_is_ram_device(mr) &&
> +        !memory_region_is_rom(mr) &&
> +        (rb->fd == -1 || !qemu_ram_is_shared(rb))) {
> +        *(const char **)opaque = memory_region_name(mr);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +int qemu_check_ram_volatile(Error **errp)
> +{
> +    char *name;

Does that need to be const char *name for safety since you're casting
it to it below?

Other than that,


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +
> +    if (qemu_ram_foreach_block(check_volatile, &name)) {
> +        error_setg(errp, "Memory region %s is volatile", name);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static void listener_add_address_space(MemoryListener *listener,
>                                         AddressSpace *as)
>  {
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK