[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/7] Add support for sVirt in the LXC driver



On 01/11/2012 09:33 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> For the sake of backwards compat, LXC guests are *not*
> confined by default. This is because it is not practical
> to dynamically relabel containers using large filesystem
> trees. Applications can create confined containers though,
> by giving suitable XML configs
> 
> * src/Makefile.am: Link libvirt_lxc to security drivers
> * src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h,
>   src/lxc/lxc_conf.c, src/lxc/lxc.conf,
>   src/lxc/test_libvirtd_lxc.aug: Config file handling for
>   security driver
> * src/lxc/lxc_driver.c: Wire up security driver functions
> * src/lxc/lxc_controller.c: Add a '--security' flag to
>   specify which security driver to activate
> * src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set
>   the process label just before exec'ing init.
> ---

> +++ b/src/lxc/lxc.conf
> @@ -11,3 +11,21 @@
>  # This is disabled by default, uncomment below to enable it.
>  #
>  # log_with_libvirtd = 1
> +
> +
> +# The default security driver is SELinux. If SELinux is disabled
> +# on the host, then the security driver will automatically disable
> +# itself. If you wish to disable QEMU SELinux security driver while
> +# leaving SELinux enabled for the host in general, then set this
> +# to 'none' instead.
> +#
> +# security_driver = "selinux"
> +
> +# If set to non-zero, then the default security labelling

Same question as 5/7 about whether to prefer US spelling of labeling.

> +# will make guests confined. If set to zero, then guests
> +# will be unconfined by default. Defaults to zero
> +# security_default_confined = 1
> +
> +# If set to non-zero, then attempts to create unconfined
> +# guests will be blocked. Defaults to zero.

Consistency - one description ended with '.', the other did not.  Back
to the 5/7 question of whether this should be spelled out as 'zero' or
listed as '0'.

> +# security_require_confined = 1
> \ No newline at end of file

'make syntax-check' wasn't happy:

prohibit_empty_lines_at_EOF
src/lxc/lxc.conf
maint.mk: empty line(s) or no newline at EOF

> @@ -1598,6 +1625,12 @@ lxcBuildControllerCmd(lxc_driver_t *driver,
>          virCommandAddArgFormat(cmd, "%d", ttyFDs[i]);
>          virCommandPreserveFD(cmd, ttyFDs[i]);
>      }
> +
> +    if (driver->securityDriverName) {
> +        virCommandAddArg(cmd, "--security");
> +        virCommandAddArg(cmd, driver->securityDriverName);
> +    }

Is it worth the shorter:

if (driver->securityDriverName)
    virCommandAddArgPair(cmd, "--security", driver->securityDriverName);

> +
> +static int lxcNodeGetSecurityModel(virConnectPtr conn,
> +                                   virSecurityModelPtr secmodel)
> +{

> +
> +    p = driver->caps->host.secModel.model;
> +    if (strlen(p) >= VIR_SECURITY_MODEL_BUFLEN-1) {
> +        lxcError(VIR_ERR_INTERNAL_ERROR,
> +                 _("security model string exceeds max %d bytes"),
> +                 VIR_SECURITY_MODEL_BUFLEN-1);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +    strcpy(secmodel->model, p);

Rather than doing length checks and then strcpy, wouldn't it be better
to use virStrncpy?  (Twice in this function).

> @@ -3859,6 +4051,8 @@ static virDriver lxcDriver = {
>      .domainGetBlkioParameters = lxcDomainGetBlkioParameters, /* 0.9.8 */
>      .domainGetInfo = lxcDomainGetInfo, /* 0.4.2 */
>      .domainGetState = lxcDomainGetState, /* 0.9.2 */
> +    .domainGetSecurityLabel = lxcDomainGetSecurityLabel, /* 0.9.4 */
> +    .nodeGetSecurityModel = lxcNodeGetSecurityModel, /* 0.9.4 */

You've been sitting on this series for a while, now :)

0.9.10, not 0.9.4.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]