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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
