[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com>
> > ---
> > 
> > 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 redhat com>

Thanks

Pavel

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]