[PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro

Peter Maydell posted 1 patch 3 years, 8 months ago
Test FreeBSD passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200704092317.12943-1-peter.maydell@linaro.org
Maintainers: Jason Wang <jasowang@redhat.com>
net/tap-solaris.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Peter Maydell 3 years, 8 months ago
In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
so that it was always included from .c files and never from other .h files.
We missed adding it to net/tap-solaris.c (which previously was pulling it
in via tap-int.h), which broke building on Solaris hosts.

Fixes: a8d2532645cf5ce4
Reported-by: Michele Denber <denber@mindspring.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Untested: Michele, could you give this a try?
---
 net/tap-solaris.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 4725d2314ee..d03165c57c9 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -27,6 +27,7 @@
 #include "tap_int.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
+#include "qemu-common.h"
 
 #include <sys/ethernet.h>
 #include <sys/sockio.h>
-- 
2.20.1


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Thomas Huth 3 years, 8 months ago
On 04/07/2020 11.23, Peter Maydell wrote:
> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
> so that it was always included from .c files and never from other .h files.
> We missed adding it to net/tap-solaris.c (which previously was pulling it
> in via tap-int.h), which broke building on Solaris hosts.
> 
> Fixes: a8d2532645cf5ce4
> Reported-by: Michele Denber <denber@mindspring.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Untested: Michele, could you give this a try?
> ---
>  net/tap-solaris.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> index 4725d2314ee..d03165c57c9 100644
> --- a/net/tap-solaris.c
> +++ b/net/tap-solaris.c
> @@ -27,6 +27,7 @@
>  #include "tap_int.h"
>  #include "qemu/ctype.h"
>  #include "qemu/cutils.h"
> +#include "qemu-common.h"
>  
>  #include <sys/ethernet.h>
>  #include <sys/sockio.h>
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Michele Denber 3 years, 8 months ago
On 07/04/20 13:11, Thomas Huth wrote:
> On 04/07/2020 11.23, Peter Maydell wrote:
>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
>> so that it was always included from .c files and never from other .h files.
>> We missed adding it to net/tap-solaris.c (which previously was pulling it
>> in via tap-int.h), which broke building on Solaris hosts.
>>
>> Fixes: a8d2532645cf5ce4
>> Reported-by: Michele Denber<denber@mindspring.com>
>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
>> ---
>> Untested: Michele, could you give this a try?
>> ---
>>   net/tap-solaris.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
>> index 4725d2314ee..d03165c57c9 100644
>> --- a/net/tap-solaris.c
>> +++ b/net/tap-solaris.c
>> @@ -27,6 +27,7 @@
>>   #include "tap_int.h"
>>   #include "qemu/ctype.h"
>>   #include "qemu/cutils.h"
>> +#include "qemu-common.h"
>>
>>   #include<sys/ethernet.h>
>>   #include<sys/sockio.h>
>>
I can confirm that this works in Solaris 11.4:

root@hemlock:~/qemu-5.0.0# gpatch -p1 < tap-solaris.patch.diff
patching file net/tap-solaris.c
root@hemlock:~/qemu-5.0.0#


             - Michele

Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Laurent Vivier 3 years, 8 months ago
Le 04/07/2020 à 20:13, Michele Denber a écrit :
> On 07/04/20 13:11, Thomas Huth wrote:
>> On 04/07/2020 11.23, Peter Maydell wrote:
>>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
>>> so that it was always included from .c files and never from other .h files.
>>> We missed adding it to net/tap-solaris.c (which previously was pulling it
>>> in via tap-int.h), which broke building on Solaris hosts.
>>>
>>> Fixes: a8d2532645cf5ce4
>>> Reported-by: Michele Denber <denber@mindspring.com>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> Untested: Michele, could you give this a try?
>>> ---
>>>  net/tap-solaris.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
>>> index 4725d2314ee..d03165c57c9 100644
>>> --- a/net/tap-solaris.c
>>> +++ b/net/tap-solaris.c
>>> @@ -27,6 +27,7 @@
>>>  #include "tap_int.h"
>>>  #include "qemu/ctype.h"
>>>  #include "qemu/cutils.h"
>>> +#include "qemu-common.h"
>>>  
>>>  #include <sys/ethernet.h>
>>>  #include <sys/sockio.h>
>>>
> I can confirm that this works in Solaris 11.4:
> 
> root@hemlock:~/qemu-5.0.0# gpatch -p1 < tap-solaris.patch.diff
> patching file net/tap-solaris.c
> root@hemlock:~/qemu-5.0.0#

Do you mean building on Solaris hosts works well now?

In this case, you can add a "Tested-by:" tag.

Thanks,
Laurent


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Michele Denber 3 years, 8 months ago

On 07/06/20 13:04, Laurent Vivier wrote:
> Le 04/07/2020 à 20:13, Michele Denber a écrit :
>> On 07/04/20 13:11, Thomas Huth wrote:
>>> On 04/07/2020 11.23, Peter Maydell wrote:
>>>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
>>>> so that it was always included from .c files and never from other .h files.
>>>> We missed adding it to net/tap-solaris.c (which previously was pulling it
>>>> in via tap-int.h), which broke building on Solaris hosts.
>>>>
>>>> Fixes: a8d2532645cf5ce4
>>>> Reported-by: Michele Denber<denber@mindspring.com>
>>>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
>>>> ---
>>>> Untested: Michele, could you give this a try?
>>>> ---
>>>>   net/tap-solaris.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
>>>> index 4725d2314ee..d03165c57c9 100644
>>>> --- a/net/tap-solaris.c
>>>> +++ b/net/tap-solaris.c
>>>> @@ -27,6 +27,7 @@
>>>>   #include "tap_int.h"
>>>>   #include "qemu/ctype.h"
>>>>   #include "qemu/cutils.h"
>>>> +#include "qemu-common.h"
>>>>
>>>>   #include<sys/ethernet.h>
>>>>   #include<sys/sockio.h>
>>>>
>> I can confirm that this works in Solaris 11.4:
>>
>> root@hemlock:~/qemu-5.0.0# gpatch -p1<  tap-solaris.patch.diff
>> patching file net/tap-solaris.c
>> root@hemlock:~/qemu-5.0.0#
> Do you mean building on Solaris hosts works well now?
In that message I meant that the tap-solaris patch worked.  However, 
yes, I have now successfully built all of QEMU 5.0 on Solaris 11.4 SPARC.
>
> In this case, you can add a "Tested-by:" tag.
I'd be glad to but how do I do that?

             - Michele


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Laurent Vivier 3 years, 8 months ago
Le 07/07/2020 à 17:23, Michele Denber a écrit :
> 
> 
> On 07/06/20 13:04, Laurent Vivier wrote:
>> Le 04/07/2020 à 20:13, Michele Denber a écrit :
>>> On 07/04/20 13:11, Thomas Huth wrote:
>>>> On 04/07/2020 11.23, Peter Maydell wrote:
>>>>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h
>>>>> header
>>>>> so that it was always included from .c files and never from other
>>>>> .h files.
>>>>> We missed adding it to net/tap-solaris.c (which previously was
>>>>> pulling it
>>>>> in via tap-int.h), which broke building on Solaris hosts.
>>>>>
>>>>> Fixes: a8d2532645cf5ce4
>>>>> Reported-by: Michele Denber<denber@mindspring.com>
>>>>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
>>>>> ---
>>>>> Untested: Michele, could you give this a try?
>>>>> ---
>>>>>   net/tap-solaris.c | 1 +
>>>>>   1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
>>>>> index 4725d2314ee..d03165c57c9 100644
>>>>> --- a/net/tap-solaris.c
>>>>> +++ b/net/tap-solaris.c
>>>>> @@ -27,6 +27,7 @@
>>>>>   #include "tap_int.h"
>>>>>   #include "qemu/ctype.h"
>>>>>   #include "qemu/cutils.h"
>>>>> +#include "qemu-common.h"
>>>>>
>>>>>   #include<sys/ethernet.h>
>>>>>   #include<sys/sockio.h>
>>>>>
>>> I can confirm that this works in Solaris 11.4:
>>>
>>> root@hemlock:~/qemu-5.0.0# gpatch -p1<  tap-solaris.patch.diff
>>> patching file net/tap-solaris.c
>>> root@hemlock:~/qemu-5.0.0#
>> Do you mean building on Solaris hosts works well now?
> In that message I meant that the tap-solaris patch worked.  However,
> yes, I have now successfully built all of QEMU 5.0 on Solaris 11.4 SPARC.
>>
>> In this case, you can add a "Tested-by:" tag.
> I'd be glad to but how do I do that?

Philippe has already pointed out your message with all the details.

The "Tested-by:" tag has been added by Philippe when he replied to the
message with the tag and your name.

The patch is now merged in QEMU.

Thanks,
Laurent


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
On Mon, Jul 6, 2020 at 7:30 PM Laurent Vivier <laurent@vivier.eu> wrote:
> Le 04/07/2020 à 20:13, Michele Denber a écrit :
> > On 07/04/20 13:11, Thomas Huth wrote:
> >> On 04/07/2020 11.23, Peter Maydell wrote:
> >>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
> >>> so that it was always included from .c files and never from other .h files.
> >>> We missed adding it to net/tap-solaris.c (which previously was pulling it
> >>> in via tap-int.h), which broke building on Solaris hosts.
> >>>
> >>> Fixes: a8d2532645cf5ce4
> >>> Reported-by: Michele Denber <denber@mindspring.com>
> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>> ---
> >>> Untested: Michele, could you give this a try?
> >>> ---
> >>>  net/tap-solaris.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> >>> index 4725d2314ee..d03165c57c9 100644
> >>> --- a/net/tap-solaris.c
> >>> +++ b/net/tap-solaris.c
> >>> @@ -27,6 +27,7 @@
> >>>  #include "tap_int.h"
> >>>  #include "qemu/ctype.h"
> >>>  #include "qemu/cutils.h"
> >>> +#include "qemu-common.h"
> >>>
> >>>  #include <sys/ethernet.h>
> >>>  #include <sys/sockio.h>
> >>>
> > I can confirm that this works in Solaris 11.4:
> >
> > root@hemlock:~/qemu-5.0.0# gpatch -p1 < tap-solaris.patch.diff
> > patching file net/tap-solaris.c
> > root@hemlock:~/qemu-5.0.0#
>
> Do you mean building on Solaris hosts works well now?

This only shows the patch applies properly, but ...

>
> In this case, you can add a "Tested-by:" tag.

Here after you have the confirmation the patch worked:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg720033.html
So IMO this is worth a:
Tested-by: Michele Denber <denber@mindspring.com>

>
> Thanks,
> Laurent
>
>


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Laurent Vivier 3 years, 8 months ago
Le 06/07/2020 à 19:56, Philippe Mathieu-Daudé a écrit :
> On Mon, Jul 6, 2020 at 7:30 PM Laurent Vivier <laurent@vivier.eu> wrote:
>> Le 04/07/2020 à 20:13, Michele Denber a écrit :
>>> On 07/04/20 13:11, Thomas Huth wrote:
>>>> On 04/07/2020 11.23, Peter Maydell wrote:
>>>>> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
>>>>> so that it was always included from .c files and never from other .h files.
>>>>> We missed adding it to net/tap-solaris.c (which previously was pulling it
>>>>> in via tap-int.h), which broke building on Solaris hosts.
>>>>>
>>>>> Fixes: a8d2532645cf5ce4
>>>>> Reported-by: Michele Denber <denber@mindspring.com>
>>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>> ---
>>>>> Untested: Michele, could you give this a try?
>>>>> ---
>>>>>  net/tap-solaris.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
>>>>> index 4725d2314ee..d03165c57c9 100644
>>>>> --- a/net/tap-solaris.c
>>>>> +++ b/net/tap-solaris.c
>>>>> @@ -27,6 +27,7 @@
>>>>>  #include "tap_int.h"
>>>>>  #include "qemu/ctype.h"
>>>>>  #include "qemu/cutils.h"
>>>>> +#include "qemu-common.h"
>>>>>
>>>>>  #include <sys/ethernet.h>
>>>>>  #include <sys/sockio.h>
>>>>>
>>> I can confirm that this works in Solaris 11.4:
>>>
>>> root@hemlock:~/qemu-5.0.0# gpatch -p1 < tap-solaris.patch.diff
>>> patching file net/tap-solaris.c
>>> root@hemlock:~/qemu-5.0.0#
>>
>> Do you mean building on Solaris hosts works well now?
> 
> This only shows the patch applies properly, but ...
> 
>>
>> In this case, you can add a "Tested-by:" tag.
> 
> Here after you have the confirmation the patch worked:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg720033.html
> So IMO this is worth a:
> Tested-by: Michele Denber <denber@mindspring.com>
> 

Thank you.

Applied to my trivial-patches-for-5.1 branch.

Thanks,
Laurent


Re: [PATCH] net/tap-solaris.c: Include qemu-common.h for TFR macro
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
On 7/4/20 11:23 AM, Peter Maydell wrote:
> In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
> so that it was always included from .c files and never from other .h files.
> We missed adding it to net/tap-solaris.c (which previously was pulling it
> in via tap-int.h), which broke building on Solaris hosts.
> 
> Fixes: a8d2532645cf5ce4
> Reported-by: Michele Denber <denber@mindspring.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Untested: Michele, could you give this a try?
> ---
>  net/tap-solaris.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> index 4725d2314ee..d03165c57c9 100644
> --- a/net/tap-solaris.c
> +++ b/net/tap-solaris.c
> @@ -27,6 +27,7 @@
>  #include "tap_int.h"
>  #include "qemu/ctype.h"
>  #include "qemu/cutils.h"
> +#include "qemu-common.h"
>  
>  #include <sys/ethernet.h>
>  #include <sys/sockio.h>
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>