[libvirt] [PATCH v3 4/5] Support for multiple default security drivers in QEMU config

[Michal Privoznik]

On 03.08.2012 16:18, 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.
> 
> Signed-off-by: Marcelo Cerri <mhcerri at linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_conf.c   |   38 +++++++++++++++++-
>  src/qemu/qemu_conf.h   |    2 +-
>  src/qemu/qemu_driver.c |   99 +++++++++++++++++++++++++++++++++++++-----------
>  3 files changed, 113 insertions(+), 26 deletions(-)
> 

Maybe it's worth mentioning in qemu.conf that multiple drivers per
security_driver variable are supported.

> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index b7db277..ed6d832 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -193,13 +193,45 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
>      }
>  
>      p = virConfGetValue (conf, "security_driver");
> -    CHECK_TYPE ("security_driver", VIR_CONF_STRING);
> -    if (p && p->str) {
> -        if (!(driver->securityDriverName = strdup(p->str))) {
> +    if (p && p->type == VIR_CONF_LIST) {
> +        size_t len;
> +        virConfValuePtr pp;
> +
> +        /* Calc lenght and check items */
> +        for (len = 0, pp = p->list; pp; len++, pp = pp->next) {
> +            if (pp->type != VIR_CONF_STRING) {
> +                VIR_ERROR(_("security_driver be a list of strings"));
> +                virConfFree(conf);
> +                return -1;
> +            }
> +        }
> +
> +        if (VIR_ALLOC_N(driver->securityDriverNames, len + 1) < 0) {
>              virReportOOMError();
>              virConfFree(conf);
>              return -1;
>          }
> +
> +        for (i = 0, pp = p->list; pp; i++, pp = pp->next) {
> +            driver->securityDriverNames[i] = strdup(pp->str);
> +            if (driver->securityDriverNames == NULL) {
> +                virReportOOMError();
> +                virConfFree(conf);
> +                return -1;
> +            }
> +        }
> +        driver->securityDriverNames[len] = NULL;
> +    } else {
> +        CHECK_TYPE ("security_driver", VIR_CONF_STRING);
> +        if (p && p->str) {
> +            if (VIR_ALLOC_N(driver->securityDriverNames, 2) < 0 ||
> +                !(driver->securityDriverNames[0] = strdup(p->str))) {
> +                virReportOOMError();
> +                virConfFree(conf);
> +                return -1;
> +            }
> +            driver->securityDriverNames[1] = NULL;
> +        }
>      }
>  
>      p = virConfGetValue (conf, "security_default_confined");
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 92e4968..8a51471 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -116,7 +116,7 @@ struct qemud_driver {
>  
>      virDomainEventStatePtr domainEventState;
>  
> -    char *securityDriverName;
> +    char **securityDriverNames;
>      bool securityDefaultConfined;
>      bool securityRequireConfined;
>      virSecurityManagerPtr securityManager;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e3f71c3..ec0f02b 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;
>  
> +    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 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))
> +                goto error;
> +            names++;
> +        }
> +    }
> +
>      if (driver->privileged) {
> -        virSecurityManagerPtr dac = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> -                                                             driver->user,
> -                                                             driver->group,
> -                                                             driver->allowDiskFormatProbing,
> -                                                             driver->securityDefaultConfined,
> -                                                             driver->securityRequireConfined,
> -                                                             driver->dynamicOwnership);
> -        if (!dac)
> -            goto error;
> -
> -        if (!(driver->securityManager = virSecurityManagerNewStack(mgr)) ||
> -            !(virSecurityManagerStackAddNested(mgr, dac))) {
> -
> -            virSecurityManagerFree(dac);
> -            goto error;
> +        /* When a DAC driver is required, check if there is already one in the
> +         * additional drivers */
> +        names = driver->securityDriverNames;
> +        while (names && *names) {
> +            if (STREQ("dac", *names)) {
> +               break;
> +            }
> +            names++;
> +        }
> +        /* If there is no DAC driver, create a new one and add it to the stack
> +         * manager */
> +        if (names == NULL || *names == NULL) {
> +            nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> +                                              driver->user,
> +                                              driver->group,
> +                                              driver->allowDiskFormatProbing,
> +                                              driver->securityDefaultConfined,
> +                                              driver->securityRequireConfined,
> +                                              driver->dynamicOwnership);
> +            if (nested == NULL)
> +                goto error;
> +            if (virSecurityManagerStackAddNested(stack, nested))
> +                goto error;
>          }
> -    } else {
> -        driver->securityManager = mgr;
>      }
>  
> +    driver->securityManager = mgr;
>      return 0;
>  
>  error:
> 

ACK with qemu.conf updated.

Michal