[libvirt] [PATCH] virfile: relax checks for hugetlbfs

[Michal Privoznik]

On 12/04/2017 04:21 PM, David Vrabel wrote:
> There are use cases where it is useful to use the support in libvirt
> for file-backed guest memory, but without using hugetlbfs but tmpfs
> instead (for example, to run tests on hosts that do not have hugepages
> configured, or to use Linux's idle page tracking to monitor guest
> memory accesses at a 4k page granularity.).
> 
> Drop the check for hugetlbfs when querying the huge page size, but
> move it to the loop that's searching for a suitable mount to use.
> 
> Change-Id: I2c9589191e14653724725b944171689553ee6bae
> ---
>  src/util/virfile.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> > diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 82cb36dbc..24ff5e208 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3438,19 +3438,23 @@ virFileGetHugepageSize(const char *path,
>          goto cleanup;
>      }
>  
> -    if (fs.f_type != HUGETLBFS_MAGIC) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("not a hugetlbfs mount: '%s'"),
> -                       path);
> -        goto cleanup;
> -    }
> -
>      *size = fs.f_bsize / 1024; /* we are storing size in KiB */
>      ret = 0;
>   cleanup:
>      return ret;
>  }
>  
> +static bool
> +virFileIsHugeTLBFS(const char *path)
> +{
> +    struct statfs fs;
> +
> +    if (statfs(path, &fs) < 0) {
> +        return false;
> +    }


Note that we don't put curly braces around one line bodies.

> +    return fs.f_type == HUGETLBFS_MAGIC;
> +}
> +
>  # define PROC_MEMINFO "/proc/meminfo"
>  # define HUGEPAGESIZE_STR "Hugepagesize:"
>  
> @@ -3517,6 +3521,9 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
>          if (STRNEQ(mb.mnt_type, "hugetlbfs"))
>              continue;
>  
> +        if (!virFileIsHugeTLBFS(mb.mnt_dir))
> +            continue;
> +
>          if (VIR_EXPAND_N(fs, nfs, 1) < 0)
>               goto cleanup;
>  
> 

Frankly, I'm not a fan of this patch. Currently, when people have
<hugepages/> in their domain XML we guarantee that hugepages are used to
back the guest memory. With this patch users might be mislead as it
enables *any* other filesystem. All that's needed is to put mount points
into qemu.conf. I know you want it for testing, but is it an upstream
material? IOW, I'm not nacking this, I just want some conversation to
happen first.

Michal