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