| Message ID | 1431432187-10993-6-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/12/2015 06:02 AM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > net/tap.c | 14 +------------- > 1 file changed, 1 insertion(+), 13 deletions(-) > > @@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, > } > > fcntl(fd, F_SETFL, O_NONBLOCK); Well, fcntl() could fail. And don't we have the helper function util/oslib-posix.c:qemu_set_nonblock() for doing this correctly? (Then again, that helper also ignores failures). But the raw use of fcntl here is pre-existing and you didn't touch it, so it doesn't affect my review of this patch. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 05/12/2015 06:02 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> net/tap.c | 14 +------------- >> 1 file changed, 1 insertion(+), 13 deletions(-) >> > >> @@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, >> } >> >> fcntl(fd, F_SETFL, O_NONBLOCK); > > Well, fcntl() could fail. And don't we have the helper function > util/oslib-posix.c:qemu_set_nonblock() for doing this correctly? (Then > again, that helper also ignores failures). > > But the raw use of fcntl here is pre-existing and you didn't touch it, > so it doesn't affect my review of this patch. I guess using the helper makes some sense here, but it's outside this series' scope. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
diff --git a/net/tap.c b/net/tap.c index 8f06cb7..adb1022 100644 --- a/net/tap.c +++ b/net/tap.c @@ -536,7 +536,6 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, /* FIXME error_setg(errp, ...) on failure */ const NetdevBridgeOptions *bridge; const char *helper, *br; - TAPState *s; int fd, vnet_hdr; @@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, } fcntl(fd, F_SETFL, O_NONBLOCK); - vnet_hdr = tap_probe_vnet_hdr(fd); - s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr); - if (!s) { - close(fd); - return -1; - } snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper, br); @@ -607,14 +600,9 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer, int vnet_hdr, int fd) { Error *err = NULL; - TAPState *s; + TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr); int vhostfd; - s = net_tap_fd_init(peer, model, name, fd, vnet_hdr); - if (!s) { - return -1; - } - if (tap_set_sndbuf(s->fd, tap) < 0) { return -1; }
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- net/tap.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)