5:30 a.m.
We are already doing that for some devices (e.g. disks,
hostdevs), but completely forgot about some others (chardevs and
RNGs). So far the hotplug worked by pure chance as /dev/random
and /dev/urandom are enabled in the device CGroups by default.
Michal Privoznik (2):
qemu: Update cgroup on RNG hotplug
qemu: Update cgroup on chardev hotplug
src/qemu/qemu_cgroup.c | 108 +++++++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_cgroup.h | 8 ++++
src/qemu/qemu_hotplug.c | 37 ++++++++++++++---
3 files changed, 131 insertions(+), 22 deletions(-)
--
2.8.4
5:30 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Update cgroup on RNG hotplug
If users try to hotplug RNG device with a backend different to
/dev/random or /dev/urandom the whole operation fails as qemu is
unable to access the device. The problem is we don't update
device CGroups during the operation.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 62 +++++++++++++++++++++++++++++++++++++++----------
src/qemu/qemu_cgroup.h | 4 ++++
src/qemu/qemu_hotplug.c | 17 ++++++++++++--
3 files changed, 69 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1443f7e..e7ce032 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -569,6 +569,54 @@ qemuSetupFirmwareCgroup(virDomainObjPtr vm)
}
+int
+qemuSetupRNGCgroup(virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rv;
+
+ if (rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) {
+ VIR_DEBUG("Setting Cgroup ACL for RNG device");
+ rv = virCgroupAllowDevicePath(priv->cgroup,
+ rng->source.file,
+ VIR_CGROUP_DEVICE_RW, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "allow",
+ rng->source.file,
+ "rw", rv == 0);
+ if (rv < 0 &&
+ !virLastErrorIsSystemErrno(ENOENT))
+ return -1;
+ }
+
+ return 0;
+}
+
+
+int
+qemuTeardownRNGCgroup(virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rv;
+
+ if (rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) {
+ VIR_DEBUG("Setting Cgroup ACL for RNG device");
+ rv = virCgroupDenyDevicePath(priv->cgroup,
+ rng->source.file,
+ VIR_CGROUP_DEVICE_RW, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "deny",
+ rng->source.file,
+ "rw", rv == 0);
+ if (rv < 0 &&
+ !virLastErrorIsSystemErrno(ENOENT))
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm)
@@ -663,18 +711,8 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
}
for (i = 0; i < vm->def->nrngs; i++) {
- if (vm->def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) {
- VIR_DEBUG("Setting Cgroup ACL for RNG device");
- rv = virCgroupAllowDevicePath(priv->cgroup,
- vm->def->rngs[i]->source.file,
- VIR_CGROUP_DEVICE_RW, false);
- virDomainAuditCgroupPath(vm, priv->cgroup, "allow",
- vm->def->rngs[i]->source.file,
- "rw", rv == 0);
- if (rv < 0 &&
- !virLastErrorIsSystemErrno(ENOENT))
- goto cleanup;
- }
+ if (qemuSetupRNGCgroup(vm, vm->def->rngs[i]) < 0)
+ goto cleanup;
}
ret = 0;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 623823e..1c3b7ff 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 qemuSetupRNGCgroup(virDomainObjPtr vm,
+ virDomainRNGDefPtr rng);
+int qemuTeardownRNGCgroup(virDomainObjPtr vm,
+ virDomainRNGDefPtr rng);
int qemuConnectCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm);
int qemuSetupCgroup(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6b3a068..b43d9dd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1951,6 +1951,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
char *tlsAlias = NULL;
char *secAlias = NULL;
bool releaseaddr = false;
+ bool teardowncgroup = false;
bool chardevAdded = false;
bool objAdded = false;
bool tlsobjAdded = false;
@@ -1996,6 +1997,10 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
goto cleanup;
}
+ if (qemuSetupRNGCgroup(vm, rng) < 0)
+ goto cleanup;
+ teardowncgroup = true;
+
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
qemuDomainPrepareChardevSourceTLS(rng->source.chardev, cfg);
@@ -2073,8 +2078,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
virJSONValueFree(tlsProps);
virJSONValueFree(secProps);
virJSONValueFree(props);
- if (ret < 0 && releaseaddr)
- qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL);
+ if (ret < 0) {
+ if (releaseaddr)
+ qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL);
+ if (teardowncgroup && qemuTeardownRNGCgroup(vm, rng) < 0)
+ VIR_WARN("Unable to remove RNG device cgroup ACL on hotplug
fail");
+ }
+
VIR_FREE(tlsAlias);
VIR_FREE(secAlias);
VIR_FREE(charAlias);
@@ -3912,6 +3922,9 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
if (rc < 0)
goto cleanup;
+ if (qemuTeardownRNGCgroup(vm, rng) < 0)
+ VIR_WARN("Failed to remove RNG device cgroup ACL");
+
event = virDomainEventDeviceRemovedNewFromObj(vm, rng->info.alias);
qemuDomainEventQueue(driver, event);
--
2.8.4
8:20 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Update cgroup on RNG hotplug
[...]
+int
+qemuTeardownRNGCgroup(virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rv;
+
+ if (rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) {
+ VIR_DEBUG("Setting Cgroup ACL for RNG device");
s/Setting/Tearing down/
(or something similar - Unsetting, Removing)
Looks reasonable otherwise,
ACK
John
+ rv = virCgroupDenyDevicePath(priv->cgroup,
+ rng->source.file,
+ VIR_CGROUP_DEVICE_RW, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "deny",
+ rng->source.file,
+ "rw", rv == 0);
+ if (rv < 0 &&
+ !virLastErrorIsSystemErrno(ENOENT))
+ return -1;
+ }
+
+ return 0;
+}
+
+
[...]
9:47 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Update cgroup on RNG hotplug
On 23.11.2016 15:20, John Ferlan wrote:
[...]
> +int
> +qemuTeardownRNGCgroup(virDomainObjPtr vm,
> + virDomainRNGDefPtr rng)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + int rv;
> +
> + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) {
> + VIR_DEBUG("Setting Cgroup ACL for RNG device");
s/Setting/Tearing down/
(or something similar - Unsetting, Removing)
Looks reasonable otherwise,
Ah, right.
ACK
Thank you. I've just pushed both.
Michal
5:30 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Update cgroup on chardev hotplug
Just like in the previous commit, we are not updating CGroups on
chardev hot(un-)plug and thus leaving qemu unable to access any
non-default device users are trying to hotplug.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_cgroup.h | 4 ++++
src/qemu/qemu_hotplug.c | 20 ++++++++++++++++----
3 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e7ce032..50e3d35 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -189,10 +189,32 @@ qemuSetupChrSourceCgroup(virDomainObjPtr vm,
return ret;
}
+
static int
-qemuSetupChardevCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED,
- virDomainChrDefPtr dev,
- void *opaque)
+qemuTeardownChrSourceCgroup(virDomainObjPtr vm,
+ virDomainChrSourceDefPtr source)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret;
+
+ if (source->type != VIR_DOMAIN_CHR_TYPE_DEV)
+ return 0;
+
+ VIR_DEBUG("Process path '%s' for device",
source->data.file.path);
+
+ ret = virCgroupDenyDevicePath(priv->cgroup, source->data.file.path,
+ VIR_CGROUP_DEVICE_RW, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "deny",
+ source->data.file.path, "rw", ret == 0);
+
+ return ret;
+}
+
+
+static int
+qemuSetupChardevCgroupCB(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainChrDefPtr dev,
+ void *opaque)
{
virDomainObjPtr vm = opaque;
@@ -617,6 +639,22 @@ qemuTeardownRNGCgroup(virDomainObjPtr vm,
}
+int
+qemuSetupChardevCgroup(virDomainObjPtr vm,
+ virDomainChrDefPtr dev)
+{
+ return qemuSetupChrSourceCgroup(vm, dev->source);
+}
+
+
+int
+qemuTeardownChardevCgroup(virDomainObjPtr vm,
+ virDomainChrDefPtr dev)
+{
+ return qemuTeardownChrSourceCgroup(vm, dev->source);
+}
+
+
static int
qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm)
@@ -693,7 +731,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
if (virDomainChrDefForeach(vm->def,
true,
- qemuSetupChardevCgroup,
+ qemuSetupChardevCgroupCB,
vm) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 1c3b7ff..6e2c742 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -47,6 +47,10 @@ int qemuSetupRNGCgroup(virDomainObjPtr vm,
virDomainRNGDefPtr rng);
int qemuTeardownRNGCgroup(virDomainObjPtr vm,
virDomainRNGDefPtr rng);
+int qemuSetupChardevCgroup(virDomainObjPtr vm,
+ virDomainChrDefPtr dev);
+int qemuTeardownChardevCgroup(virDomainObjPtr vm,
+ virDomainChrDefPtr dev);
int qemuConnectCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm);
int qemuSetupCgroup(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b43d9dd..5038ce1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1830,6 +1830,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
char *charAlias = NULL;
bool chardevAttached = false;
bool tlsobjAdded = false;
+ bool teardowncgroup = false;
bool secobjAdded = false;
virJSONValuePtr tlsProps = NULL;
char *tlsAlias = NULL;
@@ -1851,6 +1852,10 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
if (rc == 1)
need_release = true;
+ if (qemuSetupChardevCgroup(vm, chr) < 0)
+ goto cleanup;
+ teardowncgroup = true;
+
if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
goto cleanup;
@@ -1903,10 +1908,14 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
audit:
virDomainAuditChardev(vm, NULL, chr, "attach", ret == 0);
cleanup:
- if (ret < 0 && virDomainObjIsActive(vm))
- qemuDomainChrInsertPreAllocCleanup(vmdef, chr);
- if (ret < 0 && need_release)
- qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
+ if (ret < 0) {
+ if (virDomainObjIsActive(vm))
+ qemuDomainChrInsertPreAllocCleanup(vmdef, chr);
+ if (need_release)
+ qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
+ if (teardowncgroup && qemuTeardownChardevCgroup(vm, chr) < 0)
+ VIR_WARN("Unable to remove chr device cgroup ACL on hotplug
fail");
+ }
VIR_FREE(tlsAlias);
virJSONValueFree(tlsProps);
VIR_FREE(secAlias);
@@ -3847,6 +3856,9 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
if (rc < 0)
goto cleanup;
+ if (qemuTeardownChardevCgroup(vm, chr) < 0)
+ VIR_WARN("Failed to remove chr device cgroup ACL");
+
event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias);
qemuDomainEventQueue(driver, event);
--
2.8.4
8:20 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Update cgroup on chardev hotplug
On 11/18/2016 06:30 AM, Michal Privoznik wrote:
Just like in the previous commit, we are not updating CGroups on
chardev hot(un-)plug and thus leaving qemu unable to access any
non-default device users are trying to hotplug.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_cgroup.h | 4 ++++
src/qemu/qemu_hotplug.c | 20 ++++++++++++++++----
3 files changed, 62 insertions(+), 8 deletions(-)
ACK,
John
6:57 a.m.
On 18.11.2016 12:30, Michal Privoznik wrote:
We are already doing that for some devices (e.g. disks,
hostdevs), but completely forgot about some others (chardevs and
RNGs). So far the hotplug worked by pure chance as /dev/random
and /dev/urandom are enabled in the device CGroups by default.
Michal Privoznik (2):
qemu: Update cgroup on RNG hotplug
qemu: Update cgroup on chardev hotplug
src/qemu/qemu_cgroup.c | 108 +++++++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_cgroup.h | 8 ++++
src/qemu/qemu_hotplug.c | 37 ++++++++++++++---
3 files changed, 131 insertions(+), 22 deletions(-)
ping? It would be nice to have these in before the freeze.
Michal
2921
days inactive
2926
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik