On Mon, Mar 09, 2020 at 09:29:42 +0100, Michal Privoznik wrote: > Our decision whether to remember seclabel for a disk image > depends on a few factors. If the image is readonly or shared or > not the chain top the remembering is suppressed for the image. > However, the virSecurityManagerSetImageLabel() is too low level > to determine whether passed @src is chain top or not. Even though > the function has the @parent argument it does not necessarily > reflect the chain top - it only points to the top level image in > the chain we want to relabel and not to the topmost image of the > whole chain. And this can't be derived from the passed domain > definition reliably neither - in some cases (like snapshots or > block copy) the @src is added to the definition only after the > operation succeeded. Therefore, introduce a flag which callers > can use to help us with the decision. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/security/security_dac.c | 15 ++++++++++----- > src/security/security_manager.h | 4 ++++ > src/security/security_selinux.c | 17 +++++++++++------ > 3 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index f412054d0e..34da07fc9f 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -889,14 +889,14 @@ static int > virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, > virDomainDefPtr def, > virStorageSourcePtr src, > - virStorageSourcePtr parent) > + virStorageSourcePtr parent, > + bool is_topparent) The variable name is still confusing. Since it's the point of first contact with the code, you should change that one as well. I'm sorry I didn't point this out explicitly in my previous review, but I wanted both the flag name and the variable names changed. Especially since they are the same except for being lowercase. Also please fix all the functions, not just this one.