Re: [PATCH v3 10/21] ram_addr: Split RAMBlock definition

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

* Juan Quintela (quintela@redhat.com) wrote:
> We need some of the fields without having to poison everything else.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

OK.
(I guess the copyright matches the file we're splitting from; would be
good to update the date some time).

I wondered if any of the RAMBlock related function declarations should
move as wlel; but it's not obvious which ones.

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

> ---
>  MAINTAINERS             |  1 +
>  include/exec/ram_addr.h | 40 +-------------------------
>  include/exec/ramblock.h | 64 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 66 insertions(+), 39 deletions(-)
>  create mode 100644 include/exec/ramblock.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2c768ed3d8..3732f746b3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1965,6 +1965,7 @@ F: ioport.c
>  F: include/exec/memop.h
>  F: include/exec/memory.h
>  F: include/exec/ram_addr.h
> +F: include/exec/ramblock.h
>  F: memory.c
>  F: include/exec/memory-internal.h
>  F: exec.c
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5adebb0bc7..5e59a3d8d7 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -24,45 +24,7 @@
>  #include "hw/xen/xen.h"
>  #include "sysemu/tcg.h"
>  #include "exec/ramlist.h"
> -
> -struct RAMBlock {
> -    struct rcu_head rcu;
> -    struct MemoryRegion *mr;
> -    uint8_t *host;
> -    uint8_t *colo_cache; /* For colo, VM's ram cache */
> -    ram_addr_t offset;
> -    ram_addr_t used_length;
> -    ram_addr_t max_length;
> -    void (*resized)(const char*, uint64_t length, void *host);
> -    uint32_t flags;
> -    /* Protected by iothread lock.  */
> -    char idstr[256];
> -    /* RCU-enabled, writes protected by the ramlist lock */
> -    QLIST_ENTRY(RAMBlock) next;
> -    QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
> -    int fd;
> -    size_t page_size;
> -    /* dirty bitmap used during migration */
> -    unsigned long *bmap;
> -    /* bitmap of already received pages in postcopy */
> -    unsigned long *receivedmap;
> -
> -    /*
> -     * bitmap to track already cleared dirty bitmap.  When the bit is
> -     * set, it means the corresponding memory chunk needs a log-clear.
> -     * Set this up to non-NULL to enable the capability to postpone
> -     * and split clearing of dirty bitmap on the remote node (e.g.,
> -     * KVM).  The bitmap will be set only when doing global sync.
> -     *
> -     * NOTE: this bitmap is different comparing to the other bitmaps
> -     * in that one bit can represent multiple guest pages (which is
> -     * decided by the `clear_bmap_shift' variable below).  On
> -     * destination side, this should always be NULL, and the variable
> -     * `clear_bmap_shift' is meaningless.
> -     */
> -    unsigned long *clear_bmap;
> -    uint8_t clear_bmap_shift;
> -};
> +#include "exec/ramblock.h"
>  
>  /**
>   * clear_bmap_size: calculate clear bitmap size
> diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
> new file mode 100644
> index 0000000000..07d50864d8
> --- /dev/null
> +++ b/include/exec/ramblock.h
> @@ -0,0 +1,64 @@
> +/*
> + * Declarations for cpu physical memory functions
> + *
> + * Copyright 2011 Red Hat, Inc. and/or its affiliates
> + *
> + * Authors:
> + *  Avi Kivity <avi@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + *
> + */
> +
> +/*
> + * This header is for use by exec.c and memory.c ONLY.  Do not include it.
> + * The functions declared here will be removed soon.
> + */
> +
> +#ifndef QEMU_EXEC_RAMBLOCK_H
> +#define QEMU_EXEC_RAMBLOCK_H
> +
> +#ifndef CONFIG_USER_ONLY
> +#include "cpu-common.h"
> +
> +struct RAMBlock {
> +    struct rcu_head rcu;
> +    struct MemoryRegion *mr;
> +    uint8_t *host;
> +    uint8_t *colo_cache; /* For colo, VM's ram cache */
> +    ram_addr_t offset;
> +    ram_addr_t used_length;
> +    ram_addr_t max_length;
> +    void (*resized)(const char*, uint64_t length, void *host);
> +    uint32_t flags;
> +    /* Protected by iothread lock.  */
> +    char idstr[256];
> +    /* RCU-enabled, writes protected by the ramlist lock */
> +    QLIST_ENTRY(RAMBlock) next;
> +    QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
> +    int fd;
> +    size_t page_size;
> +    /* dirty bitmap used during migration */
> +    unsigned long *bmap;
> +    /* bitmap of already received pages in postcopy */
> +    unsigned long *receivedmap;
> +
> +    /*
> +     * bitmap to track already cleared dirty bitmap.  When the bit is
> +     * set, it means the corresponding memory chunk needs a log-clear.
> +     * Set this up to non-NULL to enable the capability to postpone
> +     * and split clearing of dirty bitmap on the remote node (e.g.,
> +     * KVM).  The bitmap will be set only when doing global sync.
> +     *
> +     * NOTE: this bitmap is different comparing to the other bitmaps
> +     * in that one bit can represent multiple guest pages (which is
> +     * decided by the `clear_bmap_shift' variable below).  On
> +     * destination side, this should always be NULL, and the variable
> +     * `clear_bmap_shift' is meaningless.
> +     */
> +    unsigned long *clear_bmap;
> +    uint8_t clear_bmap_shift;
> +};
> +#endif
> +#endif
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK