Re: [PATCH] Don't require secdrivers to implement .domainMoveImageMetadata

On 5/18/20 8:16 AM, Erik Skultety wrote: > On Fri, May 15, 2020 at 05:44:38PM +0200, Michal Privoznik wrote: > > The AppArmor secdriver does not use labels to grant access to > > resources. Therefore, it doesn't use XATTRs and hence it lacks > > implementation of .domainMoveImageMetadata callback. This leads > > to a harmless but needless error message appearing in the logs: > > > > virSecurityManagerMoveImageMetadata:476 : this function is not > > supported by the connection driver: virSecurityManagerMoveImageMetadata > > > > Closes: https://gitlab.com/libvirt/libvirt/-/issues/25 > > > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > > --- > > src/security/security_manager.c | 3 +-- > > src/security/security_nop.c | 10 ---------- > > 2 files changed, 1 insertion(+), 12 deletions(-) > > > > diff --git a/src/security/security_manager.c b/src/security/security_manager.c > > index 2dea294784..b1237d63b6 100644 > > --- a/src/security/security_manager.c > > +++ b/src/security/security_manager.c > > @@ -473,8 +473,7 @@ virSecurityManagerMoveImageMetadata(virSecurityManagerPtr mgr, > > return ret; > > } > > - virReportUnsupportedError(); > > - return -1; > > + return 0; > > } > > To ^this hunk: > Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; > > > diff --git a/src/security/security_nop.c b/src/security/security_nop.c > > index c1856eb421..d5f715b916 100644 > > --- a/src/security/security_nop.c > > +++ b/src/security/security_nop.c > > @@ -225,15 +225,6 @@ virSecurityDomainSetImageLabelNop(virSecurityManagerPtr mgr G_GNUC_UNUSED, > > return 0; > > } > > -static int > > -virSecurityDomainMoveImageMetadataNop(virSecurityManagerPtr mgr G_GNUC_UNUSED, > > - pid_t pid G_GNUC_UNUSED, > > - virStorageSourcePtr src G_GNUC_UNUSED, > > - virStorageSourcePtr dst G_GNUC_UNUSED) > > -{ > > - return 0; > > -} > > - > > ^This is an unrelated change and I also think that the Nop driver should > implement (ideally) all the functions, so I don't see a reason in removing > this one, otherwise we should remove more than just this very function. > It's not unrelated. The NOP driver is always present (see security_drivers[] in src/security/security_driver.c). Therefore, this dummy implementation was needed because if no other driver was loaded then NOP implementation saved us from reporting unsupported error.