[v2,2/4] osdep: Make qemu_madvise() return ENOSYS on unsupported OSes

Commit Message

Michal Privoznik May 31, 2024, 7:28 a.m. UTC

Not every OS is capable of madvise() or posix_madvise() even. In
that case, errno should be set to ENOSYS as it reflects the cause
better. This also mimic what madvise()/posix_madvise() would
return if kernel lacks corresponding syscall (e.g. due to
configuration).

Signed-off-by: Michal Privoznik <[e-mail]>
---
 util/osdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé May 31, 2024, 7:48 a.m. UTC | #1

On 31/5/24 09:28, Michal Privoznik wrote:
> Not every OS is capable of madvise() or posix_madvise() even. In
> that case, errno should be set to ENOSYS as it reflects the cause
> better. This also mimic what madvise()/posix_madvise() would
> return if kernel lacks corresponding syscall (e.g. due to
> configuration).
> 
> Signed-off-by: Michal Privoznik <[e-mail]>
> ---
>   util/osdep.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>

David Hildenbrand May 31, 2024, 7:53 a.m. UTC | #2

On 31.05.24 09:28, Michal Privoznik wrote:
> Not every OS is capable of madvise() or posix_madvise() even. In
> that case, errno should be set to ENOSYS as it reflects the cause
> better. This also mimic what madvise()/posix_madvise() would
> return if kernel lacks corresponding syscall (e.g. due to
> configuration).

Yes and no. According to the man page

" EINVAL advice is not a valid."

if a particular MADV_* call is not implemented, we would get EINVAL, 
which is really unfortunate ... to detect what is actually supported :(

For the patch here ENOSYS makes sense:

Reviewed-by: David Hildenbrand <[e-mail]>

Philippe Mathieu-Daudé May 31, 2024, 8:02 a.m. UTC | #3

On 31/5/24 09:53, David Hildenbrand wrote:
> On 31.05.24 09:28, Michal Privoznik wrote:
>> Not every OS is capable of madvise() or posix_madvise() even. In
>> that case, errno should be set to ENOSYS as it reflects the cause
>> better. This also mimic what madvise()/posix_madvise() would
>> return if kernel lacks corresponding syscall (e.g. due to
>> configuration).
> 
> Yes and no. According to the man page
> 
> " EINVAL advice is not a valid."
> 
> if a particular MADV_* call is not implemented, we would get EINVAL, 
> which is really unfortunate ... to detect what is actually supported :(

Maybe skip "This also mimic what madvise()/posix_madvise()
would return if kernel lacks corresponding syscall (e.g. due
to configuration)." for clarity?

> For the patch here ENOSYS makes sense:
> 
> Reviewed-by: David Hildenbrand <[e-mail]>
>

Patch

diff --git a/util/osdep.c b/util/osdep.c
index e42f4e8121..5d23bbfbec 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -64,7 +64,7 @@  int qemu_madvise(void *addr, size_t len, int advice)
     }
     return 0;
 #else
-    errno = EINVAL;
+    errno = ENOSYS;
     return -1;
 #endif
 }