From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gonglei Subject: Re: [PATCH v3 1/2] contrib: add ivshmem client and server Date: Sun, 10 Aug 2014 11:57:24 +0800 Message-ID: References: <1407488118-11245-1-git-send-email-david.marchand@6wind.com> <1407488118-11245-2-git-send-email-david.marchand@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Cc: 'Olivier Matz' , kvm@vger.kernel.org, claudio.fontana@huawei.com, armbru@redhat.com, arei.gonglei@huawei.com, pbonzini@redhat.com, jani.kokkonen@huawei.com, cam@cs.ualberta.ca To: "'David Marchand'" , Return-path: In-Reply-To: <1407488118-11245-2-git-send-email-david.marchand@6wind.com> Content-Language: zh-cn List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org Hi, > Subject: [Qemu-devel] [PATCH v3 1/2] contrib: add ivshmem client and = server >=20 > When using ivshmem devices, notifications between guests can be sent = as > interrupts using a ivshmem-server (typical use described in = documentation). > The client is provided as a debug tool. >=20 > Signed-off-by: Olivier Matz > Signed-off-by: David Marchand > --- > contrib/ivshmem-client/Makefile | 29 +++ > contrib/ivshmem-client/ivshmem-client.c | 418 > ++++++++++++++++++++++++++++++ > contrib/ivshmem-client/ivshmem-client.h | 238 ++++++++++++++++++ > contrib/ivshmem-client/main.c | 246 ++++++++++++++++++ > contrib/ivshmem-server/Makefile | 29 +++ > contrib/ivshmem-server/ivshmem-server.c | 420 > +++++++++++++++++++++++++++++++ > contrib/ivshmem-server/ivshmem-server.h | 185 ++++++++++++++ > contrib/ivshmem-server/main.c | 296 > ++++++++++++++++++++++ > qemu-doc.texi | 10 +- > 9 files changed, 1868 insertions(+), 3 deletions(-) > create mode 100644 contrib/ivshmem-client/Makefile > create mode 100644 contrib/ivshmem-client/ivshmem-client.c > create mode 100644 contrib/ivshmem-client/ivshmem-client.h > create mode 100644 contrib/ivshmem-client/main.c > create mode 100644 contrib/ivshmem-server/Makefile > create mode 100644 contrib/ivshmem-server/ivshmem-server.c > create mode 100644 contrib/ivshmem-server/ivshmem-server.h > create mode 100644 contrib/ivshmem-server/main.c >=20 > diff --git a/contrib/ivshmem-client/Makefile = b/contrib/ivshmem-client/Makefile > new file mode 100644 > index 0000000..eee97c6 > --- /dev/null > +++ b/contrib/ivshmem-client/Makefile > @@ -0,0 +1,29 @@ > +# Copyright 6WIND S.A., 2014 > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# (at your option) any later version. See the COPYING file in the > +# top-level directory. > + > +S ?=3D $(CURDIR) > +O ?=3D $(CURDIR) > + > +CFLAGS +=3D -Wall -Wextra -Werror -g > +LDFLAGS +=3D > +LDLIBS +=3D -lrt > + > +VPATH =3D $(S) > +PROG =3D ivshmem-client > +OBJS :=3D $(O)/ivshmem-client.o > +OBJS +=3D $(O)/main.o > + > +$(O)/%.o: %.c > + $(CC) $(CFLAGS) -o $@ -c $< > + > +$(O)/$(PROG): $(OBJS) > + $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) > + > +.PHONY: all > +all: $(O)/$(PROG) > + > +clean: > + rm -f $(OBJS) $(O)/$(PROG) > diff --git a/contrib/ivshmem-client/ivshmem-client.c > b/contrib/ivshmem-client/ivshmem-client.c > new file mode 100644 > index 0000000..2166b64 > --- /dev/null > +++ b/contrib/ivshmem-client/ivshmem-client.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright 6WIND S.A., 2014 > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > + * (at your option) any later version. See the COPYING file in the > + * top-level directory. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "ivshmem-client.h" > + > +/* log a message on stdout if verbose=3D1 */ > +#define debug_log(client, fmt, ...) do { \ > + if ((client)->verbose) { \ > + printf(fmt, ## __VA_ARGS__); \ > + } \ > + } while (0) > + > +/* read message from the unix socket */ > +static int > +read_one_msg(struct ivshmem_client *client, long *index, int *fd) > +{ > + int ret; > + struct msghdr msg; > + struct iovec iov[1]; > + union { > + struct cmsghdr cmsg; > + char control[CMSG_SPACE(sizeof(int))]; > + } msg_control; > + struct cmsghdr *cmsg; > + > + iov[0].iov_base =3D index; > + iov[0].iov_len =3D sizeof(*index); > + > + memset(&msg, 0, sizeof(msg)); > + msg.msg_iov =3D iov; > + msg.msg_iovlen =3D 1; > + msg.msg_control =3D &msg_control; > + msg.msg_controllen =3D sizeof(msg_control); > + > + ret =3D recvmsg(client->sock_fd, &msg, 0); > + if (ret < 0) { > + debug_log(client, "cannot read message: %s\n", = strerror(errno)); > + return -1; > + } > + if (ret =3D=3D 0) { > + debug_log(client, "lost connection to server\n"); > + return -1; > + } > + > + *fd =3D -1; > + > + for (cmsg =3D CMSG_FIRSTHDR(&msg); cmsg; cmsg =3D > CMSG_NXTHDR(&msg, cmsg)) { > + > + if (cmsg->cmsg_len !=3D CMSG_LEN(sizeof(int)) || > + cmsg->cmsg_level !=3D SOL_SOCKET || > + cmsg->cmsg_type !=3D SCM_RIGHTS) { > + continue; > + } > + > + memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd)); > + } > + > + return 0; > +} > + > +/* free a peer when the server advertise a disconnection or when the > + * client is freed */ > +static void > +free_peer(struct ivshmem_client *client, struct ivshmem_client_peer = *peer) > +{ > + unsigned vector; > + > + TAILQ_REMOVE(&client->peer_list, peer, next); > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + close(peer->vectors[vector]); > + } > + > + free(peer); > +} > + > +/* handle message coming from server (new peer, new vectors) */ > +static int > +handle_server_msg(struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + long peer_id; > + int ret, fd; > + > + ret =3D read_one_msg(client, &peer_id, &fd); > + if (ret < 0) { > + return -1; > + } > + > + /* can return a peer or the local client */ > + peer =3D ivshmem_client_search_peer(client, peer_id); > + > + /* delete peer */ > + if (fd =3D=3D -1) { > + Maybe the above check should be moved before getting the peer. And the next check peer is extra. > + if (peer =3D=3D NULL || peer =3D=3D &client->local) { > + debug_log(client, "receive delete for invalid peer %ld", Missing '\n' ? > peer_id); > + return -1; > + } > + > + debug_log(client, "delete peer id =3D %ld\n", peer_id); > + free_peer(client, peer); > + return 0; > + } > + > + /* new peer */ > + if (peer =3D=3D NULL) { > + peer =3D malloc(sizeof(*peer)); g_malloc0 ?. > + if (peer =3D=3D NULL) { > + debug_log(client, "cannot allocate new peer\n"); > + return -1; > + } > + memset(peer, 0, sizeof(*peer)); > + peer->id =3D peer_id; > + peer->vectors_count =3D 0; > + TAILQ_INSERT_TAIL(&client->peer_list, peer, next); > + debug_log(client, "new peer id =3D %ld\n", peer_id); > + } > + > + /* new vector */ > + debug_log(client, " new vector %d (fd=3D%d) for peer id %ld\n", > + peer->vectors_count, fd, peer->id); > + peer->vectors[peer->vectors_count] =3D fd; > + peer->vectors_count++; > + > + return 0; > +} > + > +/* init a new ivshmem client */ > +int > +ivshmem_client_init(struct ivshmem_client *client, const char > *unix_sock_path, > + ivshmem_client_notif_cb_t notif_cb, void = *notif_arg, > + int verbose) > +{ > + unsigned i; > + > + memset(client, 0, sizeof(*client)); > + > + snprintf(client->unix_sock_path, sizeof(client->unix_sock_path), > + "%s", unix_sock_path); > + > + for (i =3D 0; i < IVSHMEM_CLIENT_MAX_VECTORS; i++) { > + client->local.vectors[i] =3D -1; > + } > + > + TAILQ_INIT(&client->peer_list); > + client->local.id =3D -1; > + > + client->notif_cb =3D notif_cb; > + client->notif_arg =3D notif_arg; > + client->verbose =3D verbose; Missing client->sock_fd =3D -1; ? > + > + return 0; > +} > + > +/* create and connect to the unix socket */ > +int > +ivshmem_client_connect(struct ivshmem_client *client) > +{ > + struct sockaddr_un sun; > + int fd; > + long tmp; > + > + debug_log(client, "connect to client %s\n", = client->unix_sock_path); > + > + client->sock_fd =3D socket(AF_UNIX, SOCK_STREAM, 0); > + if (client->sock_fd < 0) { > + debug_log(client, "cannot create socket: %s\n", = strerror(errno)); > + return -1; > + } > + > + sun.sun_family =3D AF_UNIX; > + snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", > client->unix_sock_path); > + if (connect(client->sock_fd, (struct sockaddr *)&sun, = sizeof(sun)) < 0) { > + debug_log(client, "cannot connect to %s: %s\n", sun.sun_path, > + strerror(errno)); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; > + } > + > + /* first, we expect our index + a fd =3D=3D -1 */ > + if (read_one_msg(client, &client->local.id, &fd) < 0 || > + client->local.id < 0 || fd !=3D -1) { Why not fd < 0 ? > + debug_log(client, "cannot read from server\n"); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; > + } > + debug_log(client, "our_id=3D%ld\n", client->local.id); > + > + /* now, we expect shared mem fd + a -1 index, note that shm fd > + * is not used */ > + if (read_one_msg(client, &tmp, &fd) < 0 || > + tmp !=3D -1 || fd < 0) { > + debug_log(client, "cannot read from server (2)\n"); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; I think the error logic handle can move the end of this function, = reducing=20 duplicated code. Something like this: goto err; } err: debug_log(client, "cannot read from server (2)\n"); close(client->sock_fd); client->sock_fd =3D -1; return -1; > + } > + debug_log(client, "shm_fd=3D%d\n", fd); > + > + return 0; > +} > + > +/* close connection to the server, and free all peer structures */ > +void > +ivshmem_client_close(struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + unsigned i; > + > + debug_log(client, "close client\n"); > + > + while ((peer =3D TAILQ_FIRST(&client->peer_list)) !=3D NULL) { > + free_peer(client, peer); > + } > + > + close(client->sock_fd); > + client->sock_fd =3D -1; > + client->local.id =3D -1; > + for (i =3D 0; i < IVSHMEM_CLIENT_MAX_VECTORS; i++) { > + client->local.vectors[i] =3D -1; > + } > +} > + > +/* get the fd_set according to the unix socket and peer list */ > +void > +ivshmem_client_get_fds(const struct ivshmem_client *client, fd_set = *fds, > + int *maxfd) > +{ > + int fd; > + unsigned vector; > + > + FD_SET(client->sock_fd, fds); > + if (client->sock_fd >=3D *maxfd) { > + *maxfd =3D client->sock_fd + 1; > + } > + > + for (vector =3D 0; vector < client->local.vectors_count; = vector++) { > + fd =3D client->local.vectors[vector]; > + FD_SET(fd, fds); > + if (fd >=3D *maxfd) { > + *maxfd =3D fd + 1; > + } > + } > +} > + > +/* handle events from eventfd: just print a message on notification = */ > +static int > +handle_event(struct ivshmem_client *client, const fd_set *cur, int = maxfd) > +{ > + struct ivshmem_client_peer *peer; > + uint64_t kick; > + unsigned i; > + int ret; > + > + peer =3D &client->local; > + > + for (i =3D 0; i < peer->vectors_count; i++) { > + if (peer->vectors[i] >=3D maxfd || = !FD_ISSET(peer->vectors[i], cur)) { > + continue; > + } > + > + ret =3D read(peer->vectors[i], &kick, sizeof(kick)); > + if (ret < 0) { > + return ret; > + } > + if (ret !=3D sizeof(kick)) { > + debug_log(client, "invalid read size =3D %d\n", ret); > + errno =3D EINVAL; > + return -1; > + } > + debug_log(client, "received event on fd %d vector %d: %ld\n", > + peer->vectors[i], i, kick); > + if (client->notif_cb !=3D NULL) { > + client->notif_cb(client, peer, i, client->notif_arg); > + } > + } > + > + return 0; > +} > + > +/* read and handle new messages on the given fd_set */ > +int > +ivshmem_client_handle_fds(struct ivshmem_client *client, fd_set *fds, = int > maxfd) > +{ > + if (client->sock_fd < maxfd && FD_ISSET(client->sock_fd, fds) && > + handle_server_msg(client) < 0 && errno !=3D EINTR) { > + debug_log(client, "handle_server_msg() failed\n"); > + return -1; > + } else if (handle_event(client, fds, maxfd) < 0 && errno !=3D = EINTR) { > + debug_log(client, "handle_event() failed\n"); > + return -1; > + } > + > + return 0; > +} > + > +/* send a notification on a vector of a peer */ > +int > +ivshmem_client_notify(const struct ivshmem_client *client, > + const struct ivshmem_client_peer *peer, = unsigned > vector) > +{ > + uint64_t kick; > + int fd; > + > + if (vector > peer->vectors_count) { Maybe if (vector >=3D peer->vectors_count) , otherwise the peer->vectors[] array bounds. > + debug_log(client, "invalid vector %u on peer %ld\n", vector, > peer->id); > + return -1; > + } > + fd =3D peer->vectors[vector]; Or fd =3D peer->vectors[vector - 1]; ? > + debug_log(client, "notify peer %ld on vector %d, fd %d\n", = peer->id, > vector, > + fd); > + > + kick =3D 1; > + if (write(fd, &kick, sizeof(kick)) !=3D sizeof(kick)) { > + fprintf(stderr, "could not write to %d: %s\n", = peer->vectors[vector], > + strerror(errno)); Why not debug_log() at this here? > + return -1; > + } > + return 0; > +} > + > +/* send a notification to all vectors of a peer */ > +int > +ivshmem_client_notify_all_vects(const struct ivshmem_client *client, > + const struct ivshmem_client_peer > *peer) > +{ > + unsigned vector; > + int ret =3D 0; > + > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + if (ivshmem_client_notify(client, peer, vector) < 0) { > + ret =3D -1; The ret's value will be covered when multi clients failed. Do we need store the failed status for server?. > + } > + } > + > + return ret; > +} > + > +/* send a notification to all peers */ > +int > +ivshmem_client_notify_broadcast(const struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + int ret =3D 0; > + > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + if (ivshmem_client_notify_all_vects(client, peer) < 0) { > + ret =3D -1; > + } > + } > + > + return ret; > +} > + > +/* lookup peer from its id */ > +struct ivshmem_client_peer * > +ivshmem_client_search_peer(struct ivshmem_client *client, long = peer_id) > +{ > + struct ivshmem_client_peer *peer; > + > + if (peer_id =3D=3D client->local.id) { > + return &client->local; > + } > + > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + if (peer->id =3D=3D peer_id) { > + return peer; > + } > + } > + return NULL; > +} > + > +/* dump our info, the list of peers their vectors on stdout */ > +void > +ivshmem_client_dump(const struct ivshmem_client *client) > +{ > + const struct ivshmem_client_peer *peer; > + unsigned vector; > + > + /* dump local infos */ > + peer =3D &client->local; > + printf("our_id =3D %ld\n", peer->id); > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + printf(" vector %d is enabled (fd=3D%d)\n", vector, > + peer->vectors[vector]); > + } > + > + /* dump peers */ > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + printf("peer_id =3D %ld\n", peer->id); > + > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + printf(" vector %d is enabled (fd=3D%d)\n", vector, > + peer->vectors[vector]); > + } > + } > +} To be continued... Best regards, -Gonglei From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34455) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGKGk-0001mQ-Dq for qemu-devel@nongnu.org; Sat, 09 Aug 2014 23:58:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XGKGd-0005Kd-OP for qemu-devel@nongnu.org; Sat, 09 Aug 2014 23:58:06 -0400 Received: from blu004-omc1s6.hotmail.com ([65.55.116.17]:50760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGKGd-0005KZ-Jr for qemu-devel@nongnu.org; Sat, 09 Aug 2014 23:57:59 -0400 Message-ID: From: Gonglei References: <1407488118-11245-1-git-send-email-david.marchand@6wind.com> <1407488118-11245-2-git-send-email-david.marchand@6wind.com> In-Reply-To: <1407488118-11245-2-git-send-email-david.marchand@6wind.com> Date: Sun, 10 Aug 2014 11:57:24 +0800 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Subject: Re: [Qemu-devel] [PATCH v3 1/2] contrib: add ivshmem client and server List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'David Marchand' , qemu-devel@nongnu.org Cc: 'Olivier Matz' , kvm@vger.kernel.org, claudio.fontana@huawei.com, armbru@redhat.com, arei.gonglei@huawei.com, pbonzini@redhat.com, jani.kokkonen@huawei.com, cam@cs.ualberta.ca Hi, > Subject: [Qemu-devel] [PATCH v3 1/2] contrib: add ivshmem client and = server >=20 > When using ivshmem devices, notifications between guests can be sent = as > interrupts using a ivshmem-server (typical use described in = documentation). > The client is provided as a debug tool. >=20 > Signed-off-by: Olivier Matz > Signed-off-by: David Marchand > --- > contrib/ivshmem-client/Makefile | 29 +++ > contrib/ivshmem-client/ivshmem-client.c | 418 > ++++++++++++++++++++++++++++++ > contrib/ivshmem-client/ivshmem-client.h | 238 ++++++++++++++++++ > contrib/ivshmem-client/main.c | 246 ++++++++++++++++++ > contrib/ivshmem-server/Makefile | 29 +++ > contrib/ivshmem-server/ivshmem-server.c | 420 > +++++++++++++++++++++++++++++++ > contrib/ivshmem-server/ivshmem-server.h | 185 ++++++++++++++ > contrib/ivshmem-server/main.c | 296 > ++++++++++++++++++++++ > qemu-doc.texi | 10 +- > 9 files changed, 1868 insertions(+), 3 deletions(-) > create mode 100644 contrib/ivshmem-client/Makefile > create mode 100644 contrib/ivshmem-client/ivshmem-client.c > create mode 100644 contrib/ivshmem-client/ivshmem-client.h > create mode 100644 contrib/ivshmem-client/main.c > create mode 100644 contrib/ivshmem-server/Makefile > create mode 100644 contrib/ivshmem-server/ivshmem-server.c > create mode 100644 contrib/ivshmem-server/ivshmem-server.h > create mode 100644 contrib/ivshmem-server/main.c >=20 > diff --git a/contrib/ivshmem-client/Makefile = b/contrib/ivshmem-client/Makefile > new file mode 100644 > index 0000000..eee97c6 > --- /dev/null > +++ b/contrib/ivshmem-client/Makefile > @@ -0,0 +1,29 @@ > +# Copyright 6WIND S.A., 2014 > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# (at your option) any later version. See the COPYING file in the > +# top-level directory. > + > +S ?=3D $(CURDIR) > +O ?=3D $(CURDIR) > + > +CFLAGS +=3D -Wall -Wextra -Werror -g > +LDFLAGS +=3D > +LDLIBS +=3D -lrt > + > +VPATH =3D $(S) > +PROG =3D ivshmem-client > +OBJS :=3D $(O)/ivshmem-client.o > +OBJS +=3D $(O)/main.o > + > +$(O)/%.o: %.c > + $(CC) $(CFLAGS) -o $@ -c $< > + > +$(O)/$(PROG): $(OBJS) > + $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) > + > +.PHONY: all > +all: $(O)/$(PROG) > + > +clean: > + rm -f $(OBJS) $(O)/$(PROG) > diff --git a/contrib/ivshmem-client/ivshmem-client.c > b/contrib/ivshmem-client/ivshmem-client.c > new file mode 100644 > index 0000000..2166b64 > --- /dev/null > +++ b/contrib/ivshmem-client/ivshmem-client.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright 6WIND S.A., 2014 > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > + * (at your option) any later version. See the COPYING file in the > + * top-level directory. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "ivshmem-client.h" > + > +/* log a message on stdout if verbose=3D1 */ > +#define debug_log(client, fmt, ...) do { \ > + if ((client)->verbose) { \ > + printf(fmt, ## __VA_ARGS__); \ > + } \ > + } while (0) > + > +/* read message from the unix socket */ > +static int > +read_one_msg(struct ivshmem_client *client, long *index, int *fd) > +{ > + int ret; > + struct msghdr msg; > + struct iovec iov[1]; > + union { > + struct cmsghdr cmsg; > + char control[CMSG_SPACE(sizeof(int))]; > + } msg_control; > + struct cmsghdr *cmsg; > + > + iov[0].iov_base =3D index; > + iov[0].iov_len =3D sizeof(*index); > + > + memset(&msg, 0, sizeof(msg)); > + msg.msg_iov =3D iov; > + msg.msg_iovlen =3D 1; > + msg.msg_control =3D &msg_control; > + msg.msg_controllen =3D sizeof(msg_control); > + > + ret =3D recvmsg(client->sock_fd, &msg, 0); > + if (ret < 0) { > + debug_log(client, "cannot read message: %s\n", = strerror(errno)); > + return -1; > + } > + if (ret =3D=3D 0) { > + debug_log(client, "lost connection to server\n"); > + return -1; > + } > + > + *fd =3D -1; > + > + for (cmsg =3D CMSG_FIRSTHDR(&msg); cmsg; cmsg =3D > CMSG_NXTHDR(&msg, cmsg)) { > + > + if (cmsg->cmsg_len !=3D CMSG_LEN(sizeof(int)) || > + cmsg->cmsg_level !=3D SOL_SOCKET || > + cmsg->cmsg_type !=3D SCM_RIGHTS) { > + continue; > + } > + > + memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd)); > + } > + > + return 0; > +} > + > +/* free a peer when the server advertise a disconnection or when the > + * client is freed */ > +static void > +free_peer(struct ivshmem_client *client, struct ivshmem_client_peer = *peer) > +{ > + unsigned vector; > + > + TAILQ_REMOVE(&client->peer_list, peer, next); > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + close(peer->vectors[vector]); > + } > + > + free(peer); > +} > + > +/* handle message coming from server (new peer, new vectors) */ > +static int > +handle_server_msg(struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + long peer_id; > + int ret, fd; > + > + ret =3D read_one_msg(client, &peer_id, &fd); > + if (ret < 0) { > + return -1; > + } > + > + /* can return a peer or the local client */ > + peer =3D ivshmem_client_search_peer(client, peer_id); > + > + /* delete peer */ > + if (fd =3D=3D -1) { > + Maybe the above check should be moved before getting the peer. And the next check peer is extra. > + if (peer =3D=3D NULL || peer =3D=3D &client->local) { > + debug_log(client, "receive delete for invalid peer %ld", Missing '\n' ? > peer_id); > + return -1; > + } > + > + debug_log(client, "delete peer id =3D %ld\n", peer_id); > + free_peer(client, peer); > + return 0; > + } > + > + /* new peer */ > + if (peer =3D=3D NULL) { > + peer =3D malloc(sizeof(*peer)); g_malloc0 ?. > + if (peer =3D=3D NULL) { > + debug_log(client, "cannot allocate new peer\n"); > + return -1; > + } > + memset(peer, 0, sizeof(*peer)); > + peer->id =3D peer_id; > + peer->vectors_count =3D 0; > + TAILQ_INSERT_TAIL(&client->peer_list, peer, next); > + debug_log(client, "new peer id =3D %ld\n", peer_id); > + } > + > + /* new vector */ > + debug_log(client, " new vector %d (fd=3D%d) for peer id %ld\n", > + peer->vectors_count, fd, peer->id); > + peer->vectors[peer->vectors_count] =3D fd; > + peer->vectors_count++; > + > + return 0; > +} > + > +/* init a new ivshmem client */ > +int > +ivshmem_client_init(struct ivshmem_client *client, const char > *unix_sock_path, > + ivshmem_client_notif_cb_t notif_cb, void = *notif_arg, > + int verbose) > +{ > + unsigned i; > + > + memset(client, 0, sizeof(*client)); > + > + snprintf(client->unix_sock_path, sizeof(client->unix_sock_path), > + "%s", unix_sock_path); > + > + for (i =3D 0; i < IVSHMEM_CLIENT_MAX_VECTORS; i++) { > + client->local.vectors[i] =3D -1; > + } > + > + TAILQ_INIT(&client->peer_list); > + client->local.id =3D -1; > + > + client->notif_cb =3D notif_cb; > + client->notif_arg =3D notif_arg; > + client->verbose =3D verbose; Missing client->sock_fd =3D -1; ? > + > + return 0; > +} > + > +/* create and connect to the unix socket */ > +int > +ivshmem_client_connect(struct ivshmem_client *client) > +{ > + struct sockaddr_un sun; > + int fd; > + long tmp; > + > + debug_log(client, "connect to client %s\n", = client->unix_sock_path); > + > + client->sock_fd =3D socket(AF_UNIX, SOCK_STREAM, 0); > + if (client->sock_fd < 0) { > + debug_log(client, "cannot create socket: %s\n", = strerror(errno)); > + return -1; > + } > + > + sun.sun_family =3D AF_UNIX; > + snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", > client->unix_sock_path); > + if (connect(client->sock_fd, (struct sockaddr *)&sun, = sizeof(sun)) < 0) { > + debug_log(client, "cannot connect to %s: %s\n", sun.sun_path, > + strerror(errno)); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; > + } > + > + /* first, we expect our index + a fd =3D=3D -1 */ > + if (read_one_msg(client, &client->local.id, &fd) < 0 || > + client->local.id < 0 || fd !=3D -1) { Why not fd < 0 ? > + debug_log(client, "cannot read from server\n"); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; > + } > + debug_log(client, "our_id=3D%ld\n", client->local.id); > + > + /* now, we expect shared mem fd + a -1 index, note that shm fd > + * is not used */ > + if (read_one_msg(client, &tmp, &fd) < 0 || > + tmp !=3D -1 || fd < 0) { > + debug_log(client, "cannot read from server (2)\n"); > + close(client->sock_fd); > + client->sock_fd =3D -1; > + return -1; I think the error logic handle can move the end of this function, = reducing=20 duplicated code. Something like this: goto err; } err: debug_log(client, "cannot read from server (2)\n"); close(client->sock_fd); client->sock_fd =3D -1; return -1; > + } > + debug_log(client, "shm_fd=3D%d\n", fd); > + > + return 0; > +} > + > +/* close connection to the server, and free all peer structures */ > +void > +ivshmem_client_close(struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + unsigned i; > + > + debug_log(client, "close client\n"); > + > + while ((peer =3D TAILQ_FIRST(&client->peer_list)) !=3D NULL) { > + free_peer(client, peer); > + } > + > + close(client->sock_fd); > + client->sock_fd =3D -1; > + client->local.id =3D -1; > + for (i =3D 0; i < IVSHMEM_CLIENT_MAX_VECTORS; i++) { > + client->local.vectors[i] =3D -1; > + } > +} > + > +/* get the fd_set according to the unix socket and peer list */ > +void > +ivshmem_client_get_fds(const struct ivshmem_client *client, fd_set = *fds, > + int *maxfd) > +{ > + int fd; > + unsigned vector; > + > + FD_SET(client->sock_fd, fds); > + if (client->sock_fd >=3D *maxfd) { > + *maxfd =3D client->sock_fd + 1; > + } > + > + for (vector =3D 0; vector < client->local.vectors_count; = vector++) { > + fd =3D client->local.vectors[vector]; > + FD_SET(fd, fds); > + if (fd >=3D *maxfd) { > + *maxfd =3D fd + 1; > + } > + } > +} > + > +/* handle events from eventfd: just print a message on notification = */ > +static int > +handle_event(struct ivshmem_client *client, const fd_set *cur, int = maxfd) > +{ > + struct ivshmem_client_peer *peer; > + uint64_t kick; > + unsigned i; > + int ret; > + > + peer =3D &client->local; > + > + for (i =3D 0; i < peer->vectors_count; i++) { > + if (peer->vectors[i] >=3D maxfd || = !FD_ISSET(peer->vectors[i], cur)) { > + continue; > + } > + > + ret =3D read(peer->vectors[i], &kick, sizeof(kick)); > + if (ret < 0) { > + return ret; > + } > + if (ret !=3D sizeof(kick)) { > + debug_log(client, "invalid read size =3D %d\n", ret); > + errno =3D EINVAL; > + return -1; > + } > + debug_log(client, "received event on fd %d vector %d: %ld\n", > + peer->vectors[i], i, kick); > + if (client->notif_cb !=3D NULL) { > + client->notif_cb(client, peer, i, client->notif_arg); > + } > + } > + > + return 0; > +} > + > +/* read and handle new messages on the given fd_set */ > +int > +ivshmem_client_handle_fds(struct ivshmem_client *client, fd_set *fds, = int > maxfd) > +{ > + if (client->sock_fd < maxfd && FD_ISSET(client->sock_fd, fds) && > + handle_server_msg(client) < 0 && errno !=3D EINTR) { > + debug_log(client, "handle_server_msg() failed\n"); > + return -1; > + } else if (handle_event(client, fds, maxfd) < 0 && errno !=3D = EINTR) { > + debug_log(client, "handle_event() failed\n"); > + return -1; > + } > + > + return 0; > +} > + > +/* send a notification on a vector of a peer */ > +int > +ivshmem_client_notify(const struct ivshmem_client *client, > + const struct ivshmem_client_peer *peer, = unsigned > vector) > +{ > + uint64_t kick; > + int fd; > + > + if (vector > peer->vectors_count) { Maybe if (vector >=3D peer->vectors_count) , otherwise the peer->vectors[] array bounds. > + debug_log(client, "invalid vector %u on peer %ld\n", vector, > peer->id); > + return -1; > + } > + fd =3D peer->vectors[vector]; Or fd =3D peer->vectors[vector - 1]; ? > + debug_log(client, "notify peer %ld on vector %d, fd %d\n", = peer->id, > vector, > + fd); > + > + kick =3D 1; > + if (write(fd, &kick, sizeof(kick)) !=3D sizeof(kick)) { > + fprintf(stderr, "could not write to %d: %s\n", = peer->vectors[vector], > + strerror(errno)); Why not debug_log() at this here? > + return -1; > + } > + return 0; > +} > + > +/* send a notification to all vectors of a peer */ > +int > +ivshmem_client_notify_all_vects(const struct ivshmem_client *client, > + const struct ivshmem_client_peer > *peer) > +{ > + unsigned vector; > + int ret =3D 0; > + > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + if (ivshmem_client_notify(client, peer, vector) < 0) { > + ret =3D -1; The ret's value will be covered when multi clients failed. Do we need store the failed status for server?. > + } > + } > + > + return ret; > +} > + > +/* send a notification to all peers */ > +int > +ivshmem_client_notify_broadcast(const struct ivshmem_client *client) > +{ > + struct ivshmem_client_peer *peer; > + int ret =3D 0; > + > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + if (ivshmem_client_notify_all_vects(client, peer) < 0) { > + ret =3D -1; > + } > + } > + > + return ret; > +} > + > +/* lookup peer from its id */ > +struct ivshmem_client_peer * > +ivshmem_client_search_peer(struct ivshmem_client *client, long = peer_id) > +{ > + struct ivshmem_client_peer *peer; > + > + if (peer_id =3D=3D client->local.id) { > + return &client->local; > + } > + > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + if (peer->id =3D=3D peer_id) { > + return peer; > + } > + } > + return NULL; > +} > + > +/* dump our info, the list of peers their vectors on stdout */ > +void > +ivshmem_client_dump(const struct ivshmem_client *client) > +{ > + const struct ivshmem_client_peer *peer; > + unsigned vector; > + > + /* dump local infos */ > + peer =3D &client->local; > + printf("our_id =3D %ld\n", peer->id); > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + printf(" vector %d is enabled (fd=3D%d)\n", vector, > + peer->vectors[vector]); > + } > + > + /* dump peers */ > + TAILQ_FOREACH(peer, &client->peer_list, next) { > + printf("peer_id =3D %ld\n", peer->id); > + > + for (vector =3D 0; vector < peer->vectors_count; vector++) { > + printf(" vector %d is enabled (fd=3D%d)\n", vector, > + peer->vectors[vector]); > + } > + } > +} To be continued... Best regards, -Gonglei