Re: [Qemu-devel] [PATCH] kvm-all: put kvm_mem_flags to more work

[Paolo Bonzini]

On 06/03/2015 16:17, Andrew Jones wrote:
> Currently kvm_mem_flags just translates bools to bits, let's
> make it also determine the bools first. This avoids its parameter
> list growing each time we add a flag.
> 
> Signed-off-by: Andrew Jones <address@hidden>
> ---
> This patch comes from an experimental series that added a kvm
> memslot flag. That series probably won't ever be merged, but
> this patch on it's own seems like a nice cleanup to me.
> 
>  kvm-all.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)

I'm putting this on hold for a while because I have some other
conflicting changes around memory_region_is_logging().  But I'll pick it
up and rebase as soon as those are finalized.

Paolo

> diff --git a/kvm-all.c b/kvm-all.c
> index 05a79c20e0bba..507fa7204e062 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -294,10 +294,14 @@ err:
>   * dirty pages logging control
>   */
>  
> -static int kvm_mem_flags(KVMState *s, bool log_dirty, bool readonly)
> +static int kvm_mem_flags(MemoryRegion *mr)
>  {
> +    bool readonly = mr->readonly || memory_region_is_romd(mr);
>      int flags = 0;
> -    flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
> +
> +    if (memory_region_is_logging(mr)) {
> +        flags |= KVM_MEM_LOG_DIRTY_PAGES;
> +    }
>      if (readonly && kvm_readonly_mem_allowed) {
>          flags |= KVM_MEM_READONLY;
>      }
> @@ -312,7 +316,10 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, 
> bool log_dirty)
>  
>      old_flags = mem->flags;
>  
> -    flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false);
> +    flags = mem->flags & ~mask;
> +    if (log_dirty) {
> +        flags |= KVM_MEM_LOG_DIRTY_PAGES;
> +    }
>      mem->flags = flags;
>  
>      /* If nothing changed effectively, no need to issue ioctl */
> @@ -642,9 +649,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>      KVMSlot *mem, old;
>      int err;
>      MemoryRegion *mr = section->mr;
> -    bool log_dirty = memory_region_is_logging(mr);
>      bool writeable = !mr->readonly && !mr->rom_device;
> -    bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
>      hwaddr start_addr = section->offset_within_address_space;
>      ram_addr_t size = int128_get64(section->size);
>      void *ram = NULL;
> @@ -688,7 +693,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>              (ram - start_addr == mem->ram - mem->start_addr)) {
>              /* The new slot fits into the existing one and comes with
>               * identical parameters - update flags and done. */
> -            kvm_slot_dirty_pages_log_change(mem, log_dirty);
> +            kvm_slot_dirty_pages_log_change(mem, 
> memory_region_is_logging(mr));
>              return;
>          }
>  
> @@ -721,7 +726,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>              mem->memory_size = old.memory_size;
>              mem->start_addr = old.start_addr;
>              mem->ram = old.ram;
> -            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
> +            mem->flags = kvm_mem_flags(mr);
>  
>              err = kvm_set_user_memory_region(s, mem);
>              if (err) {
> @@ -742,7 +747,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>              mem->memory_size = start_addr - old.start_addr;
>              mem->start_addr = old.start_addr;
>              mem->ram = old.ram;
> -            mem->flags =  kvm_mem_flags(s, log_dirty, readonly_flag);
> +            mem->flags =  kvm_mem_flags(mr);
>  
>              err = kvm_set_user_memory_region(s, mem);
>              if (err) {
> @@ -766,7 +771,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>              size_delta = mem->start_addr - old.start_addr;
>              mem->memory_size = old.memory_size - size_delta;
>              mem->ram = old.ram + size_delta;
> -            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
> +            mem->flags = kvm_mem_flags(mr);
>  
>              err = kvm_set_user_memory_region(s, mem);
>              if (err) {
> @@ -788,7 +793,7 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>      mem->memory_size = size;
>      mem->start_addr = start_addr;
>      mem->ram = ram;
> -    mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
> +    mem->flags = kvm_mem_flags(mr);
>  
>      err = kvm_set_user_memory_region(s, mem);
>      if (err) {
>