Hi Jason
On Thu, Nov 15, 2018 at 3:39 AM Jason Wang <jasow...@redhat.com> wrote:
>
>
> On 2018/11/14 下午9:01, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Nov 14, 2018 at 7:46 AM Jason Wang <jasow...@redhat.com> wrote:
> >>
> >> On 2018/11/10 上午3:56, Marc-André Lureau wrote:
> >>> -net socket has a fd argument, and may be passed pre-opened sockets.
> >>>
> >>> TCP sockets use framing.
> >>> UDP sockets have datagram boundaries.
> >>>
> >>> When given a unix dgram socket, it will be able to read from it, but
> >>> will attempt to send on the dgram_dst, which is unset. The other end
> >>> will not receive the data.
> >>>
> >>> Let's teach -net socket to recognize a UNIX DGRAM socket, and use the
> >>> regular send() command (without dgram_dst).
> >>>
> >>> This makes running slirp out-of-process possible that
> >>> way (python pseudo-code):
> >>>
> >>> a, b = socket.socketpair(socket.AF_UNIX, socket.SOCK_DGRAM)
> >>>
> >>> subprocess.Popen('qemu -net socket,fd=%d -net user' % a.fileno(),
> >>> shell=True)
> >>> subprocess.Popen('qemu ... -net nic -net socket,fd=%d' % b.fileno(),
> >>> shell=True)
> >>>
> >>> (to make slirp a seperate project altogether, we would have to have
> >>> some compatibility code and/or deprecate various options & HMP
> >>> commands for dynamic port forwarding etc - but this looks like a
> >>> reachable goal)
> >>>
> >>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> >>
> >> I believe instead of supporting unnamed sockets, we should also support
> >> named one through cli?
> > This could be a later patch, I have no need for it yet. Perhaps it
> > should be a chardev then?
>
>
> I mean something like: -socket id=ud0,path=/tmp/XXX
>
Why not, but I have no need for it. If somebody has, he can make a patch.
> >
> >>> ---
> >>> net/socket.c | 25 +++++++++++++++++++++----
> >>> 1 file changed, 21 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/net/socket.c b/net/socket.c
> >>> index 7095eb749f..8a9c30892d 100644
> >>> --- a/net/socket.c
> >>> +++ b/net/socket.c
> >>> @@ -119,9 +119,13 @@ static ssize_t
> >>> net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
> >>> ssize_t ret;
> >>>
> >>> do {
> >>> - ret = qemu_sendto(s->fd, buf, size, 0,
> >>> - (struct sockaddr *)&s->dgram_dst,
> >>> - sizeof(s->dgram_dst));
> >>> + if (s->dgram_dst.sin_family != AF_UNIX) {
> >>> + ret = qemu_sendto(s->fd, buf, size, 0,
> >>> + (struct sockaddr *)&s->dgram_dst,
> >>> + sizeof(s->dgram_dst));
> >>> + } else {
> >>> + ret = send(s->fd, buf, size, 0);
> >>> + }
> >>
> >> Any reason that send is a must here? send(2) said:
> >> call
> >>
> >> send(sockfd, buf, len, flags);
> >>
> >> is equivalent to
> >>
> >> sendto(sockfd, buf, len, flags, NULL, 0);
> > Yes they should be equivalent, but then we need to add ?: operators
> > for the dest arguments. I preferred to have an if() instead.
> >
> > thanks
>
>
> One possible issue here is I'm not sure there's a equivalent send() in
> e.g non POSIX system.
send() should be as common as sendto().
Should I resend the patch without RFC? no other changes needed?
thanks!
>
> Thanks
>
>
> >
> >>> } while (ret == -1 && errno == EINTR);
> >>>
> >>> if (ret == -1 && errno == EAGAIN) {
> >>> @@ -322,6 +326,15 @@ static NetSocketState
> >>> *net_socket_fd_init_dgram(NetClientState *peer,
> >>> int newfd;
> >>> NetClientState *nc;
> >>> NetSocketState *s;
> >>> + SocketAddress *sa;
> >>> + SocketAddressType sa_type;
> >>> +
> >>> + sa = socket_local_address(fd, errp);
> >>> + if (!sa) {
> >>> + return NULL;
> >>> + }
> >>> + sa_type = sa->type;
> >>> + qapi_free_SocketAddress(sa);
> >>>
> >>> /* fd passed: multicast: "learn" dgram_dst address from bound
> >>> address and save it
> >>> * Because this may be "shared" socket from a "master" process,
> >>> datagrams would be recv()
> >>> @@ -365,8 +378,12 @@ static NetSocketState
> >>> *net_socket_fd_init_dgram(NetClientState *peer,
> >>> "socket: fd=%d (cloned mcast=%s:%d)",
> >>> fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> >>> } else {
> >>> + if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
> >>> + s->dgram_dst.sin_family = AF_UNIX;
> >>> + }
> >>> +
> >>> snprintf(nc->info_str, sizeof(nc->info_str),
> >>> - "socket: fd=%d", fd);
> >>> + "socket: fd=%d %s", fd, SocketAddressType_str(sa_type));
> >>> }
> >>>
> >>> return s;
>
--
Marc-André Lureau
