[libvirt] [PATCH 1/2] qemu: Create hugepage path on per domain basis

[nert at redhat.com]

On Tue, Nov 29, 2016 at 09:28:29AM +0100, Michal Privoznik wrote:
>On 25.11.2016 09:35, Martin Kletzander wrote:
>> On Tue, Nov 22, 2016 at 01:45:42PM +0100, Michal Privoznik wrote:
>>> If you've ever tried running a huge page backed guest under
>>> different user than root, you probably failed. Problem is even
>>
>> Surely you mean different than the default user from qemu.conf.
>>
>>> though we have corresponding APIs in the security drivers,
>>> there's no implementation and thus we don't relabel the huge page
>>> path. But even if we did, so far all of the domains share the
>>> same path:
>>>
>>>   /hugepageMount/libvirt/qemu
>>>
>>> Our only option there would be to set 0777 mode on the qemu dir
>>> which is totally unsafe. Therefore, we can create dir on
>>> per-domain basis, i.e.:
>>>
>>>   /hugepageMount/libvirt/qemu/domainName
>>>
>>> and chown domainName dir to the user that domain is configured to
>>> run under.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>> src/qemu/qemu_command.c                            |  4 +-
>>> src/qemu/qemu_conf.c                               | 45
>>> ++++++++++++++++------
>>> src/qemu/qemu_conf.h                               | 16 +++++---
>>> src/qemu/qemu_driver.c                             | 19 +++------
>>> src/qemu/qemu_process.c                            | 25 +++++++++++-
>>> .../qemuxml2argv-hugepages-numa.args               |  4 +-
>>> .../qemuxml2argv-hugepages-pages.args              | 14 +++----
>>> .../qemuxml2argv-hugepages-pages2.args             |  2 +-
>>> .../qemuxml2argv-hugepages-pages3.args             |  2 +-
>>> .../qemuxml2argv-hugepages-pages5.args             |  2 +-
>>> .../qemuxml2argv-hugepages-shared.args             | 12 +++---
>>> tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
>>> .../qemuxml2argv-memory-hotplug-dimm-addr.args     |  4 +-
>>> .../qemuxml2argv-memory-hotplug-dimm.args          |  4 +-
>>> 14 files changed, 97 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>>> index 0ed88f5..942ad86 100644
>>> --- a/src/qemu/qemu_conf.c
>>> +++ b/src/qemu/qemu_conf.c
>>> @@ -1468,8 +1468,26 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
>>> }
>>>
>>>
>>> +char *
>>> +qemuGetDomainHugepagePath(const virDomainDef *def,
>>> +                          virHugeTLBFSPtr hugepage)
>>> +{
>>> +    char *base = qemuGetBaseHugepagePath(hugepage);
>>> +    char *ret;
>>> +
>>> +    if (!base ||
>>> +        virAsprintf(&ret, "%s/%s", base, def->name) < 0) {
>>> +        VIR_FREE(base);
>>> +        return NULL;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>
>> You can't simply user the name because our restrictions for the name are
>> too lax.  You should get unique directory name usable for this using
>> virDomainObjGetShortName() to make sure the creation doesn't fail.
>
>I thought that when we are using plain domain name for storing domain
>status XML or pid file that I'm safe here too. But okay, I can change
>it. I jut hope that by the time the command line is built domain already
>has id allocated.
>

Oh yeah, I used that because we used a prefix for the name and since you
are not doing that here, there's no need for this function to be used.
Sorry for the confusion.

>>
>> However, that reminds me that you might need to deal with similar thing
>> I had to deal with when adding per-domain subdirectories for private
>> domain paths.  You should save the path (or at least the information
>> that the newer path is used) in the domain object and save/restore it
>> in/from the state XML.  The way it's implemented now will break for
>> example hotplug of hugepage-backed memory after libvirt upgrade.
>
>Not really. We don't expose the path anywhere, and whenever it is needed
>we construct it. I've tested this and basically the only problem I ran
>into was that we don't build the path on domain hotplug (rather than on
>domain startup), but it is trivial to fix.
>

Yep, since we don't need that, we just need to make sure the path exists
(with proper permissions) on hotplug, not only when domain is started.
Exactly as you said ;)

>Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161129/5d1c12ef/attachment-0001.sig>