On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
>
> Notes:
> new in v2
>
> src/conf/domain_conf.c | 46 +++++++++++++++++++----------------------
> src/conf/domain_conf.h | 9 ++++----
> src/security/security_dac.c | 26 ++++++++++-------------
> src/security/security_manager.c | 4 ++--
> src/security/security_selinux.c | 24 +++++++++------------
> 5 files changed, 49 insertions(+), 60 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c7e20b8ba1..68dc2832cb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>
> void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
> {
> + size_t i;
> +
> if (!def)
> return;
>
> virDomainChrSourceDefClear(def);
> virObjectUnref(def->privateData);
>
> + if (def->seclabels) {
> + for (i = 0; i < def->nseclabels; i++)
> + virSecurityDeviceLabelDefFree(def->seclabels[i]);
> + VIR_FREE(def->seclabels);
> + }
> +
> +
> VIR_FREE(def);
> }
>
> @@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
>
> void virDomainChrDefFree(virDomainChrDefPtr def)
> {
> - size_t i;
> -
> if (!def)
> return;
>
> @@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
> virDomainChrSourceDefFree(def->source);
> virDomainDeviceInfoClear(&def->info);
>
> - if (def->seclabels) {
> - for (i = 0; i < def->nseclabels; i++)
> - virSecurityDeviceLabelDefFree(def->seclabels[i]);
> - VIR_FREE(def->seclabels);
> - }
> -
> VIR_FREE(def);
> }
>
> @@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> if (chr_def) {
Is the @chr_def check necessary still? I assume it's there because
chr_def can be passed as NULL in some cases.
Looks like all this was added as part of commit 'f8b08d0e'
The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which
each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that
leads me to believe that the @chr_def should be removed.
The rest looks good, so
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
> xmlNodePtr saved_node = ctxt->node;
> ctxt->node = cur;
> - if (virSecurityDeviceLabelDefParseXML(&chr_def->seclabels,
> - &chr_def->nseclabels,
> + if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
> + &def->nseclabels,
> vmSeclabels,
> nvmSeclabels,
> ctxt,
> @@ -22399,19 +22400,11 @@ virDomainNetDefFormat(virBufferPtr buf,
> * output at " type='type'>". */
> static int
> virDomainChrSourceDefFormat(virBufferPtr buf,
> - virDomainChrDefPtr chr_def,
> virDomainChrSourceDefPtr def,
> bool tty_compat,
> unsigned int flags)
> {
> const char *type = virDomainChrTypeToString(def->type);
> - size_t nseclabels = 0;
> - virSecurityDeviceLabelDefPtr *seclabels = NULL;
> -
> - if (chr_def) {
> - nseclabels = chr_def->nseclabels;
> - seclabels = chr_def->seclabels;
> - }
>
> if (!type) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -22449,7 +22442,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT)
> virBufferAsprintf(buf, " append='%s'",
> virTristateSwitchTypeToString(def->data.file.append));
> - virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
> + virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
> + def->seclabels, flags);
> }
> break;
>
> @@ -22504,7 +22498,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<source mode='%s'",
> def->data.nix.listen ? "bind" : "connect");
> virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
> - virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
> + virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
> + def->seclabels, flags);
> }
> break;
>
> @@ -22553,7 +22548,7 @@ virDomainChrDefFormat(virBufferPtr buf,
> def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
> !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
> def->source->data.file.path);
> - if (virDomainChrSourceDefFormat(buf, def, def->source, tty_compat, flags) < 0)
> + if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
> return -1;
>
> /* Format <target> block */
> @@ -22675,7 +22670,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> - if (virDomainChrSourceDefFormat(buf, NULL, def->data.passthru, false,
> + if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
> flags) < 0)
> return -1;
> break;
> @@ -22981,7 +22976,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
>
> case VIR_DOMAIN_RNG_BACKEND_EGD:
> virBufferAdjustIndent(buf, 2);
> - if (virDomainChrSourceDefFormat(buf, NULL, def->source.chardev,
> + if (virDomainChrSourceDefFormat(buf, def->source.chardev,
> false, flags) < 0)
> return -1;
> virBufferAdjustIndent(buf, -2);
> @@ -23797,7 +23792,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
>
> virBufferAsprintf(buf, "<redirdev bus='%s'", bus);
> virBufferAdjustIndent(buf, 2);
> - if (virDomainChrSourceDefFormat(buf, NULL, def->source, false, flags) < 0)
> + if (virDomainChrSourceDefFormat(buf, def->source, false, flags) < 0)
> return -1;
> if (virDomainDeviceInfoFormat(buf, &def->info,
> flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0)
> @@ -26195,7 +26190,8 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
>
>
> virSecurityDeviceLabelDefPtr
> -virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
> +virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
> + const char *model)
> {
> size_t i;
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 83e0672691..1951ba74bb 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1166,6 +1166,9 @@ struct _virDomainChrSourceDef {
> } data;
> char *logfile;
> int logappend;
> +
> + size_t nseclabels;
> + virSecurityDeviceLabelDefPtr *seclabels;
> };
>
> /* A complete character device, both host and domain views. */
> @@ -1188,9 +1191,6 @@ struct _virDomainChrDef {
> virDomainChrSourceDefPtr source;
>
> virDomainDeviceInfo info;
> -
> - size_t nseclabels;
> - virSecurityDeviceLabelDefPtr *seclabels;
> };
>
> typedef enum {
> @@ -3068,7 +3068,8 @@ virSecurityLabelDefPtr
> virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
>
> virSecurityDeviceLabelDefPtr
> -virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
> +virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
> + const char *model);
>
> typedef const char* (*virEventActionToStringFunc)(int type);
> typedef int (*virEventActionFromStringFunc)(const char *type);
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 7dcf4c15f7..fd4d8f5047 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1159,7 +1159,6 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
> static int
> virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> virDomainChrSourceDefPtr dev_source)
>
> {
> @@ -1173,9 +1172,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
>
> seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
>
> - if (dev)
> - chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> - SECURITY_DAC_NAME);
> + chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> + SECURITY_DAC_NAME);
>
> if (chr_seclabel && !chr_seclabel->relabel)
> return 0;
> @@ -1245,7 +1243,6 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
> static int
> virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def ATTRIBUTE_UNUSED,
> - virDomainChrDefPtr dev,
> virDomainChrSourceDefPtr dev_source)
> {
> virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> @@ -1253,9 +1250,8 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
> char *in = NULL, *out = NULL;
> int ret = -1;
>
> - if (dev)
> - chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> - SECURITY_DAC_NAME);
> + chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> + SECURITY_DAC_NAME);
>
> if (chr_seclabel && !chr_seclabel->relabel)
> return 0;
> @@ -1304,12 +1300,12 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
>
> static int
> virSecurityDACRestoreChardevCallback(virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> + virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
> void *opaque)
> {
> virSecurityManagerPtr mgr = opaque;
>
> - return virSecurityDACRestoreChardevLabel(mgr, def, dev, dev->source);
> + return virSecurityDACRestoreChardevLabel(mgr, def, dev->source);
> }
>
>
> @@ -1322,7 +1318,7 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
>
> switch (tpm->type) {
> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> - ret = virSecurityDACSetChardevLabel(mgr, def, NULL,
> + ret = virSecurityDACSetChardevLabel(mgr, def,
> &tpm->data.passthrough.source);
> break;
> case VIR_DOMAIN_TPM_TYPE_LAST:
> @@ -1342,8 +1338,8 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
>
> switch (tpm->type) {
> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> - ret = virSecurityDACRestoreChardevLabel(mgr, def, NULL,
> - &tpm->data.passthrough.source);
> + ret = virSecurityDACRestoreChardevLabel(mgr, def,
> + &tpm->data.passthrough.source);
> break;
> case VIR_DOMAIN_TPM_TYPE_LAST:
> break;
> @@ -1506,12 +1502,12 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
>
> static int
> virSecurityDACSetChardevCallback(virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> + virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
> void *opaque)
> {
> virSecurityManagerPtr mgr = opaque;
>
> - return virSecurityDACSetChardevLabel(mgr, def, dev, dev->source);
> + return virSecurityDACSetChardevLabel(mgr, def, dev->source);
> }
>
>
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 6c777db1e6..90d491c1bc 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -811,8 +811,8 @@ virSecurityManagerCheckChardevLabel(virSecurityManagerPtr mgr,
> {
> size_t i;
>
> - for (i = 0; i < dev->nseclabels; i++) {
> - if (virSecurityManagerCheckModel(mgr, dev->seclabels[i]->model) < 0)
> + for (i = 0; i < dev->source->nseclabels; i++) {
> + if (virSecurityManagerCheckModel(mgr, dev->source->seclabels[i]->model) < 0)
> return -1;
> }
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 9504a4be34..75f387b3fa 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2179,7 +2179,6 @@ virSecuritySELinuxRestoreHostdevLabel(virSecurityManagerPtr mgr,
> static int
> virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> virDomainChrSourceDefPtr dev_source)
>
> {
> @@ -2193,9 +2192,8 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
> if (!seclabel || !seclabel->relabel)
> return 0;
>
> - if (dev)
> - chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> - SECURITY_SELINUX_NAME);
> + chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> + SECURITY_SELINUX_NAME);
>
> if (chr_seclabel && !chr_seclabel->relabel)
> return 0;
> @@ -2254,7 +2252,6 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
> static int
> virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> virDomainChrSourceDefPtr dev_source)
>
> {
> @@ -2267,9 +2264,8 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
> if (!seclabel || !seclabel->relabel)
> return 0;
>
> - if (dev)
> - chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> - SECURITY_SELINUX_NAME);
> + chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> + SECURITY_SELINUX_NAME);
> if (chr_seclabel && !chr_seclabel->relabel)
> return 0;
>
> @@ -2318,12 +2314,12 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>
> static int
> virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> + virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
> void *opaque)
> {
> virSecurityManagerPtr mgr = opaque;
>
> - return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev, dev->source);
> + return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->source);
> }
>
>
> @@ -2346,7 +2342,7 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
> return virSecuritySELinuxRestoreFileLabel(mgr, database);
>
> case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> - return virSecuritySELinuxRestoreChardevLabel(mgr, def, NULL, dev->data.passthru);
> + return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->data.passthru);
>
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2707,12 +2703,12 @@ virSecuritySELinuxClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>
> static int
> virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
> - virDomainChrDefPtr dev,
> + virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
> void *opaque)
> {
> virSecurityManagerPtr mgr = opaque;
>
> - return virSecuritySELinuxSetChardevLabel(mgr, def, dev, dev->source);
> + return virSecuritySELinuxSetChardevLabel(mgr, def, dev->source);
> }
>
>
> @@ -2736,7 +2732,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
> return virSecuritySELinuxSetFilecon(mgr, database, data->content_context);
>
> case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> - return virSecuritySELinuxSetChardevLabel(mgr, def, NULL,
> + return virSecuritySELinuxSetChardevLabel(mgr, def,
> dev->data.passthru);
>
> default:
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
