[libvirt] [PATCH 5/5] qemuBuildMemoryBackendProps: Get pagesize early

[Michal Privoznik]

On 4/2/19 2:57 PM, Ján Tomko wrote:
> On Mon, Apr 01, 2019 at 04:04:37PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1693066
>>
>> Up until memfd introduction (in 24b74d187ca) we did not need to
>> know @pagesize because qemuGetDomainHupageMemPath() could deal
>> with it being zero (value of zero means use the default hugetlbfs
>> mount). But since for memfd we are not passing a path to
>> hugetlbfs mount rather the page size value we need to know its
>> value upfront.
> 
> This phrasing makes it seem like getting the pagesize is being moved,
> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>> src/qemu/qemu_command.c                       | 14 +++++++
>> ...memory-default-hugepage.x86_64-latest.args | 37 ++++++++++++++++
>> .../memfd-memory-default-hugepage.xml         | 42 +++++++++++++++++++
>> tests/qemuxml2argvtest.c                      |  1 +
>> .../memfd-memory-default-hugepage.xml         |  1 +
>> tests/qemuxml2xmltest.c                       |  3 ++
>> 6 files changed, 98 insertions(+)
> 
> but there are no deletions. I'd expect that now it will no longer be
> needed in qemuGetDomainHupageMemPath (sic).

In fact, it is. See below.

> 
>> create mode 100644 
>> tests/qemuxml2argvdata/memfd-memory-default-hugepage.x86_64-latest.args
>> create mode 100644 
>> tests/qemuxml2argvdata/memfd-memory-default-hugepage.xml
>> create mode 120000 
>> tests/qemuxml2xmloutdata/memfd-memory-default-hugepage.xml
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index f81d20e5f7..ba1b56c2da 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3547,6 +3547,20 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
>> *backendProps,
>>         pagesize = 0;
>>         needHugepage = false;
>>         useHugepage = false;
>> +    } else if (pagesize == 0) {
>> +        virHugeTLBFSPtr p;
>> +
>> +        if (!cfg->nhugetlbfs) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                           "%s", _("hugetlbfs filesystem is not 
>> mounted "
>> +                                   "or disabled by administrator 
>> config"));
>> +            goto cleanup;
>> +        }
>> +
>> +        if (!(p = virFileGetDefaultHugepage(cfg->hugetlbfs, 
>> cfg->nhugetlbfs)))
>> +            p = &cfg->hugetlbfs[0];
>> +
>> +        pagesize = p->size;
> 
> Given how big and beautiful qemuBuildMemoryBackendProps has become,
> this would look nicer in another wrapper.

Okay.

> 
> With the corresponding deletions in qemuGetDomainHupageMemPath:

qemuGetDomainHupageMemPath is called when building -mem-path too 
(qemuBuildMemPathStr). And that's the place where pagesize can be 0 too.

> Reviewed-by: Ján Tomko <jtomko at redhat.com>

Thanks.

Michal