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

David Vrabel david.vrabel at nutanix.com
Fri Dec 15 04:50:54 UTC 2017


On 06/12/17 14:55, Michal Privoznik wrote:
> 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.

I think this is a fair point.  If a system inadvertently loses its
/dev/hugepages mount, the VMs shouldn't start memory backed by the
filesystem in /dev.  This could result in hard-to-diagnose problems.

Not sure I'll be able to find time to rework this patch to add new
configuration options, though.

David




More information about the libvir-list mailing list