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

On Tue, Jun 13, 2017 at 12:35:10PM -0400, John Ferlan wrote: > > > 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. But this hunk is from virDomainChrSourceDefParseXML() function.

> The rest looks good, so > > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; Thanks Pavel