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

[Steven Sistare]

On 2/24/2022 1:28 PM, Dr. David Alan Gilbert wrote:
> * 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?

Will do, thanks.

- Steve

> 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
>>
>>