[v2] os-posix: Expand setrlimit() syscall compatibility

[PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Trent Huber 5 months, 1 week ago

Darwin uses a subtly different version of the setrlimit() syscall as
described in the COMPATIBILITY section of the macOS man page. The value
of the rlim_cur member has been adjusted accordingly for Darwin-based
systems.

Signed-off-by: Trent Huber <trentmhuber@gmail.com>
---
The previous version assumed OPEN_MAX was a constant defined on all
POSIX systems--turns out it's only a macOS constant. This version adds
preprocessing conditionals to maintain compatibility with Linux.

 os-posix.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/os-posix.c b/os-posix.c
index a4284e2c07..43f9a43f3f 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -270,7 +270,11 @@ void os_setup_limits(void)
         return;
     }
 
+#ifdef CONFIG_DARWIN
+    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
+#else
     nofile.rlim_cur = nofile.rlim_max;
+#endif
 
     if (setrlimit(RLIMIT_NOFILE, &nofile) < 0) {
         warn_report("unable to set NOFILE limit: %s", strerror(errno));
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Daniel P. Berrangé 5 months, 1 week ago

On Fri, Jun 14, 2024 at 05:06:38PM -0400, Trent Huber wrote:
> Darwin uses a subtly different version of the setrlimit() syscall as
> described in the COMPATIBILITY section of the macOS man page. The value
> of the rlim_cur member has been adjusted accordingly for Darwin-based
> systems.
> 
> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
> ---
> The previous version assumed OPEN_MAX was a constant defined on all
> POSIX systems--turns out it's only a macOS constant. This version adds
> preprocessing conditionals to maintain compatibility with Linux.
> 
>  os-posix.c | 4 ++++
>  1 file changed, 4 insertions(+)>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Philippe Mathieu-Daudé 5 months, 1 week ago

Hi Trent,

On 14/6/24 23:06, Trent Huber wrote:
> Darwin uses a subtly different version of the setrlimit() syscall as
> described in the COMPATIBILITY section of the macOS man page. The value
> of the rlim_cur member has been adjusted accordingly for Darwin-based
> systems.
> 
> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
> ---
> The previous version assumed OPEN_MAX was a constant defined on all
> POSIX systems--turns out it's only a macOS constant. This version adds
> preprocessing conditionals to maintain compatibility with Linux.
> 
>   os-posix.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/os-posix.c b/os-posix.c
> index a4284e2c07..43f9a43f3f 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>           return;
>       }
>   
> +#ifdef CONFIG_DARWIN
> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;

Why open-code min()? (The man-page also suggests it).

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +#else
>       nofile.rlim_cur = nofile.rlim_max;
> +#endif
>   
>       if (setrlimit(RLIMIT_NOFILE, &nofile) < 0) {
>           warn_report("unable to set NOFILE limit: %s", strerror(errno));

Re: [PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Michael Tokarev 5 months, 1 week ago

17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
> Hi Trent,
> 
> On 14/6/24 23:06, Trent Huber wrote:
>> Darwin uses a subtly different version of the setrlimit() syscall as
>> described in the COMPATIBILITY section of the macOS man page. The value
>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>> systems.
>>
>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>> ---
>> The previous version assumed OPEN_MAX was a constant defined on all
>> POSIX systems--turns out it's only a macOS constant. This version adds
>> preprocessing conditionals to maintain compatibility with Linux.
>>
>>   os-posix.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/os-posix.c b/os-posix.c
>> index a4284e2c07..43f9a43f3f 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>           return;
>>       }
>> +#ifdef CONFIG_DARWIN
>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
> 
> Why open-code min()? (The man-page also suggests it).

I guess it's because stddef.h isn't included there, so min() isn't immediately
available :)

Applied to trivial-patches,

/mjt

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt

Re: [PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Philippe Mathieu-Daudé 5 months, 1 week ago

On 17/6/24 15:07, Michael Tokarev wrote:
> 17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
>> Hi Trent,
>>
>> On 14/6/24 23:06, Trent Huber wrote:
>>> Darwin uses a subtly different version of the setrlimit() syscall as
>>> described in the COMPATIBILITY section of the macOS man page. The value
>>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>>> systems.
>>>
>>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>>> ---
>>> The previous version assumed OPEN_MAX was a constant defined on all
>>> POSIX systems--turns out it's only a macOS constant. This version adds
>>> preprocessing conditionals to maintain compatibility with Linux.
>>>
>>>   os-posix.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/os-posix.c b/os-posix.c
>>> index a4284e2c07..43f9a43f3f 100644
>>> --- a/os-posix.c
>>> +++ b/os-posix.c
>>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>>           return;
>>>       }
>>> +#ifdef CONFIG_DARWIN
>>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : 
>>> nofile.rlim_max;
>>
>> Why open-code min()? (The man-page also suggests it).
> 
> I guess it's because stddef.h isn't included there, so min() isn't 
> immediately
> available :)

I see os-posix.c -> "qemu/osdep.h" -> <stddef.h>. Anyway,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Applied to trivial-patches,

Thanks!

Re: [PATCH v2] os-posix: Expand setrlimit() syscall compatibility

Posted by Richard Henderson 5 months, 1 week ago

On 6/17/24 08:15, Philippe Mathieu-Daudé wrote:
> On 17/6/24 15:07, Michael Tokarev wrote:
>> 17.06.2024 10:19, Philippe Mathieu-Daudé wrote:
>>> Hi Trent,
>>>
>>> On 14/6/24 23:06, Trent Huber wrote:
>>>> Darwin uses a subtly different version of the setrlimit() syscall as
>>>> described in the COMPATIBILITY section of the macOS man page. The value
>>>> of the rlim_cur member has been adjusted accordingly for Darwin-based
>>>> systems.
>>>>
>>>> Signed-off-by: Trent Huber <trentmhuber@gmail.com>
>>>> ---
>>>> The previous version assumed OPEN_MAX was a constant defined on all
>>>> POSIX systems--turns out it's only a macOS constant. This version adds
>>>> preprocessing conditionals to maintain compatibility with Linux.
>>>>
>>>>   os-posix.c | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/os-posix.c b/os-posix.c
>>>> index a4284e2c07..43f9a43f3f 100644
>>>> --- a/os-posix.c
>>>> +++ b/os-posix.c
>>>> @@ -270,7 +270,11 @@ void os_setup_limits(void)
>>>>           return;
>>>>       }
>>>> +#ifdef CONFIG_DARWIN
>>>> +    nofile.rlim_cur = OPEN_MAX < nofile.rlim_max ? OPEN_MAX : nofile.rlim_max;
>>>
>>> Why open-code min()? (The man-page also suggests it).
>>
>> I guess it's because stddef.h isn't included there, so min() isn't immediately
>> available :)
> 
> I see os-posix.c -> "qemu/osdep.h" -> <stddef.h>. Anyway,

We also have MIN in osdep.h.


r~