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 evenSurely 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.
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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
