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