| Message ID | 1494426918-32737-3-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Wed, May 10, 2017 at 08:05:14PM +0530, Bhupinder Thakur wrote: > Xenconsole supports only PV console currently. This patch adds support > for supporting multiple consoles. > > This patch modifies different data structures and APIs used > in xenconsole to support multiple consoles. > > Change summary: > > 1. Split the domain structure into a console structure and the > domain structure, where each console structure represents one > console. > > 2. Modify different APIs such as buffer_append() etc. to take > console structure as input and perform per console specific > operations. > > 3. Define a generic console_create_ring(), which sets up the > ring buffer and event channel for each console. > > 3. Modify domain_create_ring() to use console_create_ring(). > > 4. Modifications in handle_ring_read() to read ring buffer data > from multiple consoles. > > 5. Add log file support for multiple consoles. I feel that it would be far easier to review if you spilt this patch into 5. Can you do that?
On Wed, 10 May 2017, Bhupinder Thakur wrote: > Xenconsole supports only PV console currently. This patch adds support > for supporting multiple consoles. > > This patch modifies different data structures and APIs used > in xenconsole to support multiple consoles. > > Change summary: > > 1. Split the domain structure into a console structure and the > domain structure, where each console structure represents one > console. > > 2. Modify different APIs such as buffer_append() etc. to take > console structure as input and perform per console specific > operations. > > 3. Define a generic console_create_ring(), which sets up the > ring buffer and event channel for each console. > > 3. Modify domain_create_ring() to use console_create_ring(). > > 4. Modifications in handle_ring_read() to read ring buffer data > from multiple consoles. > > 5. Add log file support for multiple consoles. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> There is something wrong with this patch: I cannot apply it. Also, it is still way to big for me to review. I cannot track all the changes and figure out if they are correct. One option is to introduce struct console in one patch, with only one struct console per domain. Then the second patch could introduce multiple struct console with the helpers such as console_iter_void_arg1. Finally the third patch could add vuart support. > --- > > Changes since v2: > > - Defined a new function console_create_ring() which sets up the ring buffer and > event channel a new console. domain_create_ring() uses this function to setup > a console. > - This patch does not contain vuart specific changes, which would be introduced in > the next patch. > - Changes for keeping the PV log file name unchanged. > > Changes since v1: > > - Split the domain struture to a separate console structure > - Modified the functions to operate on the console struture > - Replaced repetitive per console code with generic code > > tools/console/daemon/io.c | 650 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 460 insertions(+), 190 deletions(-) > > diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c > index 7e6a886..9bb14de 100644 > --- a/tools/console/daemon/io.c > +++ b/tools/console/daemon/io.c > @@ -89,29 +89,139 @@ struct buffer { > size_t max_capacity; > }; > > -struct domain { > - int domid; > +struct console { > + char *xsname; > + char *ttyname; > int master_fd; > int master_pollfd_idx; > int slave_fd; > int log_fd; > - bool is_dead; > - unsigned last_seen; > struct buffer buffer; > - struct domain *next; > - char *conspath; > int ring_ref; > xenevtchn_port_or_error_t local_port; > xenevtchn_port_or_error_t remote_port; > + struct xencons_interface *interface; > + struct domain *d; /* Reference to the domain it is contained in. */ > + char *xspath; > + int (*map_ring_ref)(struct console *, int); > + bool mandatory; > +}; > + > +struct console_data { > + char *xsname; > + char *ttyname; > + int (*mapfunc)(struct console *, int); > + bool mandatory; > +}; > + > +static int map_pvcon_ring_ref(struct console *, int ); > + > +static struct console_data console_data[] = { > + > + { > + .xsname = "/console", > + .ttyname = "tty", > + .mapfunc = map_pvcon_ring_ref, > + .mandatory = true > + }, > +}; > + > +#define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data)) > + > +struct domain { > + int domid; > + bool is_dead; > + unsigned last_seen; > + struct domain *next; > xenevtchn_handle *xce_handle; > int xce_pollfd_idx; > - struct xencons_interface *interface; > int event_count; > long long next_period; > + struct console console[MAX_CONSOLE]; > }; > > static struct domain *dom_head; > > +typedef void (*VOID_ITER_FUNC_ARG1)(struct console *); > +typedef bool (*BOOL_ITER_FUNC_ARG1)(struct console *); > +typedef int (*INT_ITER_FUNC_ARG1)(struct console *); > +typedef void (*VOID_ITER_FUNC_ARG2)(struct console *, unsigned int); > +typedef int (*INT_ITER_FUNC_ARG3)(struct console *, > + struct domain *dom, void **); > + > +static inline bool console_enabled(struct console *con) > +{ > + return con->local_port != -1; > +} > + > +static inline void console_iter_void_arg1(struct domain *d, > + VOID_ITER_FUNC_ARG1 iter_func) > +{ > + int i = 0; > + struct console *con = &(d->console[0]); > + > + for (i = 0; i < MAX_CONSOLE; i++, con++) > + { > + iter_func(con); > + } > +} > + > +static inline void console_iter_void_arg2(struct domain *d, > + VOID_ITER_FUNC_ARG2 iter_func, > + unsigned int iter_data) > +{ > + int i = 0; > + struct console *con = &(d->console[0]); > + > + for (i = 0; i < MAX_CONSOLE; i++, con++) > + { > + iter_func(con, iter_data); > + } > +} > + > +static inline bool console_iter_bool_arg1(struct domain *d, > + BOOL_ITER_FUNC_ARG1 iter_func) > +{ > + int i = 0; > + struct console *con = &(d->console[0]); > + > + for (i = 0; i < MAX_CONSOLE; i++, con++) > + { > + if (iter_func(con)) > + return true; > + } > + return false; > +} > + > +static inline int console_iter_int_arg1(struct domain *d, > + INT_ITER_FUNC_ARG1 iter_func) > +{ > + int i = 0; > + struct console *con = &(d->console[0]); > + > + for (i = 0; i < MAX_CONSOLE; i++, con++) > + { > + if (iter_func(con)) > + return 1; > + } > + return 0; > +} > + > +static inline int console_iter_int_arg3(struct domain *d, > + INT_ITER_FUNC_ARG3 iter_func, > + void *iter_data) > +{ > + int i = 0; > + struct console *con = &(d->console[0]); > + > + for (i = 0; i < MAX_CONSOLE; i++, con++) > + { > + if (iter_func(con, d, iter_data)) > + return 1; > + } > + return 0; > +} > + > static int write_all(int fd, const char* buf, size_t len) > { > while (len) { > @@ -158,11 +268,29 @@ static int write_with_timestamp(int fd, const char *data, size_t sz, > return 0; > } > > -static void buffer_append(struct domain *dom) > +static inline bool buffer_available(struct console *con) > { > - struct buffer *buffer = &dom->buffer; > + if (discard_overflowed_data || > + !con->buffer.max_capacity || > + con->buffer.size < con->buffer.max_capacity) > + return true; > + else > + return false; > +} > + > +static void buffer_append(struct console *con, unsigned int data) > +{ > + struct buffer *buffer = &con->buffer; > + struct xencons_interface *intf = con->interface; > + xenevtchn_port_or_error_t port = con->local_port; > + struct domain *dom = con->d; > + xenevtchn_port_or_error_t rxport = (xenevtchn_port_or_error_t)data; > + > XENCONS_RING_IDX cons, prod, size; > - struct xencons_interface *intf = dom->interface; > + > + /* If incoming data is not for the current console then ignore. */ > + if (port != rxport) > + return; > > cons = intf->out_cons; > prod = intf->out_prod; > @@ -187,22 +315,22 @@ static void buffer_append(struct domain *dom) > > xen_mb(); > intf->out_cons = cons; > - xenevtchn_notify(dom->xce_handle, dom->local_port); > + xenevtchn_notify(dom->xce_handle, port); > > /* Get the data to the logfile as early as possible because if > * no one is listening on the console pty then it will fill up > * and handle_tty_write will stop being called. > */ > - if (dom->log_fd != -1) { > + if (con->log_fd != -1) { > int logret; > if (log_time_guest) { > logret = write_with_timestamp( > - dom->log_fd, > + con->log_fd, > buffer->data + buffer->size - size, > size, &log_time_guest_needts); > } else { > logret = write_all( > - dom->log_fd, > + con->log_fd, > buffer->data + buffer->size - size, > size); > } > @@ -290,12 +418,13 @@ static int create_hv_log(void) > return fd; > } > > -static int create_domain_log(struct domain *dom) > +static int create_console_log(struct console *con) > { > char logfile[PATH_MAX]; > char *namepath, *data, *s; > int fd; > unsigned int len; > + struct domain *dom = con->d; > > namepath = xs_get_domain_path(xs, dom->domid); > s = realloc(namepath, strlen(namepath) + 6); > @@ -314,7 +443,9 @@ static int create_domain_log(struct domain *dom) > return -1; > } > > - snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data); > + snprintf(logfile, PATH_MAX-1, "%s%s/guest-%s.log", > + log_dir, con->xspath, data); This changes the log directory, right? Are the new directories created correctly by the install scripts? > free(data); > logfile[PATH_MAX-1] = '\0'; > > @@ -336,19 +467,24 @@ static int create_domain_log(struct domain *dom) > return fd; > } > > -static void domain_close_tty(struct domain *dom) > +static void console_close_tty(struct console *con) > { > - if (dom->master_fd != -1) { > - close(dom->master_fd); > - dom->master_fd = -1; > + if (con->master_fd != -1) { > + close(con->master_fd); > + con->master_fd = -1; > } > > - if (dom->slave_fd != -1) { > - close(dom->slave_fd); > - dom->slave_fd = -1; > + if (con->slave_fd != -1) { > + close(con->slave_fd); > + con->slave_fd = -1; > } > } > > +static void domain_close_tty(struct domain *dom) > +{ > + console_iter_void_arg1(dom, console_close_tty); > +} > + > #ifdef __sun__ > static int openpty(int *amaster, int *aslave, char *name, > struct termios *termp, struct winsize *winp) > @@ -409,7 +545,7 @@ void cfmakeraw(struct termios *termios_p) > } > #endif /* __sun__ */ > > -static int domain_create_tty(struct domain *dom) > +static int console_create_tty(struct console *con) > { > const char *slave; > char *path; > @@ -418,19 +554,23 @@ static int domain_create_tty(struct domain *dom) > char *data; > unsigned int len; > struct termios term; > + struct domain *dom = con->d; > + > + if (!console_enabled(con)) > + return 0; > > - assert(dom->slave_fd == -1); > - assert(dom->master_fd == -1); > + assert(con->master_fd == -1); > + assert(con->slave_fd == -1); > > - if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) { > + if (openpty(&con->master_fd, &con->slave_fd, NULL, NULL, NULL) < 0) { > err = errno; > dolog(LOG_ERR, "Failed to create tty for domain-%d " > - "(errno = %i, %s)", > - dom->domid, err, strerror(err)); > - return 0; > + "(errno = %i, %s)", > + dom->domid, err, strerror(err)); > + goto out; still changing the return into a goto? > } > > - if (tcgetattr(dom->slave_fd, &term) < 0) { > + if (tcgetattr(con->slave_fd, &term) < 0) { > err = errno; > dolog(LOG_ERR, "Failed to get tty attributes for domain-%d " > "(errno = %i, %s)", > @@ -438,7 +578,7 @@ static int domain_create_tty(struct domain *dom) > goto out; > } > cfmakeraw(&term); > - if (tcsetattr(dom->slave_fd, TCSANOW, &term) < 0) { > + if (tcsetattr(con->slave_fd, TCSANOW, &term) < 0) { > err = errno; > dolog(LOG_ERR, "Failed to set tty attributes for domain-%d " > "(errno = %i, %s)", > @@ -446,40 +586,54 @@ static int domain_create_tty(struct domain *dom) > goto out; > } > > - if ((slave = ptsname(dom->master_fd)) == NULL) { > + if ((slave = ptsname(con->master_fd)) == NULL) { > err = errno; > dolog(LOG_ERR, "Failed to get slave name for domain-%d " > - "(errno = %i, %s)", > - dom->domid, err, strerror(err)); > + "(errno = %i, %s)", > + dom->domid, err, strerror(err)); > goto out; > } > > - success = asprintf(&path, "%s/limit", dom->conspath) != > + success = asprintf(&path, "%s/limit", con->xspath) != > -1; > if (!success) > goto out; > data = xs_read(xs, XBT_NULL, path, &len); > if (data) { > - dom->buffer.max_capacity = strtoul(data, 0, 0); > + con->buffer.max_capacity = strtoul(data, 0, 0); > free(data); > } > free(path); > > - success = (asprintf(&path, "%s/tty", dom->conspath) != -1); > + success = (asprintf(&path, "%s/%s", con->xspath, con->ttyname) != -1); > + > if (!success) > goto out; > success = xs_write(xs, XBT_NULL, path, slave, strlen(slave)); > free(path); > - if (!success) > + > + if (fcntl(con->master_fd, F_SETFL, O_NONBLOCK) == -1) > goto out; > > - if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1) > + if (!success) > goto out; > > - return 1; > -out: > - domain_close_tty(dom); > return 0; > + > +out: > + return 1; > +} > + > +static int domain_create_tty(struct domain *dom) > +{ > + int ret; > + > + ret = console_iter_int_arg1(dom, console_create_tty); > + > + if (ret) > + domain_close_tty(dom); > + > + return ret; > } > > /* Takes tuples of names, scanf-style args, and void **, NULL terminated. */ > @@ -517,31 +671,106 @@ static int xs_gather(struct xs_handle *xs, const char *dir, ...) > return ret; > } > > -static void domain_unmap_interface(struct domain *dom) > +static void console_unmap_interface(struct console *con) > { > - if (dom->interface == NULL) > + if (con->interface == NULL) > return; > - if (xgt_handle && dom->ring_ref == -1) > - xengnttab_unmap(xgt_handle, dom->interface, 1); > + if (xgt_handle && con->ring_ref == -1) > + xengnttab_unmap(xgt_handle, con->interface, 1); > else > - munmap(dom->interface, XC_PAGE_SIZE); > - dom->interface = NULL; > - dom->ring_ref = -1; > + munmap(con->interface, XC_PAGE_SIZE); > + con->interface = NULL; > + con->ring_ref = -1; > +} > + > +static void domain_unmap_interface(struct domain *dom) > +{ > + console_iter_void_arg1(dom, console_unmap_interface); > +} > + > +static int bind_event_channel(struct domain *dom, > + int new_rport, > + int *lport, > + int *rport) > +{ > + int err = 0, rc; > + > + /* Go no further if port has not changed and we are still bound. */ > + if (new_rport == *rport) { > + xc_evtchn_status_t status = { > + .dom = DOMID_SELF, > + .port = *lport }; > + if ((xc_evtchn_status(xc, &status) == 0) && > + (status.status == EVTCHNSTAT_interdomain)) > + goto out; > + } > + > + *lport = -1; > + *rport = -1; > + rc = xenevtchn_bind_interdomain(dom->xce_handle, > + dom->domid, new_rport); > + > + if (rc == -1) { > + err = errno; > + goto out; > + } > + > + *lport = rc; > + *rport = new_rport; > +out: > + return err; > } > > -static int domain_create_ring(struct domain *dom) > +static int map_pvcon_ring_ref(struct console *con, int ring_ref) > { > - int err, remote_port, ring_ref, rc; > + int err = 0; > + struct domain *dom = con->d; > + > + if (!con->interface && xgt_handle) { > + /* Prefer using grant table */ > + con->interface = xengnttab_map_grant_ref(xgt_handle, > + dom->domid, > + GNTTAB_RESERVED_CONSOLE, > + PROT_READ|PROT_WRITE); > + con->ring_ref = -1; > + } > + > + if (!con->interface) { > + con->interface = xc_map_foreign_range(xc, > + dom->domid, > + XC_PAGE_SIZE, > + PROT_READ|PROT_WRITE, > + (unsigned long)ring_ref); > + if (con->interface == NULL) { > + err = EINVAL; > + goto out; > + } > + con->ring_ref = ring_ref; > + } > + > +out: > + return err; > +} > + > +static int console_create_ring(struct console *con) > +{ > + int err, remote_port, ring_ref; > char *type, path[PATH_MAX]; > + struct domain *dom = con->d; > + > + err = xs_gather(xs, con->xspath, > + "ring-ref", "%u", &ring_ref, > + "port", "%i", &remote_port, > + NULL); > > - err = xs_gather(xs, dom->conspath, > - "ring-ref", "%u", &ring_ref, > - "port", "%i", &remote_port, > - NULL); > if (err) > + { > + /* If the console is not mandatory then do not return an error. */ > + if (!con->mandatory) > + err = 0; > goto out; > + } > > - snprintf(path, sizeof(path), "%s/type", dom->conspath); > + snprintf(path, sizeof(path), "%s/type", con->xspath); > type = xs_read(xs, XBT_NULL, path, NULL); > if (type && strcmp(type, "xenconsoled") != 0) { > free(type); > @@ -550,41 +779,44 @@ static int domain_create_ring(struct domain *dom) > free(type); > > /* If using ring_ref and it has changed, remap */ > - if (ring_ref != dom->ring_ref && dom->ring_ref != -1) > - domain_unmap_interface(dom); > + if (ring_ref != con->ring_ref && con->ring_ref != -1) > + console_unmap_interface(con); > > - if (!dom->interface && xgt_handle) { > - /* Prefer using grant table */ > - dom->interface = xengnttab_map_grant_ref(xgt_handle, > - dom->domid, GNTTAB_RESERVED_CONSOLE, > - PROT_READ|PROT_WRITE); > - dom->ring_ref = -1; > - } > - if (!dom->interface) { > - /* Fall back to xc_map_foreign_range */ > - dom->interface = xc_map_foreign_range( > - xc, dom->domid, XC_PAGE_SIZE, > - PROT_READ|PROT_WRITE, > - (unsigned long)ring_ref); > - if (dom->interface == NULL) { > - err = EINVAL; > - goto out; > + err = con->map_ring_ref(con, ring_ref); > + > + if (err) > + goto out; > + > + err = bind_event_channel(dom, remote_port, > + &con->local_port, > + &con->remote_port); > + if (err) > + goto out1; > + > + if (con->master_fd == -1) { > + if (console_create_tty(con)) { > + err = errno; > + con->local_port = -1; > + con->remote_port = -1; > + goto out1; > } > - dom->ring_ref = ring_ref; > } > > - /* Go no further if port has not changed and we are still bound. */ > - if (remote_port == dom->remote_port) { > - xc_evtchn_status_t status = { > - .dom = DOMID_SELF, > - .port = dom->local_port }; > - if ((xc_evtchn_status(xc, &status) == 0) && > - (status.status == EVTCHNSTAT_interdomain)) > - goto out; > - } > + if (log_guest && (con->log_fd == -1)) > + con->log_fd = create_console_log(con); > + > + return err; > + > + out1: > + console_unmap_interface(con); > + out: > + return err; > +} > + > +static int domain_create_ring(struct domain *dom) > +{ > + int err; > > - dom->local_port = -1; > - dom->remote_port = -1; > if (dom->xce_handle != NULL) > xenevtchn_close(dom->xce_handle); > > @@ -592,37 +824,17 @@ static int domain_create_ring(struct domain *dom) > * wasteful, but that's how the code is structured... */ > dom->xce_handle = xenevtchn_open(NULL, 0); > if (dom->xce_handle == NULL) { > - err = errno; > - goto out; > + return errno; > } > - > - rc = xenevtchn_bind_interdomain(dom->xce_handle, > - dom->domid, remote_port); > > - if (rc == -1) { > - err = errno; > + err = console_iter_int_arg1(dom, console_create_ring); > + > + if (err) > + { > xenevtchn_close(dom->xce_handle); > dom->xce_handle = NULL; > - goto out; > - } > - dom->local_port = rc; > - dom->remote_port = remote_port; > - > - if (dom->master_fd == -1) { > - if (!domain_create_tty(dom)) { > - err = errno; > - xenevtchn_close(dom->xce_handle); > - dom->xce_handle = NULL; > - dom->local_port = -1; > - dom->remote_port = -1; > - goto out; > - } > } > > - if (log_guest && (dom->log_fd == -1)) > - dom->log_fd = create_domain_log(dom); > - > - out: > return err; > } > > @@ -630,27 +842,66 @@ static bool watch_domain(struct domain *dom, bool watch) > { > char domid_str[3 + MAX_STRLEN(dom->domid)]; > bool success; > + char *path = dom->console[0].xspath; > > snprintf(domid_str, sizeof(domid_str), "dom%u", dom->domid); > if (watch) { > - success = xs_watch(xs, dom->conspath, domid_str); > + success = xs_watch(xs, path, domid_str); > if (success) > domain_create_ring(dom); > else > - xs_unwatch(xs, dom->conspath, domid_str); > + xs_unwatch(xs, path, domid_str); > } else { > - success = xs_unwatch(xs, dom->conspath, domid_str); > + success = xs_unwatch(xs, path, domid_str); > } > > return success; > } > > +static int console_init(struct console *con, struct domain *dom, void **data) > +{ > + char *s; > + int err = -1; > + struct console_data **con_data = (struct console_data **)data; > + > + con->master_fd = -1; > + con->master_pollfd_idx = -1; > + con->slave_fd = -1; > + con->log_fd = -1; > + con->ring_ref = -1; > + con->local_port = -1; > + con->remote_port = -1; > + con->d = dom; > + con->ttyname = (*con_data)->ttyname; > + con->xsname = (*con_data)->xsname; > + con->map_ring_ref = (*con_data)->mapfunc; > + con->mandatory = (*con_data)->mandatory; > + con->xspath = xs_get_domain_path(xs, dom->domid); > + s = realloc(con->xspath, strlen(con->xspath) + > + strlen(con->xsname) + 1); > + > + (*con_data)++; > + > + if (s) > + { > + con->xspath = s; > + strcat(con->xspath, con->xsname); > + err = 0; > + } > + return err; > +} > + > +static void console_free(struct console *con) > +{ > + if (con->xspath) > + free(con->xspath); > +} > > static struct domain *create_domain(int domid) > { > struct domain *dom; > - char *s; > struct timespec ts; > + struct console_data *con_data = &console_data[0]; > > if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { > dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d", > @@ -667,26 +918,13 @@ static struct domain *create_domain(int domid) > > dom->domid = domid; > > - dom->conspath = xs_get_domain_path(xs, dom->domid); > - s = realloc(dom->conspath, strlen(dom->conspath) + > - strlen("/console") + 1); > - if (s == NULL) > + if (console_iter_int_arg3(dom, console_init, (void **)&con_data)) > goto out; > - dom->conspath = s; > - strcat(dom->conspath, "/console"); > > - dom->master_fd = -1; > - dom->master_pollfd_idx = -1; > - dom->slave_fd = -1; > - dom->log_fd = -1; > dom->xce_pollfd_idx = -1; > > dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD; > > - dom->ring_ref = -1; > - dom->local_port = -1; > - dom->remote_port = -1; > - > if (!watch_domain(dom, true)) > goto out; > > @@ -696,8 +934,10 @@ static struct domain *create_domain(int domid) > dolog(LOG_DEBUG, "New domain %d", domid); > > return dom; > + > out: > - free(dom->conspath); > + console_iter_void_arg1(dom, console_free); > + > free(dom); > return NULL; > } > @@ -727,20 +967,24 @@ static void remove_domain(struct domain *dom) > } > } > > -static void cleanup_domain(struct domain *d) > -{ > - domain_close_tty(d); > > - if (d->log_fd != -1) { > - close(d->log_fd); > - d->log_fd = -1; > +static void console_cleanup(struct console *con) > +{ > + if (con->log_fd != -1) { > + close(con->log_fd); > + con->log_fd = -1; > } > + free(con->buffer.data); > + con->buffer.data = NULL; > + free(con->xspath); > + con->xspath = NULL; > +} > > - free(d->buffer.data); > - d->buffer.data = NULL; > +static void cleanup_domain(struct domain *d) > +{ > + domain_close_tty(d); > > - free(d->conspath); > - d->conspath = NULL; > + console_iter_void_arg1(d, console_cleanup); > > remove_domain(d); > } > @@ -780,9 +1024,9 @@ static void enum_domains(void) > } > } > > -static int ring_free_bytes(struct domain *dom) > +static int ring_free_bytes(struct console *con) > { > - struct xencons_interface *intf = dom->interface; > + struct xencons_interface *intf = con->interface; > XENCONS_RING_IDX cons, prod, space; > > cons = intf->in_cons; > @@ -807,25 +1051,27 @@ static void domain_handle_broken_tty(struct domain *dom, int recreate) > } > } > > -static void handle_tty_read(struct domain *dom) > +static void handle_tty_read(struct console *con) > { > ssize_t len = 0; > char msg[80]; > int i; > - struct xencons_interface *intf = dom->interface; > XENCONS_RING_IDX prod; > + struct xencons_interface *intf = con->interface; > + xenevtchn_port_or_error_t port = con->local_port; > + struct domain *dom = con->d; > > if (dom->is_dead) > return; > > - len = ring_free_bytes(dom); > + len = ring_free_bytes(con); > if (len == 0) > return; > > if (len > sizeof(msg)) > len = sizeof(msg); > > - len = read(dom->master_fd, msg, len); > + len = read(con->master_fd, msg, len); > /* > * Note: on Solaris, len == 0 means the slave closed, and this > * is no problem, but Linux can't handle this usefully, so we > @@ -841,31 +1087,39 @@ static void handle_tty_read(struct domain *dom) > } > xen_wmb(); > intf->in_prod = prod; > - xenevtchn_notify(dom->xce_handle, dom->local_port); > + xenevtchn_notify(dom->xce_handle, port); > } else { > domain_close_tty(dom); > shutdown_domain(dom); > } > } > > -static void handle_tty_write(struct domain *dom) > +static void handle_tty_write(struct console *con) > { > ssize_t len; > + struct domain *dom = con->d; > > if (dom->is_dead) > return; > > - len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed, > - dom->buffer.size - dom->buffer.consumed); > + len = write(con->master_fd, > + con->buffer.data + con->buffer.consumed, > + con->buffer.size - con->buffer.consumed); > if (len < 1) { > dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n", > dom->domid, len, errno); > domain_handle_broken_tty(dom, domain_is_valid(dom->domid)); > } else { > - buffer_advance(&dom->buffer, len); > + buffer_advance(&con->buffer, len); > } > } > > +static void console_event_unmask(struct console *con) > +{ > + if (con->local_port != -1) > + (void)xenevtchn_unmask(con->d->xce_handle, con->local_port); > +} > + > static void handle_ring_read(struct domain *dom) > { > xenevtchn_port_or_error_t port; > @@ -878,10 +1132,10 @@ static void handle_ring_read(struct domain *dom) > > dom->event_count++; > > - buffer_append(dom); > + console_iter_void_arg2(dom, buffer_append, port); > > if (dom->event_count < RATE_LIMIT_ALLOWANCE) > - (void)xenevtchn_unmask(dom->xce_handle, port); > + console_iter_void_arg1(dom, console_event_unmask); > } > > static void handle_xs(void) > @@ -943,14 +1197,22 @@ static void handle_hv_logs(xenevtchn_handle *xce_handle, bool force) > (void)xenevtchn_unmask(xce_handle, port); > } > > +static void console_open_log(struct console *con) > +{ > + if (console_enabled(con)) > + { > + if (con->log_fd != -1) > + close(con->log_fd); > + con->log_fd = create_console_log(con); > + } > +} > + > static void handle_log_reload(void) > { > if (log_guest) { > struct domain *d; > for (d = dom_head; d; d = d->next) { > - if (d->log_fd != -1) > - close(d->log_fd); > - d->log_fd = create_domain_log(d); > + console_iter_void_arg1(d, console_open_log); > } > } > > @@ -1002,6 +1264,40 @@ static void reset_fds(void) > memset(fds, 0, sizeof(struct pollfd) * current_array_size); > } > > +static void add_console_fd(struct console *con) > +{ > + if (con->master_fd != -1) { > + short events = 0; > + if (!con->d->is_dead && ring_free_bytes(con)) > + events |= POLLIN; > + > + if (!buffer_empty(&con->buffer)) > + events |= POLLOUT; > + > + if (events) > + con->master_pollfd_idx = > + set_fds(con->master_fd, events|POLLPRI); > + } > +} > + > +static void process_console(struct console *con) > +{ > + if (con->master_fd != -1 && con->master_pollfd_idx != -1) { > + if (fds[con->master_pollfd_idx].revents & > + ~(POLLIN|POLLOUT|POLLPRI)) > + domain_handle_broken_tty(con->d, domain_is_valid(con->d->domid)); > + else { > + if (fds[con->master_pollfd_idx].revents & > + POLLIN) > + handle_tty_read(con); > + if (fds[con->master_pollfd_idx].revents & > + POLLOUT) > + handle_tty_write(con); > + } > + } > + con->master_pollfd_idx = -1; > +} > + > void handle_io(void) > { > int ret; > @@ -1068,7 +1364,7 @@ void handle_io(void) > if ((now+5) > d->next_period) { > d->next_period = now + RATE_LIMIT_PERIOD; > if (d->event_count >= RATE_LIMIT_ALLOWANCE) { > - (void)xenevtchn_unmask(d->xce_handle, d->local_port); > + console_iter_void_arg1(d, console_event_unmask); > } > d->event_count = 0; > } > @@ -1081,28 +1377,15 @@ void handle_io(void) > d->next_period < next_timeout) > next_timeout = d->next_period; > } else if (d->xce_handle != NULL) { > - if (discard_overflowed_data || > - !d->buffer.max_capacity || > - d->buffer.size < d->buffer.max_capacity) { > + if (console_iter_bool_arg1(d, buffer_available)) > + { > int evtchn_fd = xenevtchn_fd(d->xce_handle); > d->xce_pollfd_idx = set_fds(evtchn_fd, > - POLLIN|POLLPRI); > + POLLIN|POLLPRI); spurious change > } > } > > - if (d->master_fd != -1) { > - short events = 0; > - if (!d->is_dead && ring_free_bytes(d)) > - events |= POLLIN; > - > - if (!buffer_empty(&d->buffer)) > - events |= POLLOUT; > - > - if (events) > - d->master_pollfd_idx = > - set_fds(d->master_fd, > - events|POLLPRI); > - } > + console_iter_void_arg1(d, add_console_fd); > } > > /* If any domain has been rate limited, we need to work > @@ -1170,22 +1453,9 @@ void handle_io(void) > handle_ring_read(d); > } > > - if (d->master_fd != -1 && d->master_pollfd_idx != -1) { > - if (fds[d->master_pollfd_idx].revents & > - ~(POLLIN|POLLOUT|POLLPRI)) > - domain_handle_broken_tty(d, > - domain_is_valid(d->domid)); > - else { > - if (fds[d->master_pollfd_idx].revents & > - POLLIN) > - handle_tty_read(d); > - if (fds[d->master_pollfd_idx].revents & > - POLLOUT) > - handle_tty_write(d); > - } > - } > + console_iter_void_arg1(d, process_console); > > - d->xce_pollfd_idx = d->master_pollfd_idx = -1; > + d->xce_pollfd_idx = -1; > > if (d->last_seen != enum_pass) > shutdown_domain(d);
Hi, On 17 May 2017 at 05:11, Stefano Stabellini <sstabellini@kernel.org> wrote: > On Wed, 10 May 2017, Bhupinder Thakur wrote: >> Xenconsole supports only PV console currently. This patch adds support >> for supporting multiple consoles. >> >> This patch modifies different data structures and APIs used >> in xenconsole to support multiple consoles. >> >> Change summary: >> >> 1. Split the domain structure into a console structure and the >> domain structure, where each console structure represents one >> console. >> >> 2. Modify different APIs such as buffer_append() etc. to take >> console structure as input and perform per console specific >> operations. >> >> 3. Define a generic console_create_ring(), which sets up the >> ring buffer and event channel for each console. >> >> 3. Modify domain_create_ring() to use console_create_ring(). >> >> 4. Modifications in handle_ring_read() to read ring buffer data >> from multiple consoles. >> >> 5. Add log file support for multiple consoles. >> >> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > > There is something wrong with this patch: I cannot apply it. > > Also, it is still way to big for me to review. I cannot track all the > changes and figure out if they are correct. > > One option is to introduce struct console in one patch, with only one > struct console per domain. Then the second patch could introduce > multiple struct console with the helpers such as console_iter_void_arg1. > > Finally the third patch could add vuart support. > I have divided the changes into 4 patches: patch#1: This patch introduces the console structure and modifies the code to use the new console structure. patch#2: This patch modifies the functions to take console structure as input instead of domain structure. Also it renames the console specific functions to start with "console_" prefix instead of "domain_" prefix. For example - domain_create_tty() is renamed to console_create_tty(). patch#3: This patch adds the support for multiple consoles and introduces the iterator functions to operate on multiple consoles. patch#4: Finally this patch adds support for a new vuart console. >> -static int create_domain_log(struct domain *dom) >> +static int create_console_log(struct console *con) >> { >> char logfile[PATH_MAX]; >> char *namepath, *data, *s; >> int fd; >> unsigned int len; >> + struct domain *dom = con->d; >> >> namepath = xs_get_domain_path(xs, dom->domid); >> s = realloc(namepath, strlen(namepath) + 6); >> @@ -314,7 +443,9 @@ static int create_domain_log(struct domain *dom) >> return -1; >> } >> >> - snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data); >> + snprintf(logfile, PATH_MAX-1, "%s%s/guest-%s.log", >> + log_dir, con->xspath, data); > > This changes the log directory, right? Are the new directories created > correctly by the install scripts? I will correct this. There should be no change in the path for PV console log. I think by default guest logging is disabled. How can I enable the logging to test it? I believe some option needs to be passed while spawning xenconsoled? Regards, Bhupinder
On Thu, 25 May 2017, Bhupinder Thakur wrote: > Hi, > > On 17 May 2017 at 05:11, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Wed, 10 May 2017, Bhupinder Thakur wrote: > >> Xenconsole supports only PV console currently. This patch adds support > >> for supporting multiple consoles. > >> > >> This patch modifies different data structures and APIs used > >> in xenconsole to support multiple consoles. > >> > >> Change summary: > >> > >> 1. Split the domain structure into a console structure and the > >> domain structure, where each console structure represents one > >> console. > >> > >> 2. Modify different APIs such as buffer_append() etc. to take > >> console structure as input and perform per console specific > >> operations. > >> > >> 3. Define a generic console_create_ring(), which sets up the > >> ring buffer and event channel for each console. > >> > >> 3. Modify domain_create_ring() to use console_create_ring(). > >> > >> 4. Modifications in handle_ring_read() to read ring buffer data > >> from multiple consoles. > >> > >> 5. Add log file support for multiple consoles. > >> > >> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > > > > There is something wrong with this patch: I cannot apply it. > > > > Also, it is still way to big for me to review. I cannot track all the > > changes and figure out if they are correct. > > > > One option is to introduce struct console in one patch, with only one > > struct console per domain. Then the second patch could introduce > > multiple struct console with the helpers such as console_iter_void_arg1. > > > > Finally the third patch could add vuart support. > > > I have divided the changes into 4 patches: > > patch#1: This patch introduces the console structure and modifies the > code to use the new console structure. > > patch#2: This patch modifies the functions to take console structure > as input instead of domain structure. Also it renames the console > specific functions to start with "console_" prefix instead of > "domain_" prefix. For example - domain_create_tty() is renamed to > console_create_tty(). > > patch#3: This patch adds the support for multiple consoles and > introduces the iterator functions to operate on multiple consoles. > > patch#4: Finally this patch adds support for a new vuart console. Thank you, it looks better on paper > >> -static int create_domain_log(struct domain *dom) > >> +static int create_console_log(struct console *con) > >> { > >> char logfile[PATH_MAX]; > >> char *namepath, *data, *s; > >> int fd; > >> unsigned int len; > >> + struct domain *dom = con->d; > >> > >> namepath = xs_get_domain_path(xs, dom->domid); > >> s = realloc(namepath, strlen(namepath) + 6); > >> @@ -314,7 +443,9 @@ static int create_domain_log(struct domain *dom) > >> return -1; > >> } > >> > >> - snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data); > >> + snprintf(logfile, PATH_MAX-1, "%s%s/guest-%s.log", > >> + log_dir, con->xspath, data); > > > > This changes the log directory, right? Are the new directories created > > correctly by the install scripts? > I will correct this. There should be no change in the path for PV > console log. I think by default guest logging is disabled. How can I > enable the logging to test it? I believe some option needs to be > passed while spawning xenconsoled? Yes, it looks like it's the -l option, see: case 'l': if (!strcmp(optarg, "all")) { log_hv = 1; log_guest = 1; } else if (!strcmp(optarg, "hv")) { log_hv = 1; } else if (!strcmp(optarg, "guest")) { log_guest = 1; } in main.c
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c index 7e6a886..9bb14de 100644 --- a/tools/console/daemon/io.c +++ b/tools/console/daemon/io.c @@ -89,29 +89,139 @@ struct buffer { size_t max_capacity; }; -struct domain { - int domid; +struct console { + char *xsname; + char *ttyname; int master_fd; int master_pollfd_idx; int slave_fd; int log_fd; - bool is_dead; - unsigned last_seen; struct buffer buffer; - struct domain *next; - char *conspath; int ring_ref; xenevtchn_port_or_error_t local_port; xenevtchn_port_or_error_t remote_port; + struct xencons_interface *interface; + struct domain *d; /* Reference to the domain it is contained in. */ + char *xspath; + int (*map_ring_ref)(struct console *, int); + bool mandatory; +}; + +struct console_data { + char *xsname; + char *ttyname; + int (*mapfunc)(struct console *, int); + bool mandatory; +}; + +static int map_pvcon_ring_ref(struct console *, int ); + +static struct console_data console_data[] = { + + { + .xsname = "/console", + .ttyname = "tty", + .mapfunc = map_pvcon_ring_ref, + .mandatory = true + }, +}; + +#define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data)) + +struct domain { + int domid; + bool is_dead; + unsigned last_seen; + struct domain *next; xenevtchn_handle *xce_handle; int xce_pollfd_idx; - struct xencons_interface *interface; int event_count; long long next_period; + struct console console[MAX_CONSOLE]; }; static struct domain *dom_head; +typedef void (*VOID_ITER_FUNC_ARG1)(struct console *); +typedef bool (*BOOL_ITER_FUNC_ARG1)(struct console *); +typedef int (*INT_ITER_FUNC_ARG1)(struct console *); +typedef void (*VOID_ITER_FUNC_ARG2)(struct console *, unsigned int); +typedef int (*INT_ITER_FUNC_ARG3)(struct console *, + struct domain *dom, void **); + +static inline bool console_enabled(struct console *con) +{ + return con->local_port != -1; +} + +static inline void console_iter_void_arg1(struct domain *d, + VOID_ITER_FUNC_ARG1 iter_func) +{ + int i = 0; + struct console *con = &(d->console[0]); + + for (i = 0; i < MAX_CONSOLE; i++, con++) + { + iter_func(con); + } +} + +static inline void console_iter_void_arg2(struct domain *d, + VOID_ITER_FUNC_ARG2 iter_func, + unsigned int iter_data) +{ + int i = 0; + struct console *con = &(d->console[0]); + + for (i = 0; i < MAX_CONSOLE; i++, con++) + { + iter_func(con, iter_data); + } +} + +static inline bool console_iter_bool_arg1(struct domain *d, + BOOL_ITER_FUNC_ARG1 iter_func) +{ + int i = 0; + struct console *con = &(d->console[0]); + + for (i = 0; i < MAX_CONSOLE; i++, con++) + { + if (iter_func(con)) + return true; + } + return false; +} + +static inline int console_iter_int_arg1(struct domain *d, + INT_ITER_FUNC_ARG1 iter_func) +{ + int i = 0; + struct console *con = &(d->console[0]); + + for (i = 0; i < MAX_CONSOLE; i++, con++) + { + if (iter_func(con)) + return 1; + } + return 0; +} + +static inline int console_iter_int_arg3(struct domain *d, + INT_ITER_FUNC_ARG3 iter_func, + void *iter_data) +{ + int i = 0; + struct console *con = &(d->console[0]); + + for (i = 0; i < MAX_CONSOLE; i++, con++) + { + if (iter_func(con, d, iter_data)) + return 1; + } + return 0; +} + static int write_all(int fd, const char* buf, size_t len) { while (len) { @@ -158,11 +268,29 @@ static int write_with_timestamp(int fd, const char *data, size_t sz, return 0; } -static void buffer_append(struct domain *dom) +static inline bool buffer_available(struct console *con) { - struct buffer *buffer = &dom->buffer; + if (discard_overflowed_data || + !con->buffer.max_capacity || + con->buffer.size < con->buffer.max_capacity) + return true; + else + return false; +} + +static void buffer_append(struct console *con, unsigned int data) +{ + struct buffer *buffer = &con->buffer; + struct xencons_interface *intf = con->interface; + xenevtchn_port_or_error_t port = con->local_port; + struct domain *dom = con->d; + xenevtchn_port_or_error_t rxport = (xenevtchn_port_or_error_t)data; + XENCONS_RING_IDX cons, prod, size; - struct xencons_interface *intf = dom->interface; + + /* If incoming data is not for the current console then ignore. */ + if (port != rxport) + return; cons = intf->out_cons; prod = intf->out_prod; @@ -187,22 +315,22 @@ static void buffer_append(struct domain *dom) xen_mb(); intf->out_cons = cons; - xenevtchn_notify(dom->xce_handle, dom->local_port); + xenevtchn_notify(dom->xce_handle, port); /* Get the data to the logfile as early as possible because if * no one is listening on the console pty then it will fill up * and handle_tty_write will stop being called. */ - if (dom->log_fd != -1) { + if (con->log_fd != -1) { int logret; if (log_time_guest) { logret = write_with_timestamp( - dom->log_fd, + con->log_fd, buffer->data + buffer->size - size, size, &log_time_guest_needts); } else { logret = write_all( - dom->log_fd, + con->log_fd, buffer->data + buffer->size - size, size); } @@ -290,12 +418,13 @@ static int create_hv_log(void) return fd; } -static int create_domain_log(struct domain *dom) +static int create_console_log(struct console *con) { char logfile[PATH_MAX]; char *namepath, *data, *s; int fd; unsigned int len; + struct domain *dom = con->d; namepath = xs_get_domain_path(xs, dom->domid); s = realloc(namepath, strlen(namepath) + 6); @@ -314,7 +443,9 @@ static int create_domain_log(struct domain *dom) return -1; } - snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data); + snprintf(logfile, PATH_MAX-1, "%s%s/guest-%s.log", + log_dir, con->xspath, data); + free(data); logfile[PATH_MAX-1] = '\0'; @@ -336,19 +467,24 @@ static int create_domain_log(struct domain *dom) return fd; } -static void domain_close_tty(struct domain *dom) +static void console_close_tty(struct console *con) { - if (dom->master_fd != -1) { - close(dom->master_fd); - dom->master_fd = -1; + if (con->master_fd != -1) { + close(con->master_fd); + con->master_fd = -1; } - if (dom->slave_fd != -1) { - close(dom->slave_fd); - dom->slave_fd = -1; + if (con->slave_fd != -1) { + close(con->slave_fd); + con->slave_fd = -1; } } +static void domain_close_tty(struct domain *dom) +{ + console_iter_void_arg1(dom, console_close_tty); +} + #ifdef __sun__ static int openpty(int *amaster, int *aslave, char *name, struct termios *termp, struct winsize *winp) @@ -409,7 +545,7 @@ void cfmakeraw(struct termios *termios_p) } #endif /* __sun__ */ -static int domain_create_tty(struct domain *dom) +static int console_create_tty(struct console *con) { const char *slave; char *path; @@ -418,19 +554,23 @@ static int domain_create_tty(struct domain *dom) char *data; unsigned int len; struct termios term; + struct domain *dom = con->d; + + if (!console_enabled(con)) + return 0; - assert(dom->slave_fd == -1); - assert(dom->master_fd == -1); + assert(con->master_fd == -1); + assert(con->slave_fd == -1); - if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) { + if (openpty(&con->master_fd, &con->slave_fd, NULL, NULL, NULL) < 0) { err = errno; dolog(LOG_ERR, "Failed to create tty for domain-%d " - "(errno = %i, %s)", - dom->domid, err, strerror(err)); - return 0; + "(errno = %i, %s)", + dom->domid, err, strerror(err)); + goto out; } - if (tcgetattr(dom->slave_fd, &term) < 0) { + if (tcgetattr(con->slave_fd, &term) < 0) { err = errno; dolog(LOG_ERR, "Failed to get tty attributes for domain-%d " "(errno = %i, %s)", @@ -438,7 +578,7 @@ static int domain_create_tty(struct domain *dom) goto out; } cfmakeraw(&term); - if (tcsetattr(dom->slave_fd, TCSANOW, &term) < 0) { + if (tcsetattr(con->slave_fd, TCSANOW, &term) < 0) { err = errno; dolog(LOG_ERR, "Failed to set tty attributes for domain-%d " "(errno = %i, %s)", @@ -446,40 +586,54 @@ static int domain_create_tty(struct domain *dom) goto out; } - if ((slave = ptsname(dom->master_fd)) == NULL) { + if ((slave = ptsname(con->master_fd)) == NULL) { err = errno; dolog(LOG_ERR, "Failed to get slave name for domain-%d " - "(errno = %i, %s)", - dom->domid, err, strerror(err)); + "(errno = %i, %s)", + dom->domid, err, strerror(err)); goto out; } - success = asprintf(&path, "%s/limit", dom->conspath) != + success = asprintf(&path, "%s/limit", con->xspath) != -1; if (!success) goto out; data = xs_read(xs, XBT_NULL, path, &len); if (data) { - dom->buffer.max_capacity = strtoul(data, 0, 0); + con->buffer.max_capacity = strtoul(data, 0, 0); free(data); } free(path); - success = (asprintf(&path, "%s/tty", dom->conspath) != -1); + success = (asprintf(&path, "%s/%s", con->xspath, con->ttyname) != -1); + if (!success) goto out; success = xs_write(xs, XBT_NULL, path, slave, strlen(slave)); free(path); - if (!success) + + if (fcntl(con->master_fd, F_SETFL, O_NONBLOCK) == -1) goto out; - if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1) + if (!success) goto out; - return 1; -out: - domain_close_tty(dom); return 0; + +out: + return 1; +} + +static int domain_create_tty(struct domain *dom) +{ + int ret; + + ret = console_iter_int_arg1(dom, console_create_tty); + + if (ret) + domain_close_tty(dom); + + return ret; } /* Takes tuples of names, scanf-style args, and void **, NULL terminated. */ @@ -517,31 +671,106 @@ static int xs_gather(struct xs_handle *xs, const char *dir, ...) return ret; } -static void domain_unmap_interface(struct domain *dom) +static void console_unmap_interface(struct console *con) { - if (dom->interface == NULL) + if (con->interface == NULL) return; - if (xgt_handle && dom->ring_ref == -1) - xengnttab_unmap(xgt_handle, dom->interface, 1); + if (xgt_handle && con->ring_ref == -1) + xengnttab_unmap(xgt_handle, con->interface, 1); else - munmap(dom->interface, XC_PAGE_SIZE); - dom->interface = NULL; - dom->ring_ref = -1; + munmap(con->interface, XC_PAGE_SIZE); + con->interface = NULL; + con->ring_ref = -1; +} + +static void domain_unmap_interface(struct domain *dom) +{ + console_iter_void_arg1(dom, console_unmap_interface); +} + +static int bind_event_channel(struct domain *dom, + int new_rport, + int *lport, + int *rport) +{ + int err = 0, rc; + + /* Go no further if port has not changed and we are still bound. */ + if (new_rport == *rport) { + xc_evtchn_status_t status = { + .dom = DOMID_SELF, + .port = *lport }; + if ((xc_evtchn_status(xc, &status) == 0) && + (status.status == EVTCHNSTAT_interdomain)) + goto out; + } + + *lport = -1; + *rport = -1; + rc = xenevtchn_bind_interdomain(dom->xce_handle, + dom->domid, new_rport); + + if (rc == -1) { + err = errno; + goto out; + } + + *lport = rc; + *rport = new_rport; +out: + return err; } -static int domain_create_ring(struct domain *dom) +static int map_pvcon_ring_ref(struct console *con, int ring_ref) { - int err, remote_port, ring_ref, rc; + int err = 0; + struct domain *dom = con->d; + + if (!con->interface && xgt_handle) { + /* Prefer using grant table */ + con->interface = xengnttab_map_grant_ref(xgt_handle, + dom->domid, + GNTTAB_RESERVED_CONSOLE, + PROT_READ|PROT_WRITE); + con->ring_ref = -1; + } + + if (!con->interface) { + con->interface = xc_map_foreign_range(xc, + dom->domid, + XC_PAGE_SIZE, + PROT_READ|PROT_WRITE, + (unsigned long)ring_ref); + if (con->interface == NULL) { + err = EINVAL; + goto out; + } + con->ring_ref = ring_ref; + } + +out: + return err; +} + +static int console_create_ring(struct console *con) +{ + int err, remote_port, ring_ref; char *type, path[PATH_MAX]; + struct domain *dom = con->d; + + err = xs_gather(xs, con->xspath, + "ring-ref", "%u", &ring_ref, + "port", "%i", &remote_port, + NULL); - err = xs_gather(xs, dom->conspath, - "ring-ref", "%u", &ring_ref, - "port", "%i", &remote_port, - NULL); if (err) + { + /* If the console is not mandatory then do not return an error. */ + if (!con->mandatory) + err = 0; goto out; + } - snprintf(path, sizeof(path), "%s/type", dom->conspath); + snprintf(path, sizeof(path), "%s/type", con->xspath); type = xs_read(xs, XBT_NULL, path, NULL); if (type && strcmp(type, "xenconsoled") != 0) { free(type); @@ -550,41 +779,44 @@ static int domain_create_ring(struct domain *dom) free(type); /* If using ring_ref and it has changed, remap */ - if (ring_ref != dom->ring_ref && dom->ring_ref != -1) - domain_unmap_interface(dom); + if (ring_ref != con->ring_ref && con->ring_ref != -1) + console_unmap_interface(con); - if (!dom->interface && xgt_handle) { - /* Prefer using grant table */ - dom->interface = xengnttab_map_grant_ref(xgt_handle, - dom->domid, GNTTAB_RESERVED_CONSOLE, - PROT_READ|PROT_WRITE); - dom->ring_ref = -1; - } - if (!dom->interface) { - /* Fall back to xc_map_foreign_range */ - dom->interface = xc_map_foreign_range( - xc, dom->domid, XC_PAGE_SIZE, - PROT_READ|PROT_WRITE, - (unsigned long)ring_ref); - if (dom->interface == NULL) { - err = EINVAL; - goto out; + err = con->map_ring_ref(con, ring_ref); + + if (err) + goto out; + + err = bind_event_channel(dom, remote_port, + &con->local_port, + &con->remote_port); + if (err) + goto out1; + + if (con->master_fd == -1) { + if (console_create_tty(con)) { + err = errno; + con->local_port = -1; + con->remote_port = -1; + goto out1; } - dom->ring_ref = ring_ref; } - /* Go no further if port has not changed and we are still bound. */ - if (remote_port == dom->remote_port) { - xc_evtchn_status_t status = { - .dom = DOMID_SELF, - .port = dom->local_port }; - if ((xc_evtchn_status(xc, &status) == 0) && - (status.status == EVTCHNSTAT_interdomain)) - goto out; - } + if (log_guest && (con->log_fd == -1)) + con->log_fd = create_console_log(con); + + return err; + + out1: + console_unmap_interface(con); + out: + return err; +} + +static int domain_create_ring(struct domain *dom) +{ + int err; - dom->local_port = -1; - dom->remote_port = -1; if (dom->xce_handle != NULL) xenevtchn_close(dom->xce_handle); @@ -592,37 +824,17 @@ static int domain_create_ring(struct domain *dom) * wasteful, but that's how the code is structured... */ dom->xce_handle = xenevtchn_open(NULL, 0); if (dom->xce_handle == NULL) { - err = errno; - goto out; + return errno; } - - rc = xenevtchn_bind_interdomain(dom->xce_handle, - dom->domid, remote_port); - if (rc == -1) { - err = errno; + err = console_iter_int_arg1(dom, console_create_ring); + + if (err) + { xenevtchn_close(dom->xce_handle); dom->xce_handle = NULL; - goto out; - } - dom->local_port = rc; - dom->remote_port = remote_port; - - if (dom->master_fd == -1) { - if (!domain_create_tty(dom)) { - err = errno; - xenevtchn_close(dom->xce_handle); - dom->xce_handle = NULL; - dom->local_port = -1; - dom->remote_port = -1; - goto out; - } } - if (log_guest && (dom->log_fd == -1)) - dom->log_fd = create_domain_log(dom); - - out: return err; } @@ -630,27 +842,66 @@ static bool watch_domain(struct domain *dom, bool watch) { char domid_str[3 + MAX_STRLEN(dom->domid)]; bool success; + char *path = dom->console[0].xspath; snprintf(domid_str, sizeof(domid_str), "dom%u", dom->domid); if (watch) { - success = xs_watch(xs, dom->conspath, domid_str); + success = xs_watch(xs, path, domid_str); if (success) domain_create_ring(dom); else - xs_unwatch(xs, dom->conspath, domid_str); + xs_unwatch(xs, path, domid_str); } else { - success = xs_unwatch(xs, dom->conspath, domid_str); + success = xs_unwatch(xs, path, domid_str); } return success; } +static int console_init(struct console *con, struct domain *dom, void **data) +{ + char *s; + int err = -1; + struct console_data **con_data = (struct console_data **)data; + + con->master_fd = -1; + con->master_pollfd_idx = -1; + con->slave_fd = -1; + con->log_fd = -1; + con->ring_ref = -1; + con->local_port = -1; + con->remote_port = -1; + con->d = dom; + con->ttyname = (*con_data)->ttyname; + con->xsname = (*con_data)->xsname; + con->map_ring_ref = (*con_data)->mapfunc; + con->mandatory = (*con_data)->mandatory; + con->xspath = xs_get_domain_path(xs, dom->domid); + s = realloc(con->xspath, strlen(con->xspath) + + strlen(con->xsname) + 1); + + (*con_data)++; + + if (s) + { + con->xspath = s; + strcat(con->xspath, con->xsname); + err = 0; + } + return err; +} + +static void console_free(struct console *con) +{ + if (con->xspath) + free(con->xspath); +} static struct domain *create_domain(int domid) { struct domain *dom; - char *s; struct timespec ts; + struct console_data *con_data = &console_data[0]; if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d", @@ -667,26 +918,13 @@ static struct domain *create_domain(int domid) dom->domid = domid; - dom->conspath = xs_get_domain_path(xs, dom->domid); - s = realloc(dom->conspath, strlen(dom->conspath) + - strlen("/console") + 1); - if (s == NULL) + if (console_iter_int_arg3(dom, console_init, (void **)&con_data)) goto out; - dom->conspath = s; - strcat(dom->conspath, "/console"); - dom->master_fd = -1; - dom->master_pollfd_idx = -1; - dom->slave_fd = -1; - dom->log_fd = -1; dom->xce_pollfd_idx = -1; dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD; - dom->ring_ref = -1; - dom->local_port = -1; - dom->remote_port = -1; - if (!watch_domain(dom, true)) goto out; @@ -696,8 +934,10 @@ static struct domain *create_domain(int domid) dolog(LOG_DEBUG, "New domain %d", domid); return dom; + out: - free(dom->conspath); + console_iter_void_arg1(dom, console_free); + free(dom); return NULL; } @@ -727,20 +967,24 @@ static void remove_domain(struct domain *dom) } } -static void cleanup_domain(struct domain *d) -{ - domain_close_tty(d); - if (d->log_fd != -1) { - close(d->log_fd); - d->log_fd = -1; +static void console_cleanup(struct console *con) +{ + if (con->log_fd != -1) { + close(con->log_fd); + con->log_fd = -1; } + free(con->buffer.data); + con->buffer.data = NULL; + free(con->xspath); + con->xspath = NULL; +} - free(d->buffer.data); - d->buffer.data = NULL; +static void cleanup_domain(struct domain *d) +{ + domain_close_tty(d); - free(d->conspath); - d->conspath = NULL; + console_iter_void_arg1(d, console_cleanup); remove_domain(d); } @@ -780,9 +1024,9 @@ static void enum_domains(void) } } -static int ring_free_bytes(struct domain *dom) +static int ring_free_bytes(struct console *con) { - struct xencons_interface *intf = dom->interface; + struct xencons_interface *intf = con->interface; XENCONS_RING_IDX cons, prod, space; cons = intf->in_cons; @@ -807,25 +1051,27 @@ static void domain_handle_broken_tty(struct domain *dom, int recreate) } } -static void handle_tty_read(struct domain *dom) +static void handle_tty_read(struct console *con) { ssize_t len = 0; char msg[80]; int i; - struct xencons_interface *intf = dom->interface; XENCONS_RING_IDX prod; + struct xencons_interface *intf = con->interface; + xenevtchn_port_or_error_t port = con->local_port; + struct domain *dom = con->d; if (dom->is_dead) return; - len = ring_free_bytes(dom); + len = ring_free_bytes(con); if (len == 0) return; if (len > sizeof(msg)) len = sizeof(msg); - len = read(dom->master_fd, msg, len); + len = read(con->master_fd, msg, len); /* * Note: on Solaris, len == 0 means the slave closed, and this * is no problem, but Linux can't handle this usefully, so we @@ -841,31 +1087,39 @@ static void handle_tty_read(struct domain *dom) } xen_wmb(); intf->in_prod = prod; - xenevtchn_notify(dom->xce_handle, dom->local_port); + xenevtchn_notify(dom->xce_handle, port); } else { domain_close_tty(dom); shutdown_domain(dom); } } -static void handle_tty_write(struct domain *dom) +static void handle_tty_write(struct console *con) { ssize_t len; + struct domain *dom = con->d; if (dom->is_dead) return; - len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed, - dom->buffer.size - dom->buffer.consumed); + len = write(con->master_fd, + con->buffer.data + con->buffer.consumed, + con->buffer.size - con->buffer.consumed); if (len < 1) { dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n", dom->domid, len, errno); domain_handle_broken_tty(dom, domain_is_valid(dom->domid)); } else { - buffer_advance(&dom->buffer, len); + buffer_advance(&con->buffer, len); } } +static void console_event_unmask(struct console *con) +{ + if (con->local_port != -1) + (void)xenevtchn_unmask(con->d->xce_handle, con->local_port); +} + static void handle_ring_read(struct domain *dom) { xenevtchn_port_or_error_t port; @@ -878,10 +1132,10 @@ static void handle_ring_read(struct domain *dom) dom->event_count++; - buffer_append(dom); + console_iter_void_arg2(dom, buffer_append, port); if (dom->event_count < RATE_LIMIT_ALLOWANCE) - (void)xenevtchn_unmask(dom->xce_handle, port); + console_iter_void_arg1(dom, console_event_unmask); } static void handle_xs(void) @@ -943,14 +1197,22 @@ static void handle_hv_logs(xenevtchn_handle *xce_handle, bool force) (void)xenevtchn_unmask(xce_handle, port); } +static void console_open_log(struct console *con) +{ + if (console_enabled(con)) + { + if (con->log_fd != -1) + close(con->log_fd); + con->log_fd = create_console_log(con); + } +} + static void handle_log_reload(void) { if (log_guest) { struct domain *d; for (d = dom_head; d; d = d->next) { - if (d->log_fd != -1) - close(d->log_fd); - d->log_fd = create_domain_log(d); + console_iter_void_arg1(d, console_open_log); } } @@ -1002,6 +1264,40 @@ static void reset_fds(void) memset(fds, 0, sizeof(struct pollfd) * current_array_size); } +static void add_console_fd(struct console *con) +{ + if (con->master_fd != -1) { + short events = 0; + if (!con->d->is_dead && ring_free_bytes(con)) + events |= POLLIN; + + if (!buffer_empty(&con->buffer)) + events |= POLLOUT; + + if (events) + con->master_pollfd_idx = + set_fds(con->master_fd, events|POLLPRI); + } +} + +static void process_console(struct console *con) +{ + if (con->master_fd != -1 && con->master_pollfd_idx != -1) { + if (fds[con->master_pollfd_idx].revents & + ~(POLLIN|POLLOUT|POLLPRI)) + domain_handle_broken_tty(con->d, domain_is_valid(con->d->domid)); + else { + if (fds[con->master_pollfd_idx].revents & + POLLIN) + handle_tty_read(con); + if (fds[con->master_pollfd_idx].revents & + POLLOUT) + handle_tty_write(con); + } + } + con->master_pollfd_idx = -1; +} + void handle_io(void) { int ret; @@ -1068,7 +1364,7 @@ void handle_io(void) if ((now+5) > d->next_period) { d->next_period = now + RATE_LIMIT_PERIOD; if (d->event_count >= RATE_LIMIT_ALLOWANCE) { - (void)xenevtchn_unmask(d->xce_handle, d->local_port); + console_iter_void_arg1(d, console_event_unmask); } d->event_count = 0; } @@ -1081,28 +1377,15 @@ void handle_io(void) d->next_period < next_timeout) next_timeout = d->next_period; } else if (d->xce_handle != NULL) { - if (discard_overflowed_data || - !d->buffer.max_capacity || - d->buffer.size < d->buffer.max_capacity) { + if (console_iter_bool_arg1(d, buffer_available)) + { int evtchn_fd = xenevtchn_fd(d->xce_handle); d->xce_pollfd_idx = set_fds(evtchn_fd, - POLLIN|POLLPRI); + POLLIN|POLLPRI); } } - if (d->master_fd != -1) { - short events = 0; - if (!d->is_dead && ring_free_bytes(d)) - events |= POLLIN; - - if (!buffer_empty(&d->buffer)) - events |= POLLOUT; - - if (events) - d->master_pollfd_idx = - set_fds(d->master_fd, - events|POLLPRI); - } + console_iter_void_arg1(d, add_console_fd); } /* If any domain has been rate limited, we need to work @@ -1170,22 +1453,9 @@ void handle_io(void) handle_ring_read(d); } - if (d->master_fd != -1 && d->master_pollfd_idx != -1) { - if (fds[d->master_pollfd_idx].revents & - ~(POLLIN|POLLOUT|POLLPRI)) - domain_handle_broken_tty(d, - domain_is_valid(d->domid)); - else { - if (fds[d->master_pollfd_idx].revents & - POLLIN) - handle_tty_read(d); - if (fds[d->master_pollfd_idx].revents & - POLLOUT) - handle_tty_write(d); - } - } + console_iter_void_arg1(d, process_console); - d->xce_pollfd_idx = d->master_pollfd_idx = -1; + d->xce_pollfd_idx = -1; if (d->last_seen != enum_pass) shutdown_domain(d);
Xenconsole supports only PV console currently. This patch adds support for supporting multiple consoles. This patch modifies different data structures and APIs used in xenconsole to support multiple consoles. Change summary: 1. Split the domain structure into a console structure and the domain structure, where each console structure represents one console. 2. Modify different APIs such as buffer_append() etc. to take console structure as input and perform per console specific operations. 3. Define a generic console_create_ring(), which sets up the ring buffer and event channel for each console. 3. Modify domain_create_ring() to use console_create_ring(). 4. Modifications in handle_ring_read() to read ring buffer data from multiple consoles. 5. Add log file support for multiple consoles. Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- Changes since v2: - Defined a new function console_create_ring() which sets up the ring buffer and event channel a new console. domain_create_ring() uses this function to setup a console. - This patch does not contain vuart specific changes, which would be introduced in the next patch. - Changes for keeping the PV log file name unchanged. Changes since v1: - Split the domain struture to a separate console structure - Modified the functions to operate on the console struture - Replaced repetitive per console code with generic code tools/console/daemon/io.c | 650 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 460 insertions(+), 190 deletions(-)