Thanks for looking at these patches.
When I rebased these onto current master, I found that there were new
test failures due to Python tests added on master in the meantime.
Avoiding these (which are genuine bugs) requires a couple of
preparatory commits. I'll re-send this as a 4-patch series soon.
On Wed, Mar 07, 2012 at 01:58:29PM -0800, Ethan Jackson wrote:
> Looks good, thanks.
>
> Ethan
>
> On Mon, Feb 27, 2012 at 11:17, Ben Pfaff <b...@nicira.com> wrote:
> > There's no reason for a Unix domain client socket to bind a name. I don't
> > know why we've always done that. Stevens's "Unix Network Programming"
> > Unix domain socket client example doesn't do a bind.
> >
> > Removes the 'unlink_path' parameter from new_fd_stream() since it is now
> > always passed as NULL.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/stream-fd.c | 10 ++--------
> > lib/stream-fd.h | 4 ++--
> > lib/stream-tcp.c | 4 ++--
> > lib/stream-unix.c | 19 +++++--------------
> > python/ovs/stream.py | 18 +++++-------------
> > 5 files changed, 16 insertions(+), 39 deletions(-)
> >
> > diff --git a/lib/stream-fd.c b/lib/stream-fd.c
> > index 7bf5ebd..38dba7c 100644
> > --- a/lib/stream-fd.c
> > +++ b/lib/stream-fd.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -42,7 +42,6 @@ struct stream_fd
> > {
> > struct stream stream;
> > int fd;
> > - char *unlink_path;
> > };
> >
> > static const struct stream_class stream_fd_class;
> > @@ -55,21 +54,17 @@ static void maybe_unlink_and_free(char *path);
> > * and stores a pointer to the stream in '*streamp'. Initial connection
> > status
> > * 'connect_status' is interpreted as described for stream_init().
> > *
> > - * When '*streamp' is closed, then 'unlink_path' (if nonnull) will be
> > passed to
> > - * fatal_signal_unlink_file_now() and then freed with free().
> > - *
> > * Returns 0 if successful, otherwise a positive errno value. (The current
> > * implementation never fails.) */
> > int
> > new_fd_stream(const char *name, int fd, int connect_status,
> > - char *unlink_path, struct stream **streamp)
> > + struct stream **streamp)
> > {
> > struct stream_fd *s;
> >
> > s = xmalloc(sizeof *s);
> > stream_init(&s->stream, &stream_fd_class, connect_status, name);
> > s->fd = fd;
> > - s->unlink_path = unlink_path;
> > *streamp = &s->stream;
> > return 0;
> > }
> > @@ -86,7 +81,6 @@ fd_close(struct stream *stream)
> > {
> > struct stream_fd *s = stream_fd_cast(stream);
> > close(s->fd);
> > - maybe_unlink_and_free(s->unlink_path);
> > free(s);
> > }
> >
> > diff --git a/lib/stream-fd.h b/lib/stream-fd.h
> > index d2a34eb..b42615f 100644
> > --- a/lib/stream-fd.h
> > +++ b/lib/stream-fd.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -26,7 +26,7 @@ struct pstream;
> > struct sockaddr;
> >
> > int new_fd_stream(const char *name, int fd, int connect_status,
> > - char *unlink_path, struct stream **streamp);
> > + struct stream **streamp);
> > int new_fd_pstream(const char *name, int fd,
> > int (*accept_cb)(int fd, const struct sockaddr *,
> > size_t sa_len, struct stream **),
> > diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
> > index 9a7614d..052ad8c 100644
> > --- a/lib/stream-tcp.c
> > +++ b/lib/stream-tcp.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -58,7 +58,7 @@ new_tcp_stream(const char *name, int fd, int
> > connect_status,
> > return errno;
> > }
> >
> > - retval = new_fd_stream(name, fd, connect_status, NULL, streamp);
> > + retval = new_fd_stream(name, fd, connect_status, streamp);
> > if (!retval) {
> > struct stream *stream = *streamp;
> > stream_set_remote_ip(stream, remote->sin_addr.s_addr);
> > diff --git a/lib/stream-unix.c b/lib/stream-unix.c
> > index d7dde8f..d2e8e82 100644
> > --- a/lib/stream-unix.c
> > +++ b/lib/stream-unix.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -39,28 +39,19 @@ VLOG_DEFINE_THIS_MODULE(stream_unix);
> >
> > /* Active UNIX socket. */
> >
> > -/* Number of unix sockets created so far, to ensure binding path
> > uniqueness. */
> > -static int n_unix_sockets;
> > -
> > static int
> > unix_open(const char *name, char *suffix, struct stream **streamp)
> > {
> > const char *connect_path = suffix;
> > - char *bind_path;
> > int fd;
> >
> > - bind_path = xasprintf("/tmp/stream-unix.%ld.%d",
> > - (long int) getpid(), n_unix_sockets++);
> > - fd = make_unix_socket(SOCK_STREAM, true, false, bind_path,
> > connect_path);
> > + fd = make_unix_socket(SOCK_STREAM, true, false, NULL, connect_path);
> > if (fd < 0) {
> > - VLOG_ERR("%s: connection to %s failed: %s",
> > - bind_path, connect_path, strerror(-fd));
> > - free(bind_path);
> > + VLOG_ERR("%s: connection failed (%s)", connect_path,
> > strerror(-fd));
> > return -fd;
> > }
> >
> > - return new_fd_stream(name, fd, check_connection_completion(fd),
> > - bind_path, streamp);
> > + return new_fd_stream(name, fd, check_connection_completion(fd),
> > streamp);
> > }
> >
> > const struct stream_class unix_stream_class = {
> > @@ -116,7 +107,7 @@ punix_accept(int fd, const struct sockaddr *sa, size_t
> > sa_len,
> > } else {
> > strcpy(name, "unix");
> > }
> > - return new_fd_stream(name, fd, 0, NULL, streamp);
> > + return new_fd_stream(name, fd, 0, streamp);
> > }
> >
> > const struct pstream_class punix_pstream_class = {
> > diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> > index 7ea9e46..08c6293 100644
> > --- a/python/ovs/stream.py
> > +++ b/python/ovs/stream.py
> > @@ -1,4 +1,4 @@
> > -# Copyright (c) 2010, 2011 Nicira Networks
> > +# Copyright (c) 2010, 2011, 2012 Nicira Networks
> > #
> > # Licensed under the Apache License, Version 2.0 (the "License");
> > # you may not use this file except in compliance with the License.
> > @@ -27,7 +27,6 @@ vlog = ovs.vlog.Vlog("stream")
> > class Stream(object):
> > """Bidirectional byte stream. Currently only Unix domain sockets
> > are implemented."""
> > - n_unix_sockets = 0
> >
> > # States.
> > __S_CONNECTING = 0
> > @@ -46,10 +45,9 @@ class Stream(object):
> > False."""
> > return name.startswith("unix:")
> >
> > - def __init__(self, socket, name, bind_path, status):
> > + def __init__(self, socket, name, status):
> > self.socket = socket
> > self.name = name
> > - self.bind_path = bind_path
> > if status == errno.EAGAIN:
> > self.state = Stream.__S_CONNECTING
> > elif status == 0:
> > @@ -76,18 +74,15 @@ class Stream(object):
> > if not Stream.is_valid_name(name):
> > return errno.EAFNOSUPPORT, None
> >
> > - Stream.n_unix_sockets += 1
> > - bind_path = "/tmp/stream-unix.%d.%d" % (os.getpid(),
> > - Stream.n_unix_sockets)
> > connect_path = name[5:]
> > error, sock = ovs.socket_util.make_unix_socket(socket.SOCK_STREAM,
> > - True, bind_path,
> > + True, None,
> > connect_path)
> > if error:
> > return error, None
> > else:
> > status = ovs.socket_util.check_connection_completion(sock)
> > - return 0, Stream(sock, name, bind_path, status)
> > + return 0, Stream(sock, name, status)
> >
> > @staticmethod
> > def open_block((error, stream)):
> > @@ -117,9 +112,6 @@ class Stream(object):
> >
> > def close(self):
> > self.socket.close()
> > - if self.bind_path is not None:
> > - ovs.fatal_signal.unlink_file_now(self.bind_path)
> > - self.bind_path = None
> >
> > def __scs_connecting(self):
> > retval = ovs.socket_util.check_connection_completion(self.socket)
> > @@ -288,7 +280,7 @@ class PassiveStream(object):
> > try:
> > sock, addr = self.socket.accept()
> > ovs.socket_util.set_nonblocking(sock)
> > - return 0, Stream(sock, "unix:%s" % addr, None, 0)
> > + return 0, Stream(sock, "unix:%s" % addr, 0)
> > except socket.error, e:
> > error = ovs.socket_util.get_exception_errno(e)
> > if error != errno.EAGAIN:
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
