Re: [uq/master patch 2/5] kvm: add logging count to slots

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 5024 bytes --]

Marcelo Tosatti wrote:
> Otherwise there is no way to differentiate between global and slot 
> specific logging, so for example 
> 
> vga dirty log start
> migration start
> migration fail
> 
> Disables dirty logging for the vga slot.

This is not true (unless there is a bug): Migration logging is tracked
via KVMState::migration_log and vga logging via KVMSlot::flags. Both are
merged in kvm_set_user_memory_region. Thus no such change is required
for upstream.

Jan

> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Index: qemu-kvm/kvm-all.c
> ===================================================================
> --- qemu-kvm.orig/kvm-all.c
> +++ qemu-kvm/kvm-all.c
> @@ -47,6 +47,7 @@ typedef struct KVMSlot
>      ram_addr_t phys_offset;
>      int slot;
>      int flags;
> +    int logging_count;
>  } KVMSlot;
>  
>  typedef struct kvm_dirty_log KVMDirtyLog;
> @@ -218,20 +219,11 @@ err:
>  /*
>   * dirty pages logging control
>   */
> -static int kvm_dirty_pages_log_change(target_phys_addr_t phys_addr,
> -                                      ram_addr_t size, int flags, int mask)
> +static int kvm_dirty_pages_log_change(KVMSlot *mem, int flags, int mask)
>  {
>      KVMState *s = kvm_state;
> -    KVMSlot *mem = kvm_lookup_matching_slot(s, phys_addr, phys_addr + size);
>      int old_flags;
>  
> -    if (mem == NULL)  {
> -            fprintf(stderr, "BUG: %s: invalid parameters " TARGET_FMT_plx "-"
> -                    TARGET_FMT_plx "\n", __func__, phys_addr,
> -                    (target_phys_addr_t)(phys_addr + size - 1));
> -            return -EINVAL;
> -    }
> -
>      old_flags = mem->flags;
>  
>      flags = (mem->flags & ~mask) | flags;
> @@ -250,16 +242,42 @@ static int kvm_dirty_pages_log_change(ta
>  
>  int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
>  {
> -        return kvm_dirty_pages_log_change(phys_addr, size,
> -                                          KVM_MEM_LOG_DIRTY_PAGES,
> -                                          KVM_MEM_LOG_DIRTY_PAGES);
> +    KVMState *s = kvm_state;
> +    KVMSlot *mem = kvm_lookup_matching_slot(s, phys_addr, phys_addr + size);
> +
> +    if (mem == NULL)  {
> +            fprintf(stderr, "BUG: %s: invalid parameters " TARGET_FMT_plx "-"
> +                    TARGET_FMT_plx "\n", __func__, phys_addr,
> +                    (target_phys_addr_t)(phys_addr + size - 1));
> +            return -EINVAL;
> +    }
> +
> +    if (mem->logging_count++)
> +        return 0;
> +
> +    return kvm_dirty_pages_log_change(mem,
> +                                      KVM_MEM_LOG_DIRTY_PAGES,
> +                                      KVM_MEM_LOG_DIRTY_PAGES);
>  }
>  
>  int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
>  {
> -        return kvm_dirty_pages_log_change(phys_addr, size,
> -                                          0,
> -                                          KVM_MEM_LOG_DIRTY_PAGES);
> +    KVMState *s = kvm_state;
> +    KVMSlot *mem = kvm_lookup_matching_slot(s, phys_addr, phys_addr + size);
> +
> +    if (mem == NULL)  {
> +            fprintf(stderr, "BUG: %s: invalid parameters " TARGET_FMT_plx "-"
> +                    TARGET_FMT_plx "\n", __func__, phys_addr,
> +                    (target_phys_addr_t)(phys_addr + size - 1));
> +            return -EINVAL;
> +    }
> +
> +    if (--mem->logging_count)
> +        return 0;
> +
> +    return kvm_dirty_pages_log_change(mem,
> +                                      0,
> +                                      KVM_MEM_LOG_DIRTY_PAGES);
>  }
>  
>  static int kvm_set_migration_log(int enable)
> @@ -273,12 +291,15 @@ static int kvm_set_migration_log(int ena
>      for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
>          mem = &s->slots[i];
>  
> -        if (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) {
> -            continue;
> -        }
> -        err = kvm_set_user_memory_region(s, mem);
> -        if (err) {
> -            return err;
> +        if (mem->memory_size) {
> +            if (enable) {
> +                err = kvm_log_start(mem->start_addr, mem->memory_size);
> +            } else {
> +                err = kvm_log_stop(mem->start_addr, mem->memory_size);
> +            }
> +            if (err) {
> +                return err;
> +            }
>          }
>      }
>      return 0;
> @@ -442,6 +463,7 @@ static void kvm_set_phys_mem(target_phys
>  
>          /* unregister the overlapping slot */
>          mem->memory_size = 0;
> +        mem->logging_count = 0;
>          err = kvm_set_user_memory_region(s, mem);
>          if (err) {
>              fprintf(stderr, "%s: error unregistering overlapping slot: %s\n",
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]