Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()

[Michael S. Tsirkin]

On Fri, Oct 30, 2015 at 01:56:01PM +0800, Xiao Guangrong wrote:
> There are three places use the some logic to get the page size on
> the file path or file fd
> 
> This patch introduces qemu_file_get_page_size() to unify the code
> 
> Signed-off-by: Xiao Guangrong <address@hidden>
> ---
>  include/qemu/osdep.h |  1 +
>  target-ppc/kvm.c     | 21 +++------------------
>  util/oslib-posix.c   | 16 ++++++++++++++++
>  util/oslib-win32.c   |  5 +++++
>  4 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b568424..d4dde02 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -302,4 +302,5 @@ int qemu_read_password(char *buf, int buf_size);
>   */
>  pid_t qemu_fork(Error **errp);
>  
> +size_t qemu_file_get_page_size(const char *mem_path);
>  #endif
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index ac70f08..c661f1c 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -308,28 +308,13 @@ static void kvm_get_smmu_info(PowerPCCPU *cpu, struct 
> kvm_ppc_smmu_info *info)
>  
>  static long gethugepagesize(const char *mem_path)
>  {
> -    struct statfs fs;
> -    int ret;
> -
> -    do {
> -        ret = statfs(mem_path, &fs);
> -    } while (ret != 0 && errno == EINTR);
> +    long size = qemu_file_get_page_size(mem_path);
>  
> -    if (ret != 0) {
> -        fprintf(stderr, "Couldn't statfs() memory path: %s\n",
> -                strerror(errno));
> +    if (!size) {
>          exit(1);
>      }
>  
> -#define HUGETLBFS_MAGIC       0x958458f6
> -
> -    if (fs.f_type != HUGETLBFS_MAGIC) {
> -        /* Explicit mempath, but it's ordinary pages */
> -        return getpagesize();
> -    }
> -
> -    /* It's hugepage, return the huge page size */
> -    return fs.f_bsize;
> +    return size;
>  }
>  
>  static int find_max_supported_pagesize(Object *obj, void *opaque)
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 914cef5..ad94c5a 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -360,6 +360,22 @@ static size_t fd_getpagesize(int fd)
>      return getpagesize();
>  }
>  
> +size_t qemu_file_get_page_size(const char *path)
> +{
> +    size_t size = 0;
> +    int fd = qemu_open(path, O_RDONLY);
> +
> +    if (fd < 0) {
> +        fprintf(stderr, "Could not open %s.\n", path);
> +        goto exit;
> +    }
> +
> +    size = fd_getpagesize(fd);
> +    qemu_close(fd);
> +exit:
> +    return size;
> +}
> +
>  void os_mem_prealloc(int fd, char *area, size_t memory)
>  {
>      int ret;

So this is opening the file for the sole purpose of
doing the fstatfs on it. Seems strange, just do statfs instead.
In fact, maybe we want statfs_getpagesize.

> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 09f9e98..a18aa87 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -462,6 +462,11 @@ size_t getpagesize(void)
>      return system_info.dwPageSize;
>  }
>  
> +size_t qemu_file_get_page_size(const char *path)
> +{
> +    return getpagesize();
> +}
> +
>  void os_mem_prealloc(int fd, char *area, size_t memory)
>  {
>      int i;

And why is this needed on win32?

> -- 
> 1.8.3.1