On Wed, 2014-06-18 at 11:11 +0200, Christophe Fergeau wrote:
Hi,
On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote:
> This code depends on new API in libvirt-gconfig to extract the
> secmodels handled by the host.
> ---
> Diff to v3:
> * Added yet another missing g_object_unref.
> * Fixed the logic for supportsSelinux
> libvirt-sandbox/libvirt-sandbox-builder.c | 49 +++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c
b/libvirt-sandbox/libvirt-sandbox-builder.c
> index 48b3acc..d6b5735 100644
> --- a/libvirt-sandbox/libvirt-sandbox-builder.c
> +++ b/libvirt-sandbox/libvirt-sandbox-builder.c
> @@ -322,12 +322,10 @@ static gboolean
gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build
> return TRUE;
> }
>
> -
> -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder
G_GNUC_UNUSED,
> - GVirSandboxConfig *config
G_GNUC_UNUSED,
> - const gchar *statedir
G_GNUC_UNUSED,
> - GVirConfigDomain *domain,
> - GError **error
G_GNUC_UNUSED)
> +static gboolean gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuilder
*builder,
> + GVirSandboxConfig
*config,
> + GVirConfigDomain
*domain,
> + GError **error)
> {
> GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
> const char *label = gvir_sandbox_config_get_security_label(config);
> @@ -360,6 +358,45 @@ static gboolean
gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
> return TRUE;
> }
>
> +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder
*builder,
> + GVirSandboxConfig *config,
> + const gchar *statedir
G_GNUC_UNUSED,
> + GVirConfigDomain *domain,
> + GError **error)
> +{
> + GVirConnection *connection = gvir_sandbox_builder_get_connection(builder);
> + GVirConfigCapabilities *configCapabilities;
> + GVirConfigCapabilitiesHost *hostCapabilities;
> + GList *secmodels, *iter;
> + gboolean supportsSelinux = FALSE;
> +
> + /* What security models are available on the host? */
> + if (!(configCapabilities = gvir_connection_get_capabilities(connection,
error))) {
Missing g_object_unref(connection); here too.
Oops, I forgot that one case indeed.
> + return FALSE;
> + }
> +
> + hostCapabilities = gvir_config_capabilities_get_host(configCapabilities);
> +
> + secmodels = gvir_config_capabilities_host_get_secmodels(hostCapabilities);
> + for (iter = secmodels; iter != NULL; iter = iter->next) {
> + if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
> + GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter->data)),
"selinux"))
> + supportsSelinux = TRUE;
> + g_object_unref(iter->data);
> + }
> +
> + g_list_free(secmodels);
> + g_object_unref(hostCapabilities);
> + g_object_unref(configCapabilities);
> + g_object_unref(connection);
> +
> + if (supportsSelinux)
> + return gvir_sandbox_builder_construct_security_selinux(builder, config,
> + domain, error);
> +
> + return TRUE;
Wondering whether this we should return FALSE when we did nothing
because we only support SELinux.
No idea what the original intent was... but we shouldn't fail if we just
not using any security label: that may be a valid use case.
Patch is fine otherwise, I can squash these changes in before pushing
if
you don't want to send yet another iteration ;)
Well, that would save a few mails to the mailing list ;)
--
Cedric