* [PATCH v2 0/2] tools/xenstore: simplify xenstored main loop
@ 2021-05-14 11:56 Juergen Gross
2021-05-14 11:56 ` [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct Juergen Gross
2021-05-14 11:56 ` [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop Juergen Gross
0 siblings, 2 replies; 8+ messages in thread
From: Juergen Gross @ 2021-05-14 11:56 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu
Small series to make the main loop of xenstored more readable.
Changes in V2:
- split into two patches
- use const
- NO_SOCKETS handling
Juergen Gross (2):
tools/xenstore: move per connection read and write func hooks into a
struct
tools/xenstore: simplify xenstored main loop
tools/xenstore/xenstored_core.c | 121 ++++++++++++++----------------
tools/xenstore/xenstored_core.h | 21 +++---
tools/xenstore/xenstored_domain.c | 15 +++-
3 files changed, 79 insertions(+), 78 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct 2021-05-14 11:56 [PATCH v2 0/2] tools/xenstore: simplify xenstored main loop Juergen Gross @ 2021-05-14 11:56 ` Juergen Gross 2021-05-14 16:33 ` Julien Grall 2021-05-14 11:56 ` [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop Juergen Gross 1 sibling, 1 reply; 8+ messages in thread From: Juergen Gross @ 2021-05-14 11:56 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu Put the interface type specific functions into an own structure and let struct connection contain only a pointer to that new function vector. Don't even define the socket based functions in case of NO_SOCKETS (Mini-OS). Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - split off from V1 patch (Julien Grall) - use const qualifier (Julien Grall) - drop socket specific case for Mini-OS (Julien Grall) --- tools/xenstore/xenstored_core.c | 44 +++++++++++++------------------ tools/xenstore/xenstored_core.h | 19 +++++++------ tools/xenstore/xenstored_domain.c | 13 +++++++-- 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 4b7b71cfb3..856f518075 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -226,8 +226,8 @@ static bool write_messages(struct connection *conn) sockmsg_string(out->hdr.msg.type), out->hdr.msg.len, out->buffer, conn); - ret = conn->write(conn, out->hdr.raw + out->used, - sizeof(out->hdr) - out->used); + ret = conn->funcs->write(conn, out->hdr.raw + out->used, + sizeof(out->hdr) - out->used); if (ret < 0) return false; @@ -243,8 +243,8 @@ static bool write_messages(struct connection *conn) return true; } - ret = conn->write(conn, out->buffer + out->used, - out->hdr.msg.len - out->used); + ret = conn->funcs->write(conn, out->buffer + out->used, + out->hdr.msg.len - out->used); if (ret < 0) return false; @@ -1531,8 +1531,8 @@ static void handle_input(struct connection *conn) /* Not finished header yet? */ if (in->inhdr) { if (in->used != sizeof(in->hdr)) { - bytes = conn->read(conn, in->hdr.raw + in->used, - sizeof(in->hdr) - in->used); + bytes = conn->funcs->read(conn, in->hdr.raw + in->used, + sizeof(in->hdr) - in->used); if (bytes < 0) goto bad_client; in->used += bytes; @@ -1557,8 +1557,8 @@ static void handle_input(struct connection *conn) in->inhdr = false; } - bytes = conn->read(conn, in->buffer + in->used, - in->hdr.msg.len - in->used); + bytes = conn->funcs->read(conn, in->buffer + in->used, + in->hdr.msg.len - in->used); if (bytes < 0) goto bad_client; @@ -1581,7 +1581,7 @@ static void handle_output(struct connection *conn) ignore_connection(conn); } -struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) +struct connection *new_connection(const struct interface_funcs *funcs) { struct connection *new; @@ -1591,8 +1591,7 @@ struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) new->fd = -1; new->pollfd_idx = -1; - new->write = write; - new->read = read; + new->funcs = funcs; new->is_ignored = false; new->transaction_started = 0; INIT_LIST_HEAD(&new->out_list); @@ -1621,20 +1620,8 @@ struct connection *get_connection_by_id(unsigned int conn_id) static void accept_connection(int sock) { } - -int writefd(struct connection *conn, const void *data, unsigned int len) -{ - errno = EBADF; - return -1; -} - -int readfd(struct connection *conn, void *data, unsigned int len) -{ - errno = EBADF; - return -1; -} #else -int writefd(struct connection *conn, const void *data, unsigned int len) +static int writefd(struct connection *conn, const void *data, unsigned int len) { int rc; @@ -1650,7 +1637,7 @@ int writefd(struct connection *conn, const void *data, unsigned int len) return rc; } -int readfd(struct connection *conn, void *data, unsigned int len) +static int readfd(struct connection *conn, void *data, unsigned int len) { int rc; @@ -1672,6 +1659,11 @@ int readfd(struct connection *conn, void *data, unsigned int len) return rc; } +const struct interface_funcs socket_funcs = { + .write = writefd, + .read = readfd, +}; + static void accept_connection(int sock) { int fd; @@ -1681,7 +1673,7 @@ static void accept_connection(int sock) if (fd < 0) return; - conn = new_connection(writefd, readfd); + conn = new_connection(&socket_funcs); if (conn) conn->fd = fd; else diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 6a6d0448e8..021e41076d 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -86,8 +86,11 @@ struct delayed_request { }; struct connection; -typedef int connwritefn_t(struct connection *, const void *, unsigned int); -typedef int connreadfn_t(struct connection *, void *, unsigned int); + +struct interface_funcs { + int (*write)(struct connection *, const void *, unsigned int); + int (*read)(struct connection *, void *, unsigned int); +}; struct connection { @@ -131,9 +134,8 @@ struct connection /* My watches. */ struct list_head watches; - /* Methods for communicating over this connection: write can be NULL */ - connwritefn_t *write; - connreadfn_t *read; + /* Methods for communicating over this connection. */ + const struct interface_funcs *funcs; /* Support for live update: connection id. */ unsigned int conn_id; @@ -196,7 +198,7 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, struct node *read_node(struct connection *conn, const void *ctx, const char *name); -struct connection *new_connection(connwritefn_t *write, connreadfn_t *read); +struct connection *new_connection(const struct interface_funcs *funcs); struct connection *get_connection_by_id(unsigned int conn_id); void check_store(void); void corrupt(struct connection *conn, const char *fmt, ...); @@ -254,10 +256,7 @@ void finish_daemonize(void); /* Open a pipe for signal handling */ void init_pipe(int reopen_log_pipe[2]); -int writefd(struct connection *conn, const void *data, unsigned int len); -int readfd(struct connection *conn, void *data, unsigned int len); - -extern struct interface_funcs socket_funcs; +extern const struct interface_funcs socket_funcs; extern xengnttab_handle **xgt_handle; int remember_string(struct hashtable *hash, const char *str); diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 0c17937c0f..f3cd56050e 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -172,6 +172,11 @@ static int readchn(struct connection *conn, void *data, unsigned int len) return len; } +static const struct interface_funcs domain_funcs = { + .write = writechn, + .read = readchn, +}; + static void *map_interface(domid_t domid) { return xengnttab_map_grant_ref(*xgt_handle, domid, @@ -389,7 +394,7 @@ static int new_domain(struct domain *domain, int port, bool restore) domain->introduced = true; - domain->conn = new_connection(writechn, readchn); + domain->conn = new_connection(&domain_funcs); if (!domain->conn) { errno = ENOMEM; return errno; @@ -1288,10 +1293,14 @@ void read_state_connection(const void *ctx, const void *state) struct domain *domain, *tdomain; if (sc->conn_type == XS_STATE_CONN_TYPE_SOCKET) { - conn = new_connection(writefd, readfd); +#ifdef NO_SOCKETS + barf("socket based connection without sockets"); +#else + conn = new_connection(&socket_funcs); if (!conn) barf("error restoring connection"); conn->fd = sc->spec.socket_fd; +#endif } else { domain = introduce_domain(ctx, sc->spec.ring.domid, sc->spec.ring.evtchn, true); -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct 2021-05-14 11:56 ` [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct Juergen Gross @ 2021-05-14 16:33 ` Julien Grall 2021-05-17 6:07 ` Juergen Gross 0 siblings, 1 reply; 8+ messages in thread From: Julien Grall @ 2021-05-14 16:33 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu Hi Juergen, On 14/05/2021 12:56, Juergen Gross wrote: > -struct connection *new_connection(connwritefn_t *write, connreadfn_t *read); > +struct connection *new_connection(const struct interface_funcs *funcs); > struct connection *get_connection_by_id(unsigned int conn_id); > void check_store(void); > void corrupt(struct connection *conn, const char *fmt, ...); > @@ -254,10 +256,7 @@ void finish_daemonize(void); > /* Open a pipe for signal handling */ > void init_pipe(int reopen_log_pipe[2]); > > -int writefd(struct connection *conn, const void *data, unsigned int len); > -int readfd(struct connection *conn, void *data, unsigned int len); > - > -extern struct interface_funcs socket_funcs; > +extern const struct interface_funcs socket_funcs; Shouldn't this be protected with #ifdef NO_SOCKETS? The rest looks good to me: Reviewed-by: Julien Grall <jgrall@amazon.com> Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct 2021-05-14 16:33 ` Julien Grall @ 2021-05-17 6:07 ` Juergen Gross 0 siblings, 0 replies; 8+ messages in thread From: Juergen Gross @ 2021-05-17 6:07 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: Ian Jackson, Wei Liu [-- Attachment #1.1.1: Type: text/plain, Size: 1039 bytes --] On 14.05.21 18:33, Julien Grall wrote: > Hi Juergen, > > On 14/05/2021 12:56, Juergen Gross wrote: >> -struct connection *new_connection(connwritefn_t *write, connreadfn_t >> *read); >> +struct connection *new_connection(const struct interface_funcs *funcs); >> struct connection *get_connection_by_id(unsigned int conn_id); >> void check_store(void); >> void corrupt(struct connection *conn, const char *fmt, ...); >> @@ -254,10 +256,7 @@ void finish_daemonize(void); >> /* Open a pipe for signal handling */ >> void init_pipe(int reopen_log_pipe[2]); >> -int writefd(struct connection *conn, const void *data, unsigned int >> len); >> -int readfd(struct connection *conn, void *data, unsigned int len); >> - >> -extern struct interface_funcs socket_funcs; >> +extern const struct interface_funcs socket_funcs; > Shouldn't this be protected with #ifdef NO_SOCKETS? Yes, I can add it. > > The rest looks good to me: > > Reviewed-by: Julien Grall <jgrall@amazon.com> Thanks, Juergen [-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop 2021-05-14 11:56 [PATCH v2 0/2] tools/xenstore: simplify xenstored main loop Juergen Gross 2021-05-14 11:56 ` [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct Juergen Gross @ 2021-05-14 11:56 ` Juergen Gross 2021-05-14 17:05 ` Julien Grall 1 sibling, 1 reply; 8+ messages in thread From: Juergen Gross @ 2021-05-14 11:56 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu The main loop of xenstored is rather complicated due to different handling of socket and ring-page interfaces. Unify that handling by introducing interface type specific functions can_read() and can_write(). Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - split off function vector introduction (Julien Grall) --- tools/xenstore/xenstored_core.c | 77 +++++++++++++++---------------- tools/xenstore/xenstored_core.h | 2 + tools/xenstore/xenstored_domain.c | 2 + 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 856f518075..883a1a582a 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1659,9 +1659,34 @@ static int readfd(struct connection *conn, void *data, unsigned int len) return rc; } +static bool socket_can_process(struct connection *conn, int mask) +{ + if (conn->pollfd_idx == -1) + return false; + + if (fds[conn->pollfd_idx].revents & ~(POLLIN | POLLOUT)) { + talloc_free(conn); + return false; + } + + return (fds[conn->pollfd_idx].revents & mask) && !conn->is_ignored; +} + +static bool socket_can_write(struct connection *conn) +{ + return socket_can_process(conn, POLLOUT); +} + +static bool socket_can_read(struct connection *conn) +{ + return socket_can_process(conn, POLLIN); +} + const struct interface_funcs socket_funcs = { .write = writefd, .read = readfd, + .can_write = socket_can_write, + .can_read = socket_can_read, }; static void accept_connection(int sock) @@ -2296,47 +2321,19 @@ int main(int argc, char *argv[]) if (&next->list != &connections) talloc_increase_ref_count(next); - if (conn->domain) { - if (domain_can_read(conn)) - handle_input(conn); - if (talloc_free(conn) == 0) - continue; - - talloc_increase_ref_count(conn); - if (domain_can_write(conn) && - !list_empty(&conn->out_list)) - handle_output(conn); - if (talloc_free(conn) == 0) - continue; - } else { - if (conn->pollfd_idx != -1) { - if (fds[conn->pollfd_idx].revents - & ~(POLLIN|POLLOUT)) - talloc_free(conn); - else if ((fds[conn->pollfd_idx].revents - & POLLIN) && - !conn->is_ignored) - handle_input(conn); - } - if (talloc_free(conn) == 0) - continue; - - talloc_increase_ref_count(conn); - - if (conn->pollfd_idx != -1) { - if (fds[conn->pollfd_idx].revents - & ~(POLLIN|POLLOUT)) - talloc_free(conn); - else if ((fds[conn->pollfd_idx].revents - & POLLOUT) && - !conn->is_ignored) - handle_output(conn); - } - if (talloc_free(conn) == 0) - continue; + if (conn->funcs->can_read(conn)) + handle_input(conn); + if (talloc_free(conn) == 0) + continue; - conn->pollfd_idx = -1; - } + talloc_increase_ref_count(conn); + + if (conn->funcs->can_write(conn)) + handle_output(conn); + if (talloc_free(conn) == 0) + continue; + + conn->pollfd_idx = -1; } if (delayed_requests) { diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 021e41076d..c6e04c0708 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -90,6 +90,8 @@ struct connection; struct interface_funcs { int (*write)(struct connection *, const void *, unsigned int); int (*read)(struct connection *, void *, unsigned int); + bool (*can_write)(struct connection *); + bool (*can_read)(struct connection *); }; struct connection diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index f3cd56050e..708bf68af0 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -175,6 +175,8 @@ static int readchn(struct connection *conn, void *data, unsigned int len) static const struct interface_funcs domain_funcs = { .write = writechn, .read = readchn, + .can_write = domain_can_write, + .can_read = domain_can_read, }; static void *map_interface(domid_t domid) -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop 2021-05-14 11:56 ` [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop Juergen Gross @ 2021-05-14 17:05 ` Julien Grall 2021-05-17 6:10 ` Juergen Gross 0 siblings, 1 reply; 8+ messages in thread From: Julien Grall @ 2021-05-14 17:05 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu Hi Juergen, On 14/05/2021 12:56, Juergen Gross wrote: > The main loop of xenstored is rather complicated due to different > handling of socket and ring-page interfaces. Unify that handling by > introducing interface type specific functions can_read() and > can_write(). > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - split off function vector introduction (Julien Grall) > --- > tools/xenstore/xenstored_core.c | 77 +++++++++++++++---------------- > tools/xenstore/xenstored_core.h | 2 + > tools/xenstore/xenstored_domain.c | 2 + > 3 files changed, 41 insertions(+), 40 deletions(-) > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c > index 856f518075..883a1a582a 100644 > --- a/tools/xenstore/xenstored_core.c > +++ b/tools/xenstore/xenstored_core.c > @@ -1659,9 +1659,34 @@ static int readfd(struct connection *conn, void *data, unsigned int len) > return rc; > } > > +static bool socket_can_process(struct connection *conn, int mask) > +{ > + if (conn->pollfd_idx == -1) > + return false; > + > + if (fds[conn->pollfd_idx].revents & ~(POLLIN | POLLOUT)) { > + talloc_free(conn); > + return false; > + } > + > + return (fds[conn->pollfd_idx].revents & mask) && !conn->is_ignored; > +} > + > +static bool socket_can_write(struct connection *conn) > +{ > + return socket_can_process(conn, POLLOUT); > +} > + > +static bool socket_can_read(struct connection *conn) > +{ > + return socket_can_process(conn, POLLIN); > +} > + > const struct interface_funcs socket_funcs = { > .write = writefd, > .read = readfd, > + .can_write = socket_can_write, > + .can_read = socket_can_read, > }; > > static void accept_connection(int sock) > @@ -2296,47 +2321,19 @@ int main(int argc, char *argv[]) > if (&next->list != &connections) > talloc_increase_ref_count(next); > > - if (conn->domain) { > - if (domain_can_read(conn)) > - handle_input(conn); > - if (talloc_free(conn) == 0) > - continue; > - > - talloc_increase_ref_count(conn); > - if (domain_can_write(conn) && > - !list_empty(&conn->out_list)) AFAICT, the check "!list_empty(&conn->out_list)" can be safely removed because write_messages() will check if the list is empty (list_top() returns NULL in this case). Is that correct? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop 2021-05-14 17:05 ` Julien Grall @ 2021-05-17 6:10 ` Juergen Gross 2021-05-17 17:54 ` Julien Grall 0 siblings, 1 reply; 8+ messages in thread From: Juergen Gross @ 2021-05-17 6:10 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: Ian Jackson, Wei Liu [-- Attachment #1.1.1: Type: text/plain, Size: 2926 bytes --] On 14.05.21 19:05, Julien Grall wrote: > Hi Juergen, > > On 14/05/2021 12:56, Juergen Gross wrote: >> The main loop of xenstored is rather complicated due to different >> handling of socket and ring-page interfaces. Unify that handling by >> introducing interface type specific functions can_read() and >> can_write(). >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - split off function vector introduction (Julien Grall) >> --- >> tools/xenstore/xenstored_core.c | 77 +++++++++++++++---------------- >> tools/xenstore/xenstored_core.h | 2 + >> tools/xenstore/xenstored_domain.c | 2 + >> 3 files changed, 41 insertions(+), 40 deletions(-) >> >> diff --git a/tools/xenstore/xenstored_core.c >> b/tools/xenstore/xenstored_core.c >> index 856f518075..883a1a582a 100644 >> --- a/tools/xenstore/xenstored_core.c >> +++ b/tools/xenstore/xenstored_core.c >> @@ -1659,9 +1659,34 @@ static int readfd(struct connection *conn, void >> *data, unsigned int len) >> return rc; >> } >> +static bool socket_can_process(struct connection *conn, int mask) >> +{ >> + if (conn->pollfd_idx == -1) >> + return false; >> + >> + if (fds[conn->pollfd_idx].revents & ~(POLLIN | POLLOUT)) { >> + talloc_free(conn); >> + return false; >> + } >> + >> + return (fds[conn->pollfd_idx].revents & mask) && !conn->is_ignored; >> +} >> + >> +static bool socket_can_write(struct connection *conn) >> +{ >> + return socket_can_process(conn, POLLOUT); >> +} >> + >> +static bool socket_can_read(struct connection *conn) >> +{ >> + return socket_can_process(conn, POLLIN); >> +} >> + >> const struct interface_funcs socket_funcs = { >> .write = writefd, >> .read = readfd, >> + .can_write = socket_can_write, >> + .can_read = socket_can_read, >> }; >> static void accept_connection(int sock) >> @@ -2296,47 +2321,19 @@ int main(int argc, char *argv[]) >> if (&next->list != &connections) >> talloc_increase_ref_count(next); >> - if (conn->domain) { >> - if (domain_can_read(conn)) >> - handle_input(conn); >> - if (talloc_free(conn) == 0) >> - continue; >> - >> - talloc_increase_ref_count(conn); >> - if (domain_can_write(conn) && >> - !list_empty(&conn->out_list)) > > AFAICT, the check "!list_empty(&conn->out_list)" can be safely removed > because write_messages() will check if the list is empty (list_top() > returns NULL in this case). Is that correct? Yes. Juergen [-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop 2021-05-17 6:10 ` Juergen Gross @ 2021-05-17 17:54 ` Julien Grall 0 siblings, 0 replies; 8+ messages in thread From: Julien Grall @ 2021-05-17 17:54 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu Hi Juergen, On 17/05/2021 07:10, Juergen Gross wrote: > On 14.05.21 19:05, Julien Grall wrote: >> Hi Juergen, >> >> On 14/05/2021 12:56, Juergen Gross wrote: >>> The main loop of xenstored is rather complicated due to different >>> handling of socket and ring-page interfaces. Unify that handling by >>> introducing interface type specific functions can_read() and >>> can_write(). >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> V2: >>> - split off function vector introduction (Julien Grall) >>> --- >>> tools/xenstore/xenstored_core.c | 77 +++++++++++++++---------------- >>> tools/xenstore/xenstored_core.h | 2 + >>> tools/xenstore/xenstored_domain.c | 2 + >>> 3 files changed, 41 insertions(+), 40 deletions(-) >>> >>> diff --git a/tools/xenstore/xenstored_core.c >>> b/tools/xenstore/xenstored_core.c >>> index 856f518075..883a1a582a 100644 >>> --- a/tools/xenstore/xenstored_core.c >>> +++ b/tools/xenstore/xenstored_core.c >>> @@ -1659,9 +1659,34 @@ static int readfd(struct connection *conn, >>> void *data, unsigned int len) >>> return rc; >>> } >>> +static bool socket_can_process(struct connection *conn, int mask) >>> +{ >>> + if (conn->pollfd_idx == -1) >>> + return false; >>> + >>> + if (fds[conn->pollfd_idx].revents & ~(POLLIN | POLLOUT)) { >>> + talloc_free(conn); >>> + return false; >>> + } >>> + >>> + return (fds[conn->pollfd_idx].revents & mask) && !conn->is_ignored; >>> +} >>> + >>> +static bool socket_can_write(struct connection *conn) >>> +{ >>> + return socket_can_process(conn, POLLOUT); >>> +} >>> + >>> +static bool socket_can_read(struct connection *conn) >>> +{ >>> + return socket_can_process(conn, POLLIN); >>> +} >>> + >>> const struct interface_funcs socket_funcs = { >>> .write = writefd, >>> .read = readfd, >>> + .can_write = socket_can_write, >>> + .can_read = socket_can_read, >>> }; >>> static void accept_connection(int sock) >>> @@ -2296,47 +2321,19 @@ int main(int argc, char *argv[]) >>> if (&next->list != &connections) >>> talloc_increase_ref_count(next); >>> - if (conn->domain) { >>> - if (domain_can_read(conn)) >>> - handle_input(conn); >>> - if (talloc_free(conn) == 0) >>> - continue; >>> - >>> - talloc_increase_ref_count(conn); >>> - if (domain_can_write(conn) && >>> - !list_empty(&conn->out_list)) >> >> AFAICT, the check "!list_empty(&conn->out_list)" can be safely removed >> because write_messages() will check if the list is empty (list_top() >> returns NULL in this case). Is that correct? > > Yes. Thanks, how about adding in the commit message: "Take the opportunity to remove the empty list check before calling write_messages() because the function is already able to cope with an empty list." I can update the commit message while committing it. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-17 17:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-14 11:56 [PATCH v2 0/2] tools/xenstore: simplify xenstored main loop Juergen Gross 2021-05-14 11:56 ` [PATCH v2 1/2] tools/xenstore: move per connection read and write func hooks into a struct Juergen Gross 2021-05-14 16:33 ` Julien Grall 2021-05-17 6:07 ` Juergen Gross 2021-05-14 11:56 ` [PATCH v2 2/2] tools/xenstore: simplify xenstored main loop Juergen Gross 2021-05-14 17:05 ` Julien Grall 2021-05-17 6:10 ` Juergen Gross 2021-05-17 17:54 ` Julien Grall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).