[PATCH 0/2] virbpf: Set errno instead of reporting errors

Noticed this while helping to debug a CGroupV2 problem. Michal Prívozník (2): virCgroupV2DevicesAvailable: Print stringified errno in the debug log virbpf: Set errno instead of reporting errors po/POTFILES.in | 1 - src/util/virbpf.c | 39 ++++++++++++----------------------- src/util/vircgroupv2devices.c | 2 +- 3 files changed, 14 insertions(+), 28 deletions(-) -- 2.24.1

In the virCgroupV2DevicesAvailable() function we try to determine whether CGroups version 2 are available. We do this by opening what we believe is the CGroup mount point and issuing a BPF call. When the call fails, a debug message is printed. However, the BPF call sets errno too. Include it in the debug message to help us with debugging. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircgroupv2devices.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroupv2devices.c b/src/util/vircgroupv2devices.c index 445f9c53fc..d62ee12a05 100644 --- a/src/util/vircgroupv2devices.c +++ b/src/util/vircgroupv2devices.c @@ -55,7 +55,7 @@ virCgroupV2DevicesAvailable(virCgroupPtr group) } if (virBPFQueryProg(cgroupfd, 0, BPF_CGROUP_DEVICE, &progCnt, NULL) < 0) { - VIR_DEBUG("failed to query cgroup progs"); + VIR_DEBUG("failed to query cgroup progs: %s", g_strerror(errno)); return false; } -- 2.24.1

The virbpf module wraps syscalls to BPF. However, if the kernel headers used at the compile time don't have support for BPF the module offers stubs which return a negative one to signal error to the caller. But there is a slight discrepancy between real functions and these stubs. While the former set errno and return -1 the latter report an error (without setting the errno) and return -1. This is not optimal because the caller might see stale errno and overwrite the error message with a less accurate one. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- po/POTFILES.in | 1 - src/util/virbpf.c | 39 +++++++++++++-------------------------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 982f2ebc36..8af457aa36 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -227,7 +227,6 @@ @SRCDIR@/src/util/virauth.c @SRCDIR@/src/util/virauthconfig.c @SRCDIR@/src/util/virbitmap.c -@SRCDIR@/src/util/virbpf.c @SRCDIR@/src/util/vircgroup.c @SRCDIR@/src/util/vircgroupbackend.c @SRCDIR@/src/util/vircgroupbackend.h diff --git a/src/util/virbpf.c b/src/util/virbpf.c index dec8d0133a..987fc34ef4 100644 --- a/src/util/virbpf.c +++ b/src/util/virbpf.c @@ -302,8 +302,7 @@ virBPFCreateMap(unsigned int mapType G_GNUC_UNUSED, unsigned int valSize G_GNUC_UNUSED, unsigned int maxEntries G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -313,8 +312,7 @@ virBPFLoadProg(struct bpf_insn *insns G_GNUC_UNUSED, int progType G_GNUC_UNUSED, unsigned int insnCnt G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -324,8 +322,7 @@ virBPFAttachProg(int progfd G_GNUC_UNUSED, int targetfd G_GNUC_UNUSED, int attachType G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -335,8 +332,7 @@ virBPFDetachProg(int progfd G_GNUC_UNUSED, int targetfd G_GNUC_UNUSED, int attachType G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -348,8 +344,7 @@ virBPFQueryProg(int targetfd G_GNUC_UNUSED, unsigned int *progcnt G_GNUC_UNUSED, void *progids G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -357,8 +352,7 @@ virBPFQueryProg(int targetfd G_GNUC_UNUSED, int virBPFGetProg(unsigned int id G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -368,8 +362,7 @@ virBPFGetProgInfo(int progfd G_GNUC_UNUSED, struct bpf_prog_info *info G_GNUC_UNUSED, unsigned int **mapIDs G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -377,8 +370,7 @@ virBPFGetProgInfo(int progfd G_GNUC_UNUSED, int virBPFGetMap(unsigned int id G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -387,8 +379,7 @@ int virBPFGetMapInfo(int mapfd G_GNUC_UNUSED, struct bpf_map_info *info G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -398,8 +389,7 @@ virBPFLookupElem(int mapfd G_GNUC_UNUSED, void *key G_GNUC_UNUSED, void *val G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -409,8 +399,7 @@ virBPFGetNextElem(int mapfd G_GNUC_UNUSED, void *key G_GNUC_UNUSED, void *nextKey G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -420,8 +409,7 @@ virBPFUpdateElem(int mapfd G_GNUC_UNUSED, void *key G_GNUC_UNUSED, void *val G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } @@ -430,8 +418,7 @@ int virBPFDeleteElem(int mapfd G_GNUC_UNUSED, void *key G_GNUC_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("BPF not supported with this kernel")); + errno = ENOSYS; return -1; } #endif /* !HAVE_SYS_SYSCALL_H || !HAVE_DECL_BPF_PROG_QUERY */ -- 2.24.1

On 3/9/20 9:31 AM, Michal Privoznik wrote:
Noticed this while helping to debug a CGroupV2 problem.
Michal Prívozník (2): virCgroupV2DevicesAvailable: Print stringified errno in the debug log virbpf: Set errno instead of reporting errors
po/POTFILES.in | 1 - src/util/virbpf.c | 39 ++++++++++++----------------------- src/util/vircgroupv2devices.c | 2 +- 3 files changed, 14 insertions(+), 28 deletions(-)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
participants (2)
-
Daniel Henrique Barboza
-
Michal Privoznik