Patchwork main: Hide F_GETFD and FD_CLOEXEC use for _WIN32

login
register
mail settings
Submitter Corey Bryant
Date Oct. 22, 2012, 1:53 p.m.
Message ID <1350914024-4794-1-git-send-email-coreyb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/193158/
State New
Headers show

Comments

Corey Bryant - Oct. 22, 2012, 1:53 p.m.
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 vl.c | 2 ++
 1 file changed, 2 insertions(+)
Luiz Capitulino - Oct. 22, 2012, 4:16 p.m.
On Mon, 22 Oct 2012 09:53:44 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  vl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/vl.c b/vl.c
> index 200d849..94c667d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -812,11 +812,13 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>          return -1;
>      }
>  
> +#ifndef _WIN32
>      if (fcntl(fd, F_GETFD) & FD_CLOEXEC) {
>          qerror_report(ERROR_CLASS_GENERIC_ERROR,
>                        "fd is not valid or already in use");
>          return -1;
>      }
> +#endif

Which series is this from? The command-line support for fd sets?

qerror_report() is deprecated.

>  
>      if (fdset_id < 0) {
>          qerror_report(ERROR_CLASS_GENERIC_ERROR,
Stefan Weil - Oct. 22, 2012, 4:28 p.m.
Am 22.10.2012 15:53, schrieb Corey Bryant:
> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
> ---
>   vl.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 200d849..94c667d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -812,11 +812,13 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>           return -1;
>       }
>
> +#ifndef _WIN32
>       if (fcntl(fd, F_GETFD)&  FD_CLOEXEC) {
>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>                         "fd is not valid or already in use");
>           return -1;
>       }
> +#endif
>
>       if (fdset_id<  0) {
>           qerror_report(ERROR_CLASS_GENERIC_ERROR,


Are you sure that all other hosts provide F_GETFD and FD_CLOEXEC
and that MinGW will never do that?
Maybe testing those two macros instead of _WIN32 would be better:

#if defined(F_GETFD) && defined(FD_CLOEXEC)
...
#endif

I also think that this patch should be merged with the one which adds
that code and not applied as a separate patch (otherwise git bisect would
be broken).

Regards
Stefan Weil
Corey Bryant - Oct. 22, 2012, 5:35 p.m.
On 10/22/2012 12:28 PM, Stefan Weil wrote:
> Am 22.10.2012 15:53, schrieb Corey Bryant:
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>>   vl.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 200d849..94c667d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -812,11 +812,13 @@ static int parse_add_fd(QemuOpts *opts, void
>> *opaque)
>>           return -1;
>>       }
>>
>> +#ifndef _WIN32
>>       if (fcntl(fd, F_GETFD)&  FD_CLOEXEC) {
>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>>                         "fd is not valid or already in use");
>>           return -1;
>>       }
>> +#endif
>>
>>       if (fdset_id<  0) {
>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>
>
> Are you sure that all other hosts provide F_GETFD and FD_CLOEXEC
> and that MinGW will never do that?

I don't know for sure but use of F_GETFD and FD_CLOEXEC broke the build 
for MinGW.

> Maybe testing those two macros instead of _WIN32 would be better:
>
> #if defined(F_GETFD) && defined(FD_CLOEXEC)
> ...
> #endif
>
> I also think that this patch should be merged with the one which adds
> that code and not applied as a separate patch (otherwise git bisect would
> be broken).

Kevin ended up providing a fix for this in "[PATCH v5] qemu-config: Add 
new -add-fd command line option" which fails the -add-fd command for 
Windows.  This is in-line with how the QMP add-fd command behaves.
Corey Bryant - Oct. 22, 2012, 5:46 p.m.
On 10/22/2012 12:16 PM, Luiz Capitulino wrote:
> On Mon, 22 Oct 2012 09:53:44 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>>   vl.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 200d849..94c667d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -812,11 +812,13 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>           return -1;
>>       }
>>
>> +#ifndef _WIN32
>>       if (fcntl(fd, F_GETFD) & FD_CLOEXEC) {
>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>>                         "fd is not valid or already in use");
>>           return -1;
>>       }
>> +#endif
>
> Which series is this from? The command-line support for fd sets?
>
> qerror_report() is deprecated.
>

Kevin, please let me know if you want me to fix this.  Otherwise I 
assume you'll modify the v5 patch.

>>
>>       if (fdset_id < 0) {
>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>
Kevin Wolf - Oct. 23, 2012, 9:47 a.m.
Am 22.10.2012 19:46, schrieb Corey Bryant:
> 
> 
> On 10/22/2012 12:16 PM, Luiz Capitulino wrote:
>> On Mon, 22 Oct 2012 09:53:44 -0400
>> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>>
>>>
>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>> ---
>>>   vl.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 200d849..94c667d 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -812,11 +812,13 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>>           return -1;
>>>       }
>>>
>>> +#ifndef _WIN32
>>>       if (fcntl(fd, F_GETFD) & FD_CLOEXEC) {
>>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>>>                         "fd is not valid or already in use");
>>>           return -1;
>>>       }
>>> +#endif
>>
>> Which series is this from? The command-line support for fd sets?
>>
>> qerror_report() is deprecated.
>>
> 
> Kevin, please let me know if you want me to fix this.  Otherwise I 
> assume you'll modify the v5 patch.

Any conversion of interfaces should be done on top.

Kevin

Patch

diff --git a/vl.c b/vl.c
index 200d849..94c667d 100644
--- a/vl.c
+++ b/vl.c
@@ -812,11 +812,13 @@  static int parse_add_fd(QemuOpts *opts, void *opaque)
         return -1;
     }
 
+#ifndef _WIN32
     if (fcntl(fd, F_GETFD) & FD_CLOEXEC) {
         qerror_report(ERROR_CLASS_GENERIC_ERROR,
                       "fd is not valid or already in use");
         return -1;
     }
+#endif
 
     if (fdset_id < 0) {
         qerror_report(ERROR_CLASS_GENERIC_ERROR,