On Mon, May 18, 2020 at 09:26:20AM +0200, Michal Privoznik wrote:
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@redhat.com> --- 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@redhat.com>
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.
Yes, I know, what I meant by "unrelated" here was just the fact that in order to fix Apparmor, you only need the first hunk, I guess I'll be more explicit next time :). It's true that with the first hunk the second becomes redundant, but I still feel like the NOP driver should cover the full spectrum of operations we support, but maybe I'm trying to be overly cautious here. -- Erik Skultety