On 1/15/19 8:23 AM, Ján Tomko wrote:
> Split out parts of the config parsing code to make
> the parent function easier to read.
>
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
> src/qemu/qemu_conf.c | 219 +++++++++++++++++++++++--------------------
> 1 file changed, 117 insertions(+), 102 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 7fdfed7db1..135cb9e25d 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -423,6 +423,121 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
> }
>
>
> +static int
> +virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
> + virConfPtr conf,
> + bool privileged)
This does security, cgroups, and namespaces...
> +{
> + char *user = NULL, *group = NULL;
VIR_AUTOPTR(char *)
> + char **controllers = NULL;
> + char **namespaces = NULL;
VIR_AUTOPTR(virString)
> + int ret = -1;
> + size_t i, j;
> +
> + if (virConfGetValueStringList(conf, "security_driver", true, &cfg->securityDriverNames) < 0)
> + goto cleanup;
> +
> + for (i = 0; cfg->securityDriverNames && cfg->securityDriverNames[i] != NULL; i++) {
> + for (j = i + 1; cfg->securityDriverNames[j] != NULL; j++) {
> + if (STREQ(cfg->securityDriverNames[i],
> + cfg->securityDriverNames[j])) {
> + virReportError(VIR_ERR_CONF_SYNTAX,
> + _("Duplicate security driver %s"),
> + cfg->securityDriverNames[i]);
> + goto cleanup;
> + }
> + }
> + }
> +
> + if (virConfGetValueBool(conf, "security_default_confined", &cfg->securityDefaultConfined) < 0)
> + goto cleanup;
> + if (virConfGetValueBool(conf, "security_require_confined", &cfg->securityRequireConfined) < 0)
> + goto cleanup;
nit: blank line for readability/separation
> + if (virConfGetValueString(conf, "user", &user) < 0)
> + goto cleanup;
> + if (user && virGetUserID(user, &cfg->user) < 0)
> + goto cleanup;
> +
> + if (virConfGetValueString(conf, "group", &group) < 0)
> + goto cleanup;
> + if (group && virGetGroupID(group, &cfg->group) < 0)
> + goto cleanup;
> +
> + if (virConfGetValueBool(conf, "dynamic_ownership", &cfg->dynamicOwnership) < 0)
> + goto cleanup;
> +
The following hunk feels separable since it's not security related...
> + if (virConfGetValueStringList(conf, "cgroup_controllers", false,
^^
Oh look an extra space (existing)... may as well fix it, but a separate
patch is not needed.
> + &controllers) < 0)
> + goto cleanup;
> +
> + if (controllers) {
> + cfg-> cgroupControllers = 0;
> + for (i = 0; controllers[i] != NULL; i++) {
> + int ctl;
> + if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) {
> + virReportError(VIR_ERR_CONF_SYNTAX,
> + _("Unknown cgroup controller '%s'"),
> + controllers[i]);
> + goto cleanup;
> + }
> + cfg->cgroupControllers |= (1 << ctl);
> + }
> + }
> +
> + if (virConfGetValueStringList(conf, "cgroup_device_acl", false,
^^
and again copy-paste probably
> + &cfg->cgroupDeviceACL) < 0)
> + goto cleanup;
end cgroup separable hunk
> +> + if (virConfGetValueInt(conf, "seccomp_sandbox",
&cfg->seccompSandbox) < 0)
> + goto cleanup;
> +
And again, not security related.
Still, for the concept of splitting it's fine. I trust you can
manipulate a bit more for a final result, so
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
