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

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(a)redhat.com&gt; > --- > 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.

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.