* Re: [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error
2016-06-26 13:49 [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error Jianfeng Tan
@ 2016-06-28 9:02 ` Yuanhan Liu
2016-06-28 9:41 ` Ferruh Yigit
2016-06-29 3:23 ` [PATCH v2] net/virtio-user: fix build error in SUSE 11 Jianfeng Tan
2 siblings, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2016-06-28 9:02 UTC (permalink / raw)
To: Jianfeng Tan; +Cc: dev, huawei.xie
On Sun, Jun 26, 2016 at 01:49:46PM +0000, Jianfeng Tan wrote:
> On some older systems, such as SUSE 11, the compiling error shows
> as:
> .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
> error: ‘O_CLOEXEC’ undeclared (first use in this function)
>
> The fix is to declare _GNU_SOURCE macro before include fcntl.h.
I think you need give a reason how that error comes and why this
would fix the error.
And if you look at the git history, you might want to define this
macro in Makefile. See commit e49680a87e06 ("mk: compilation fixes").
Last, don't forgot to run ./scripts/check-git-log.sh against your patch.
--yliu
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error
2016-06-26 13:49 [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error Jianfeng Tan
2016-06-28 9:02 ` Yuanhan Liu
@ 2016-06-28 9:41 ` Ferruh Yigit
2016-06-29 1:56 ` Yuanhan Liu
2016-06-29 3:23 ` [PATCH v2] net/virtio-user: fix build error in SUSE 11 Jianfeng Tan
2 siblings, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2016-06-28 9:41 UTC (permalink / raw)
To: Jianfeng Tan, dev
Cc: yuanhan.liu, huawei.xie, Panu Matilainen, Christian Ehrhardt
On 6/26/2016 2:49 PM, Jianfeng Tan wrote:
> On some older systems, such as SUSE 11, the compiling error shows
> as:
> .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
> error: ‘O_CLOEXEC’ undeclared (first use in this function)
>
> The fix is to declare _GNU_SOURCE macro before include fcntl.h.
>
> Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 3d12a32..180f824 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -31,6 +31,7 @@
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#define _GNU_SOURCE
This flag enables glibc extensions, and kind of problem on code
portability. How big problem is it?
Is there any platform that DPDK runs on which uses a libc that isn't
compatible with what this flag provides?
And it seems this flag already used in DPDK, including libraries. If we
are agree on using glibc extensions, does it make sense to move this
flag into a single common location (like mk/exec-env/) ?
> #include <stdint.h>
> #include <stdio.h>
> #include <fcntl.h>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error
2016-06-28 9:41 ` Ferruh Yigit
@ 2016-06-29 1:56 ` Yuanhan Liu
0 siblings, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2016-06-29 1:56 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Jianfeng Tan, dev, huawei.xie, Panu Matilainen,
Christian Ehrhardt, Thomas Monjalon
On Tue, Jun 28, 2016 at 10:41:24AM +0100, Ferruh Yigit wrote:
> On 6/26/2016 2:49 PM, Jianfeng Tan wrote:
> > On some older systems, such as SUSE 11, the compiling error shows
> > as:
> > .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
> > error: ‘O_CLOEXEC’ undeclared (first use in this function)
> >
> > The fix is to declare _GNU_SOURCE macro before include fcntl.h.
> >
> > Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> >
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > ---
> > drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 3d12a32..180f824 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -31,6 +31,7 @@
> > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > */
> >
> > +#define _GNU_SOURCE
> This flag enables glibc extensions, and kind of problem on code
> portability. How big problem is it?
Honestly, I don't know. But I was also thinking that define _GNU_SOURCE
here is an overkill to me. For this issue, we may simply fix it by
reference the O_CLOEXEC flag only when it's defined.
> Is there any platform that DPDK runs on which uses a libc that isn't
> compatible with what this flag provides?
>
> And it seems this flag already used in DPDK, including libraries.
Yes, "git grep _GNU_SOURCE" shows a lot of such usage.
--yliu
> If we
> are agree on using glibc extensions, does it make sense to move this
> flag into a single common location (like mk/exec-env/) ?
>
> > #include <stdint.h>
> > #include <stdio.h>
> > #include <fcntl.h>
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] net/virtio-user: fix build error in SUSE 11
2016-06-26 13:49 [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error Jianfeng Tan
2016-06-28 9:02 ` Yuanhan Liu
2016-06-28 9:41 ` Ferruh Yigit
@ 2016-06-29 3:23 ` Jianfeng Tan
2016-07-01 2:10 ` Yuanhan Liu
2 siblings, 1 reply; 6+ messages in thread
From: Jianfeng Tan @ 2016-06-29 3:23 UTC (permalink / raw)
To: dev; +Cc: yuanhan.liu, ferruh.yigit, Jianfeng Tan
On some older systems, such as SUSE 11, the compiling error shows
as:
.../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
error: ‘O_CLOEXEC’ undeclared (first use in this function)
The fix is to use EFD_CLOEXEC, which is defined in sys/eventfd.h,
instead of O_CLOEXEC which needs _GNU_SOURCE defined on some old
systems.
Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
v2:
- Change the way to fix this issue.
drivers/net/virtio/virtio_user/virtio_user_dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 3d12a32..1b1e5bf 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -63,12 +63,12 @@ virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
/* May use invalid flag, but some backend leverages kickfd and callfd as
* criteria to judge if dev is alive. so finally we use real event_fd.
*/
- callfd = eventfd(0, O_CLOEXEC | O_NONBLOCK);
+ callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
if (callfd < 0) {
PMD_DRV_LOG(ERR, "callfd error, %s\n", strerror(errno));
return -1;
}
- kickfd = eventfd(0, O_CLOEXEC | O_NONBLOCK);
+ kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
if (kickfd < 0) {
close(callfd);
PMD_DRV_LOG(ERR, "kickfd error, %s\n", strerror(errno));
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] net/virtio-user: fix build error in SUSE 11
2016-06-29 3:23 ` [PATCH v2] net/virtio-user: fix build error in SUSE 11 Jianfeng Tan
@ 2016-07-01 2:10 ` Yuanhan Liu
0 siblings, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2016-07-01 2:10 UTC (permalink / raw)
To: Jianfeng Tan; +Cc: dev, ferruh.yigit
On Wed, Jun 29, 2016 at 03:23:03AM +0000, Jianfeng Tan wrote:
> On some older systems, such as SUSE 11, the compiling error shows
> as:
> .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
> error: ‘O_CLOEXEC’ undeclared (first use in this function)
>
> The fix is to use EFD_CLOEXEC, which is defined in sys/eventfd.h,
> instead of O_CLOEXEC which needs _GNU_SOURCE defined on some old
> systems.
>
> Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Applied to dpdk-next-virtio.
Thanks.
--yliu
^ permalink raw reply [flat|nested] 6+ messages in thread