[libvirt PATCH] virt-aa-helper: replace usage of magic constant
From: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + } /* XXX should handle open errors more careful than just ignoring them. */ -- 2.52.0
On 1/27/26 11:19, Pavel Hrdina via Devel wrote:
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c
That might be also doable, but in a follow up patch.
src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + }
/* XXX should handle open errors more careful than just ignoring them. */
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On 1/29/26 16:00, Michal Prívozník wrote:
On 1/27/26 11:19, Pavel Hrdina via Devel wrote:
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c
That might be also doable, but in a follow up patch.
src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + }
/* XXX should handle open errors more careful than just ignoring them. */
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ooops, hit 'send' too early. Thing is - virt-aa-helper does not include qemu_domain.h which is where the macro is declared. So this breaks build on systems with apparmor. Michal
On Thu, Jan 29, 2026 at 04:04:17PM +0100, Michal Prívozník wrote:
On 1/29/26 16:00, Michal Prívozník wrote:
On 1/27/26 11:19, Pavel Hrdina via Devel wrote:
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c
That might be also doable, but in a follow up patch.
src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + }
/* XXX should handle open errors more careful than just ignoring them. */
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ooops, hit 'send' too early. Thing is - virt-aa-helper does not include qemu_domain.h which is where the macro is declared. So this breaks build on systems with apparmor.
Right, I'll fix it before pushing, thanks.
Michal
On Thu, Jan 29, 2026 at 16:45:46 +0100, Pavel Hrdina via Devel wrote:
On Thu, Jan 29, 2026 at 04:04:17PM +0100, Michal Prívozník wrote:
On 1/29/26 16:00, Michal Prívozník wrote:
On 1/27/26 11:19, Pavel Hrdina via Devel wrote:
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c
That might be also doable, but in a follow up patch.
src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + }
/* XXX should handle open errors more careful than just ignoring them. */
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ooops, hit 'send' too early. Thing is - virt-aa-helper does not include qemu_domain.h which is where the macro is declared. So this breaks build on systems with apparmor.
Right, I'll fix it before pushing, thanks.
IMO virt-aa-helper must not include qemu_domain.h. Similarly if you'd want to use the constant in virStorageSourceGetMetadata it would need to be moved to the utilities functions first and the qemu driver fixed to use that constant afterwards. I do agree that there's no sane reason why a different driver would want a different value for virStorageSourceGetMetadata, so that refactor should be feasible removing the need for this patch and the problems it brings.
On Thu, Jan 29, 2026 at 04:55:06PM +0100, Peter Krempa wrote:
On Thu, Jan 29, 2026 at 16:45:46 +0100, Pavel Hrdina via Devel wrote:
On Thu, Jan 29, 2026 at 04:04:17PM +0100, Michal Prívozník wrote:
On 1/29/26 16:00, Michal Prívozník wrote:
On 1/27/26 11:19, Pavel Hrdina via Devel wrote:
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
I wonder if we should just drop that argument and use QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH inside the function directly as the only remaining caller with different value is from virstoragetest.c
That might be also doable, but in a follow up patch.
src/security/virt-aa-helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 211c34f926..443646c0a1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -901,12 +901,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. - * - * The maximum depth is limited to 200 layers similarly to the qemu - * implementation. */ - if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); + if (!disk->src->backingStore) { + virStorageSourceGetMetadata(disk->src, -1, -1, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH, + false); + }
/* XXX should handle open errors more careful than just ignoring them. */
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ooops, hit 'send' too early. Thing is - virt-aa-helper does not include qemu_domain.h which is where the macro is declared. So this breaks build on systems with apparmor.
Right, I'll fix it before pushing, thanks.
IMO virt-aa-helper must not include qemu_domain.h.
I was wondering there mist be a reason it's not used there, there is.
Similarly if you'd want to use the constant in virStorageSourceGetMetadata it would need to be moved to the utilities functions first and the qemu driver fixed to use that constant afterwards.
I do agree that there's no sane reason why a different driver would want a different value for virStorageSourceGetMetadata, so that refactor should be feasible removing the need for this patch and the problems it brings.
Agreed, no need to push this patch, I'll look into the refactor. Pavel
participants (3)
-
Michal Prívozník -
Pavel Hrdina -
Peter Krempa