[libvirt] [PATCH 05/12] Introduce a stacked security driver impl for QEMU

[Daniel Veillard]

On Wed, Jan 20, 2010 at 03:15:02PM +0000, Daniel P. Berrange wrote:
> * qemu/qemu_conf.h: Add securityPrimaryDriver and
>   securitySecondaryDriver fields to 'struct qemud_driver'
> * Makefile.am: Add new files
> * qemu/qemu_security_stacked.c, qemu/qemu_security_stacked.h: A
>   simple stacked security driver
> ---
>  src/Makefile.am                  |    4 +-
>  src/qemu/qemu_conf.h             |    2 +
>  src/qemu/qemu_security_stacked.c |  353 ++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_security_stacked.h |   22 +++
>  4 files changed, 380 insertions(+), 1 deletions(-)
>  create mode 100644 src/qemu/qemu_security_stacked.c
>  create mode 100644 src/qemu/qemu_security_stacked.h
[...]
> +++ b/src/qemu/qemu_security_stacked.c
> @@ -0,0 +1,353 @@
> +/*
> + * Copyright (C) 2010 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * QEMU stacked security driver
> + */
> +
> +#include <config.h>
> +#include <selinux/selinux.h>
> +#include <selinux/context.h>

  Why do we need to include SELinux headers, the module content seems to
  be agnostic on the underlying framework (SELinux/Apparmor)

> +static int
> +qemuSecurityStackedGenLabel(virConnectPtr conn,
> +                            virDomainObjPtr vm)
> +{
> +    int rc = 0;
> +
> +    if (driver->securitySecondaryDriver &&
> +        driver->securitySecondaryDriver->domainGenSecurityLabel &&
> +        driver->securitySecondaryDriver->domainGenSecurityLabel(conn, vm) < 0)
> +        rc = -1;
> +
> +    if (driver->securityPrimaryDriver &&
> +        driver->securityPrimaryDriver->domainGenSecurityLabel &&
> +        driver->securityPrimaryDriver->domainGenSecurityLabel(conn, vm) < 0)
> +        rc = -1;
> +
> +    return rc;
> +}

  Okay, so the general pattern is that any stacked driver call will fail
if any of the subdriver entry point fail, a missing entry point not
being a failure. I'm still surprized that if both entry point are
missing we return 0 i.e. success...

> +virSecurityDriver qemuStackedSecurityDriver = {
> +    .name                       = "qemuStacked",
> +    .domainSecurityVerify = qemuSecurityStackedVerify,
> +
> +    .domainGenSecurityLabel = qemuSecurityStackedGenLabel,
> +    .domainReleaseSecurityLabel = qemuSecurityStackedReleaseLabel,
> +    .domainReserveSecurityLabel = qemuSecurityStackedReserveLabel,
> +
> +    .domainGetSecurityProcessLabel = qemuSecurityStackedGetProcessLabel,
> +    .domainSetSecurityProcessLabel = qemuSecurityStackedSetProcessLabel,
> +
> +    .domainSetSecurityImageLabel = qemuSecurityStackedSetSecurityImageLabel,
> +    .domainRestoreSecurityImageLabel = qemuSecurityStackedRestoreSecurityImageLabel,
> +
> +    .domainSetSecurityAllLabel     = qemuSecurityStackedSetSecurityAllLabel,
> +    .domainRestoreSecurityAllLabel = qemuSecurityStackedRestoreSecurityAllLabel,
> +
> +    .domainSetSecurityHostdevLabel = qemuSecurityStackedSetSecurityHostdevLabel,
> +    .domainRestoreSecurityHostdevLabel = qemuSecurityStackedRestoreSecurityHostdevLabel,
> +
> +    .domainSetSavedStateLabel = qemuSecurityStackedSetSavedStateLabel,
> +    .domainRestoreSavedStateLabel = qemuSecurityStackedRestoreSavedStateLabel,
> +};

  I tend to prefer full initialization to see if there is missing entry points.

  Just minor remarks except maybe for the return value in case both
subdrivers misses the entry point.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/