On 03/09/2017 11:06 AM, Michal Privoznik wrote: > Some users might want to pass a blockdev or a chardev as a > backend for NVDIMM. In fact, this is expected to be the mostly > used configuration. Therefore libvirt should allow the device in > devices CGroup then. > > Signed-off-by: Michal Privoznik <mpriv...@redhat.com> > --- > src/qemu/qemu_cgroup.c | 49 > +++++++++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_cgroup.h | 4 ++++ > src/qemu/qemu_hotplug.c | 10 ++++++++++ > 3 files changed, 63 insertions(+) > > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index 42a47a798..36762d4f6 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -348,6 +348,50 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, > } > > > +int > +qemuSetupMemoryDevicesCgroup(virDomainObjPtr vm, > + virDomainMemoryDefPtr mem) > +{ > + qemuDomainObjPrivatePtr priv = vm->privateData; > + int rv; > + > + if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM) > + return 0; > + > + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) > + return 0; > + > + VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s", > mem->nvdimmPath); > + rv = virCgroupAllowDevicePath(priv->cgroup, mem->nvdimmPath, > + VIR_CGROUP_DEVICE_RW, false); > + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", > + mem->nvdimmPath, "rw", rv == 0); > + > + return rv; > +} > + > + > +int > +qemuTeardownMemoryDevicesCgroup(virDomainObjPtr vm, > + virDomainMemoryDefPtr mem) > +{ > + qemuDomainObjPrivatePtr priv = vm->privateData; > + int rv; > + > + if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM) > + return 0; > + > + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) > + return 0; > + > + rv = virCgroupDenyDevicePath(priv->cgroup, mem->nvdimmPath, > + VIR_CGROUP_DEVICE_RWM, false); > + virDomainAuditCgroupPath(vm, priv->cgroup, > + "deny", mem->nvdimmPath, "rwm", rv == 0); > + return rv; > +} > + > + > static int > qemuSetupGraphicsCgroup(virDomainObjPtr vm, > virDomainGraphicsDefPtr gfx) > @@ -647,6 +691,11 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, > goto cleanup; > } > > + for (i = 0; i < vm->def->nmems; i++) { > + if (qemuSetupMemoryDevicesCgroup(vm, vm->def->mems[i]) < 0) > + goto cleanup; > + } > + > for (i = 0; i < vm->def->ngraphics; i++) { > if (qemuSetupGraphicsCgroup(vm, vm->def->graphics[i]) < 0) > goto cleanup; > diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h > index 8ae4a72ab..d016ce29d 100644 > --- a/src/qemu/qemu_cgroup.h > +++ b/src/qemu/qemu_cgroup.h > @@ -43,6 +43,10 @@ int qemuSetupHostdevCgroup(virDomainObjPtr vm, > int qemuTeardownHostdevCgroup(virDomainObjPtr vm, > virDomainHostdevDefPtr dev) > ATTRIBUTE_RETURN_CHECK; > +int qemuSetupMemoryDevicesCgroup(virDomainObjPtr vm, > + virDomainMemoryDefPtr mem); > +int qemuTeardownMemoryDevicesCgroup(virDomainObjPtr vm, > + virDomainMemoryDefPtr mem); > int qemuSetupRNGCgroup(virDomainObjPtr vm, > virDomainRNGDefPtr rng); > int qemuTeardownRNGCgroup(virDomainObjPtr vm, > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 7e19d2f82..829b40258 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -2216,6 +2216,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, > const char *backendType; > bool objAdded = false; > bool teardownlabel = false; > + bool teardowncgroup = false; > virJSONValuePtr props = NULL; > virObjectEventPtr event; > int id; > @@ -2245,6 +2246,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, > priv->qemuCaps, vm->def, mem, NULL, true) > < 0) > goto cleanup; > > + if (qemuSetupMemoryDevicesCgroup(vm, mem) < 0) > + goto cleanup; > + teardowncgroup = true; > +
This has the same @props leak as Patch15 > if (qemuSecuritySetMemoryLabel(driver, vm, mem) < 0) > goto cleanup; > teardownlabel = true; > @@ -2294,6 +2299,8 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, > virDomainAuditMemory(vm, oldmem, newmem, "update", ret == 0); > cleanup: > if (mem && ret < 0) { > + if (teardowncgroup && qemuTeardownMemoryDevicesCgroup(vm, mem) < 0) > + VIR_WARN("Unable to remove memory device cgroup ACL on hotplug > fail"); FWIW: based on patch15 comments this would move too > if (teardownlabel && qemuSecurityRestoreMemoryLabel(driver, vm, mem) > < 0) > VIR_WARN("Unable to restore security label on memdev"); > } > @@ -3761,6 +3768,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, > if (qemuSecurityRestoreMemoryLabel(driver, vm, mem) < 0) > VIR_WARN("Unable to restore security label on memdev"); > > + if (qemuTeardownMemoryDevicesCgroup(vm, mem) < 0) > + VIR_WARN("Unable to remove memory device cgroup ACL"); > + The order here is different than attach failure path of teardown cgroup, restore label If there's a relationship between them, then order could be important. ACK in principle and assuming you're following patch15 adjustments anyway. John > virDomainMemoryDefFree(mem); > > /* fix the balloon size */ > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list