Re: [Qemu-devel] [PATCH v7 07/35] util: introduce qemu_file_get_page_size()

[Eduardo Habkost]

On Mon, Nov 02, 2015 at 05:13:09PM +0800, Xiao Guangrong wrote:
> There are three places use the some logic to get the page size on
> the file path or file fd
> 
> Windows did not support file hugepage, so it will return normal page
> for this case. And this interface has not been used on windows so far
>  
> This patch introduces qemu_file_get_page_size() to unify the code
> 
> Signed-off-by: Xiao Guangrong <address@hidden>
[...]
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 914cef5..51437ff 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -340,7 +340,7 @@ static void sigbus_handler(int signal)
>      siglongjmp(sigjump, 1);
>  }
>  
> -static size_t fd_getpagesize(int fd)
> +static size_t fd_getpagesize(int fd, Error **errp)
>  {
>  #ifdef CONFIG_LINUX
>      struct statfs fs;
> @@ -351,7 +351,12 @@ static size_t fd_getpagesize(int fd)
>              ret = fstatfs(fd, &fs);
>          } while (ret != 0 && errno == EINTR);
>  
> -        if (ret == 0 && fs.f_type == HUGETLBFS_MAGIC) {
> +        if (ret) {
> +            error_setg_errno(errp, errno, "fstatfs is failed");
> +            return 0;
> +        }
> +
> +        if (fs.f_type == HUGETLBFS_MAGIC) {
>              return fs.f_bsize;
>          }

You are changing os_mem_prealloc() behavior when fstatfs() fails, here.
Have you ensured there are no cases where fstatfs() fails but this code
is still expected to work?

The change looks safe: gethugepagesize() seems to be always called in
the path that would make fd_getpagesize() be called from
os_mem_prealloc(), so allocation would abort much earlier if statfs()
failed. But I haven't confirmed that yet, and I wanted to be sure.


-- 
Eduardo