On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch replaces the key "security_driver" in QEMU
config by
"security_drivers", which accepts a list of default drivers. If
"security_drivers" can't be found, libvirt will use
"security_driver" to
ensure that it will remain compatible with older version of the config
file.
---
src/qemu/libvirtd_qemu.aug | 2 +-
src/qemu/qemu.conf | 2 +-
src/qemu/qemu_conf.c | 42 ++++++++++++++-
src/qemu/qemu_conf.h | 2 +-
src/qemu/qemu_driver.c | 97 ++++++++++++++++++++++++++++--------
src/qemu/test_libvirtd_qemu.aug.in | 2 +-
6 files changed, 119 insertions(+), 28 deletions(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 683aadb..fab97d7 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -39,7 +39,7 @@ module Libvirtd_qemu =
| str_entry "spice_tls_x509_cert_dir"
| str_entry "spice_password"
- let security_entry = str_entry "security_driver"
+ let security_entry = str_entry "security_drivers"
| bool_entry "security_default_confined"
| bool_entry "security_require_confined"
| str_entry "user"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index ed4683c..ffb03f8 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -146,7 +146,7 @@
# leaving SELinux enabled for the host in general, then set this
# to 'none' instead.
#
-#security_driver = "selinux"
+#security_drivers = "selinux"
No, we can't do that. Changing this would silently discard
any values set by users. Moreover, separating values by comma -
- that's yet another way for multiple values passing.
Can't we just re-use:
cgroup_controllers = [ "cpu", "devices", "memory",
"blkio", "cpuset", "cpuacct" ]
that is making these two syntactically and semantically correct:
security_driver = "selinux"
security_driver = [ "selinux", "apparmor",
"YetAnotherSecurtyDriver" ]
Having said that - the rest of the patch is pointless to review and need rework after we
settle down on design.
But I'll point out one obvious error anyway.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b02f28..f01566b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -220,36 +220,91 @@ qemuAutostartDomains(struct qemud_driver *driver)
static int
qemuSecurityInit(struct qemud_driver *driver)
{
- virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName,
- QEMU_DRIVER_NAME,
-
driver->allowDiskFormatProbing,
-
driver->securityDefaultConfined,
-
driver->securityRequireConfined);
+ char **names;
+ char *primary;
+ virSecurityManagerPtr mgr, nested, stack;
Stack may be use uninitialized ...
+ if (driver->securityDriverNames == NULL)
+ primary = NULL;
+ else
+ primary = driver->securityDriverNames[0];
+
+ /* Create primary driver */
+ mgr = virSecurityManagerNew(primary,
+ QEMU_DRIVER_NAME,
+ driver->allowDiskFormatProbing,
+ driver->securityDefaultConfined,
+ driver->securityRequireConfined);
if (!mgr)
goto error;
- if (driver->privileged) {
- virSecurityManagerPtr dac = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
- driver->user,
- driver->group,
-
driver->allowDiskFormatProbing,
-
driver->securityDefaultConfined,
-
driver->securityRequireConfined,
-
driver->dynamicOwnership);
- if (!dac)
+ /* If a DAC driver is required or additional drivers are provived, a stack
+ * driver should be create to group them all */
+ if (driver->privileged ||
+ (driver->securityDriverNames && driver->securityDriverNames[1]))
{
+ stack = virSecurityManagerNewStack(mgr);
+ if (!stack)
goto error;
+ mgr = stack;
+ }
+
+ /* Loop through additional driver names and add a secudary driver to each
+ * one */
+ if (driver->securityDriverNames) {
+ names = driver->securityDriverNames + 1;
+ while (names && *names) {
+ if (STREQ("dac", *names)) {
+ /* A DAC driver has specific parameters */
+ nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
+ driver->user,
+ driver->group,
+ driver->allowDiskFormatProbing,
+ driver->securityDefaultConfined,
+ driver->securityRequireConfined,
+ driver->dynamicOwnership);
+ } else {
+ nested = virSecurityManagerNew(*names,
+ QEMU_DRIVER_NAME,
+ driver->allowDiskFormatProbing,
+ driver->securityDefaultConfined,
+ driver->securityRequireConfined);
+ }
+ if (nested == NULL)
+ goto error;
+ if (virSecurityManagerStackAddNested(stack, nested))
... here.
+ goto error;
+ names++;
+ }
+ }
Michal