Re: [libvirt] [PATCHv2 1/2] security_dac: Rework to make it more readable

While going through the security DAC code, I realized it's a ragbag with plenty of thing we try to avoid. For instance, callback data are passed as: void params[2]; params[0] = mgr; params[1] = def; Moreover, there's no need to pass the whole virDomainDef in the callback as the only thing needed in the callbacks is virSecurityLabelDefPtr. Okay, in some cases the callbacks report error with a domain name, but that can be changed.

The other thing, in switch() we ought use enum types as it is much safer when adding a new item to the enum. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/security_dac.c | 271 +++++++++++++++++++++++--------------------- 1 file changed, 144 insertions(+), 127 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c

static int -virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, +virSecurityDACGetIds(virSecurityLabelDefPtr seclabel, + virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr, gid_t **groups, int *ngroups) { int ret; - if (!def && !priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to determine default DAC seclabel " - "for an unknown object")); - return -1; - } - if (groups) *groups = priv ? priv->groups : NULL; if (ngroups) *ngroups = priv ? priv->ngroups : 0; - if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) + if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) <= 0) return ret; if (!priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("DAC seclabel couldn't be determined " - "for domain '%s'"), def->name); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("DAC seclabel couldn't be determined")); return -1; } - if (uidPtr) - *uidPtr = priv->user; - if (gidPtr) - *gidPtr = priv->group; + *uidPtr = priv->user; + *gidPtr = priv->group;

return 0; }

@@ -685,41 +656,65 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, static int virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - virDomainChrSourceDefPtr dev) + virDomainChrDefPtr dev, + virDomainChrSourceDefPtr dev_source) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr chr_seclabel = NULL; char *in = NULL, *out = NULL; int ret = -1; uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) - return -1; + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - switch (dev->type) { + if (dev) + chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, + SECURITY_DAC_NAME);

+ + if (chr_seclabel && chr_seclabel->label) { + if (virParseOwnershipIds(chr_seclabel->label, &user, &group) < 0) + return -1; + } else { + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) + return -1; + } +