Re: [libvirt] [PATCH v2 1/4] conf: move seclabel for chardev source to the correct sturcture

On 05/29/2017 10:31 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > > 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(a)redhat.com&gt;