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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list