[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 08/10] xen/arm: vpl011: Modify the APIs in xenconsole to acces both PV and VCON consoles



On Mon, Apr 03, 2017 at 03:14:31PM +0530, Bhupinder Thakur wrote:
> Xenconsole supports only PV console currently. To get access to emulated pl011
> uart another backend console is required.
> 
> This patch modifies different data structures and APIs used
> in xenconsole to support two console types: PV and VCON (virtual console).
> 
> Change summary:
> 
> 1. Modify the domain structure to support two console types: PV and a
>    virtual console (VCON).
> 
> 2. Modify different APIs such as buffer_append() to take a new parameter
>    console_type as input and operate on the data structures indexed by
>    the console_type.
> 
> 3. Modfications in domain_create_ring():
>     - Bind to the vpl011 event channel obtained from the xen store as a
>       new
>       parameter
>     - Map the PFN to its address space to be used as IN/OUT ring
>       buffers.
>       It obtains the PFN from the xen store as a new parameter
> 
> 4. Modifications in handle_ring_read() to handle both PV and VCON
>    events.
> 
> 5. Add a new log_file for VCON console logs.
> 

It seems that this patch and previous one should be merged into one.

There are a lot of coding style issues, please fix all of them.

The code looks rather repetitive unfortunately. I believe a lot of the
repetitiveness can be avoided by using loops and pointers.

> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
>  tools/console/daemon/io.c | 508 
> ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 356 insertions(+), 152 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 0cd1fee..b9be5a5 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -164,11 +164,39 @@ static int write_with_timestamp(int fd, const char 
> *data, size_t sz,
>       return 0;
>  }
>  
> -static void buffer_append(struct domain *dom)
> +/*
> + * This function checks if the data is allowed to be buffered for that 
> console.
> + * If not then it marks it pending for later receiving.
> + */
> +static bool buffer_available(struct domain *dom, int console_type)
> +{
> +     if (discard_overflowed_data ||
> +             !dom->buffer[console_type].max_capacity ||
> +             dom->buffer[console_type].size < 
> dom->buffer[console_type].max_capacity)

Line too long.

> +     {
> +             dom->console_data_pending &= ~(1<<console_type);
> +             return true;
> +     }
> +     else
> +     {
> +             dom->console_data_pending |= (1<<console_type);
> +             return false;
> +     }
> +}
> +
> +static void buffer_append(struct domain *dom, int console_type)
>  {
> -     struct buffer *buffer = &dom->buffer;
> +     struct buffer *buffer = &dom->buffer[console_type];
> +     struct xencons_interface *intf=dom->interface[console_type];
> +     xenevtchn_port_or_error_t port=dom->local_port[console_type];

Spaces around "=".

> +
>       XENCONS_RING_IDX cons, prod, size;
> -     struct xencons_interface *intf = dom->interface;
> +
> +     /*
> +      * check if more data is allowed to be buffered
> +      */
> +     if (!buffer_available(dom, console_type))
> +             return;
>  
>       cons = intf->out_cons;
>       prod = intf->out_prod;
> @@ -193,22 +221,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 (dom->log_fd[console_type] != -1) {
>               int logret;
>               if (log_time_guest) {
>                       logret = write_with_timestamp(
> -                             dom->log_fd,
> +                             dom->log_fd[console_type],
>                               buffer->data + buffer->size - size,
>                               size, &log_time_guest_needts);
>               } else {
>                       logret = write_all(
> -                             dom->log_fd,
> +                             dom->log_fd[console_type],
>                               buffer->data + buffer->size - size,
>                               size);
>               }
> @@ -296,12 +324,13 @@ static int create_hv_log(void)
>       return fd;
>  }
>  
> -static int create_domain_log(struct domain *dom)
> +static int create_domain_log(struct domain *dom, int console_type)
>  {
>       char logfile[PATH_MAX];
>       char *namepath, *data, *s;
>       int fd;
>       unsigned int len;
> +     char *console_name[]={"pv", "vcon"};
>  
>       namepath = xs_get_domain_path(xs, dom->domid);
>       s = realloc(namepath, strlen(namepath) + 6);
> @@ -320,7 +349,7 @@ 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/guest-%s-%s.log", log_dir, 
> console_name[console_type], data);

Line too long.

>       free(data);
>       logfile[PATH_MAX-1] = '\0';
>  
> @@ -344,14 +373,24 @@ static int create_domain_log(struct domain *dom)
>  
>  static void domain_close_tty(struct domain *dom)
>  {
> -     if (dom->master_fd != -1) {
> -             close(dom->master_fd);
> -             dom->master_fd = -1;
> +     if (dom->master_fd[CONSOLE_TYPE_PV] != -1) {
> +             close(dom->master_fd[CONSOLE_TYPE_PV]);
> +             dom->master_fd[CONSOLE_TYPE_PV] = -1;
> +     }
> +
> +     if (dom->slave_fd[CONSOLE_TYPE_PV] != -1) {
> +             close(dom->slave_fd[CONSOLE_TYPE_PV]);
> +             dom->slave_fd[CONSOLE_TYPE_PV] = -1;
> +     }
> +
> +     if (dom->master_fd[CONSOLE_TYPE_VCON] != -1) {
> +             close(dom->master_fd[CONSOLE_TYPE_VCON]);
> +             dom->master_fd[CONSOLE_TYPE_VCON] = -1;
>       }
>  
> -     if (dom->slave_fd != -1) {
> -             close(dom->slave_fd);
> -             dom->slave_fd = -1;
> +     if (dom->slave_fd[CONSOLE_TYPE_VCON] != -1) {
> +             close(dom->slave_fd[CONSOLE_TYPE_VCON]);
> +             dom->slave_fd[CONSOLE_TYPE_VCON] = -1;
>       }

You can use two loops to avoid repetitive snippets. But I'm fine with
the code as-is, too.

>  }
>  
> @@ -424,63 +463,86 @@ static int domain_create_tty(struct domain *dom)
>       char *data;
>       unsigned int len;
>       struct termios term;
> +     int console_type=0;
>  
> -     assert(dom->slave_fd == -1);
> -     assert(dom->master_fd == -1);
> +     assert(dom->master_fd[CONSOLE_TYPE_PV] == -1);
> +     assert(dom->slave_fd[CONSOLE_TYPE_PV] == -1);
> +     assert(dom->master_fd[CONSOLE_TYPE_VCON] == -1);
> +     assert(dom->slave_fd[CONSOLE_TYPE_VCON] == -1);
>  
> -     if (openpty(&dom->master_fd, &dom->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;
> -     }
> +     /*
> +      * Open pty for both PV and vcon consoles.
> +      */
> +     for (console_type=0; console_type<MAX_CONSOLE; console_type++)

Spaces around "=" and "<".

> +     {
> +             if (openpty(&dom->master_fd[console_type], 
> &dom->slave_fd[console_type], NULL, NULL, NULL) < 0) {

Line too long

> +                     err = errno;
> +                     dolog(LOG_ERR, "Failed to create tty for domain-%d "
> +                               "(errno = %i, %s)",
> +                               dom->domid, err, strerror(err));
> +                     return 0;
> +             }
>  
> -     if (tcgetattr(dom->slave_fd, &term) < 0) {
> -             err = errno;
> -             dolog(LOG_ERR, "Failed to get tty attributes for domain-%d "
> -                     "(errno = %i, %s)",
> -                     dom->domid, err, strerror(err));
> -             goto out;
> -     }
> -     cfmakeraw(&term);
> -     if (tcsetattr(dom->slave_fd, TCSANOW, &term) < 0) {
> -             err = errno;
> -             dolog(LOG_ERR, "Failed to set tty attributes for domain-%d "
> -                     "(errno = %i, %s)",
> -                     dom->domid, err, strerror(err));
> -             goto out;
> -     }
> +             if (tcgetattr(dom->slave_fd[console_type], &term) < 0) {
> +                     err = errno;
> +                     dolog(LOG_ERR, "Failed to get tty attributes for 
> domain-%d "
> +                             "(errno = %i, %s)",
> +                             dom->domid, err, strerror(err));
> +                     goto out;
> +             }
> +             cfmakeraw(&term);
> +             if (tcsetattr(dom->slave_fd[console_type], TCSANOW, &term) < 0) 
> {
> +                     err = errno;
> +                     dolog(LOG_ERR, "Failed to set tty attributes for 
> domain-%d "
> +                             "(errno = %i, %s)",
> +                             dom->domid, err, strerror(err));
> +                     goto out;
> +             }
>  
> -     if ((slave = ptsname(dom->master_fd)) == NULL) {
> -             err = errno;
> -             dolog(LOG_ERR, "Failed to get slave name for domain-%d "
> -                   "(errno = %i, %s)",
> -                   dom->domid, err, strerror(err));
> -             goto out;
> -     }
> +             if ((slave = ptsname(dom->master_fd[console_type])) == NULL) {
> +                     err = errno;
> +                     dolog(LOG_ERR, "Failed to get slave name for domain-%d "
> +                               "(errno = %i, %s)",
> +                               dom->domid, err, strerror(err));
> +                     goto out;
> +             }
>  
> -     success = asprintf(&path, "%s/limit", dom->conspath) !=
> -             -1;
> -     if (!success)
> -             goto out;
> -     data = xs_read(xs, XBT_NULL, path, &len);
> -     if (data) {
> -             dom->buffer.max_capacity = strtoul(data, 0, 0);
> -             free(data);
> +             /*
> +              * Initialize the console buffer capacity.
> +              */
> +             success = asprintf(&path, "%s/limit", dom->conspath) !=
> +                     -1;
> +             if (!success)
> +                     goto out;
> +             data = xs_read(xs, XBT_NULL, path, &len);
> +             if (data) {
> +                     dom->buffer[console_type].max_capacity = strtoul(data, 
> 0, 0);
> +                     free(data);
> +             }
> +             free(path);
> +
> +             /*
> +              * Write console slave name to xen store.
> +              */
> +             if (console_type == CONSOLE_TYPE_VCON)
> +                     success = (asprintf(&path, "%s/vtty", dom->conspath) != 
> -1);
> +             else
> +                     success = (asprintf(&path, "%s/tty", dom->conspath) != 
> -1);
> +
> +             if (!success)
> +                     goto out;
> +             success = xs_write(xs, XBT_NULL, path, slave, strlen(slave));
> +             free(path);
> +
> +             if (fcntl(dom->master_fd[console_type], F_SETFL, O_NONBLOCK) == 
> -1)
> +                     goto out;
> +
> +             if (!dom->vcon_enabled)
> +                     break;
>       }
> -     free(path);
>  
> -     success = (asprintf(&path, "%s/tty", dom->conspath) != -1);
>       if (!success)
>               goto out;
> -     success = xs_write(xs, XBT_NULL, path, slave, strlen(slave));
> -     free(path);
> -     if (!success)
> -             goto out;
> -
> -     if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1)
> -             goto out;
>  
>       return 1;
>  out:
> @@ -523,21 +585,53 @@ static int xs_gather(struct xs_handle *xs, const char 
> *dir, ...)
>       return ret;
>  }
>  
> -static void domain_unmap_interface(struct domain *dom)
> +static void domain_unmap_interface(struct domain *dom, int console_type)
>  {
> -     if (dom->interface == NULL)
> +     if (dom->interface[console_type] == NULL)
>               return;
> -     if (xgt_handle && dom->ring_ref == -1)
> -             xengnttab_unmap(xgt_handle, dom->interface, 1);
> +     if (xgt_handle && dom->ring_ref[console_type] == -1)
> +             xengnttab_unmap(xgt_handle, dom->interface[console_type], 1);
>       else
> -             munmap(dom->interface, XC_PAGE_SIZE);
> -     dom->interface = NULL;
> -     dom->ring_ref = -1;
> +             munmap(dom->interface[console_type], XC_PAGE_SIZE);
> +     dom->interface[console_type] = NULL;
> +     dom->ring_ref[console_type] = -1;
> +}
> +
> +static int bind_event_channel(struct domain *dom, int new_rport, int *lport, 
> int *rport)

Line too long.

> +{
> +     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);

Indentation.

> +
> +     if (rc == -1) {
> +             err = errno;
> +             xenevtchn_close(dom->xce_handle);
> +             dom->xce_handle = NULL;
> +             goto out;
> +     }
> +
> +     *lport = rc;
> +     *rport = new_rport;
> +out:
> +     return err;
>  }
>   
>  static int domain_create_ring(struct domain *dom)
>  {
> -     int err, remote_port, ring_ref, rc;
> +     int err, remote_port, ring_ref, vcon_remote_port, vcon_ring_ref;
>       char *type, path[PATH_MAX];
>  
>       err = xs_gather(xs, dom->conspath,
> @@ -547,6 +641,17 @@ static int domain_create_ring(struct domain *dom)
>       if (err)
>               goto out;
>  
> +     /* vcon console is optional. */
> +     err = xs_gather(xs, dom->conspath,
> +             "vcon-ring-ref", "%u", &vcon_ring_ref,
> +             "vcon-port", "%i", &vcon_remote_port,
> +             NULL);
> +

Indentation.

> +     if (err || vcon_ring_ref == -1)
> +             dom->vcon_enabled = false;
> +     else
> +             dom->vcon_enabled = true;
> +
>       snprintf(path, sizeof(path), "%s/type", dom->conspath);
>       type = xs_read(xs, XBT_NULL, path, NULL);
>       if (type && strcmp(type, "xenconsoled") != 0) {
> @@ -556,41 +661,51 @@ 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 != dom->ring_ref[CONSOLE_TYPE_PV] && 
> dom->ring_ref[CONSOLE_TYPE_PV] != -1)

Line too long.

> +             domain_unmap_interface(dom, CONSOLE_TYPE_PV);
> +
> +     /* If using vcon ring_ref and it has changed, remap */
> +     if (dom->vcon_enabled &&
> +             vcon_ring_ref != dom->ring_ref[CONSOLE_TYPE_VCON] &&
> +             dom->ring_ref[CONSOLE_TYPE_VCON] != -1 )
> +             domain_unmap_interface(dom, CONSOLE_TYPE_VCON);

Indentation.

>  
> -     if (!dom->interface && xgt_handle) {
> +     if (!dom->interface[CONSOLE_TYPE_PV] && 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;
> +             dom->interface[CONSOLE_TYPE_PV] = 
> xengnttab_map_grant_ref(xgt_handle,
> +                                                                             
>                                                   dom->domid, 
> GNTTAB_RESERVED_CONSOLE,
> +                                                                             
>                                                   PROT_READ|PROT_WRITE);
> +             dom->ring_ref[CONSOLE_TYPE_PV] = -1;

Indentation.

>       }
> -     if (!dom->interface) {
> +
> +     if (!dom->interface[CONSOLE_TYPE_PV]) {
>               /* Fall back to xc_map_foreign_range */
> -             dom->interface = xc_map_foreign_range(
> +             dom->interface[CONSOLE_TYPE_PV] = xc_map_foreign_range(
>                       xc, dom->domid, XC_PAGE_SIZE,
>                       PROT_READ|PROT_WRITE,
>                       (unsigned long)ring_ref);
> -             if (dom->interface == NULL) {
> +             if (dom->interface[CONSOLE_TYPE_PV] == NULL) {
>                       err = EINVAL;
>                       goto out;
>               }
> -             dom->ring_ref = ring_ref;
> +             dom->ring_ref[CONSOLE_TYPE_PV] = 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))
> +     /* Map vcon console ring buffer. */
> +     if (dom->vcon_enabled && !dom->interface[CONSOLE_TYPE_VCON]) {
> +
> +             /* Fall back to xc_map_foreign_range */
> +             dom->interface[CONSOLE_TYPE_VCON] = xc_map_foreign_range(
> +             xc, dom->domid, XC_PAGE_SIZE,
> +             PROT_READ|PROT_WRITE,
> +             (unsigned long)vcon_ring_ref);
> +             if ( dom->interface[CONSOLE_TYPE_VCON] == NULL ) {
> +                     err = EINVAL;
>                       goto out;
> +             }
> +             dom->ring_ref[CONSOLE_TYPE_VCON] = vcon_ring_ref;
>       }
>  
> -     dom->local_port = -1;
> -     dom->remote_port = -1;
>       if (dom->xce_handle != NULL)
>               xenevtchn_close(dom->xce_handle);
>  
> @@ -602,31 +717,46 @@ static int domain_create_ring(struct domain *dom)
>               goto out;
>       }
>   
> -     rc = xenevtchn_bind_interdomain(dom->xce_handle,
> -             dom->domid, remote_port);
> -
> -     if (rc == -1) {
> -             err = errno;
> +     err = bind_event_channel(dom, remote_port,
> +                                                      
> &dom->local_port[CONSOLE_TYPE_PV],
> +                                                      
> &dom->remote_port[CONSOLE_TYPE_PV]);

Indentation.

> +     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 (dom->vcon_enabled)
> +     {
> +             err = bind_event_channel(dom, vcon_remote_port,
> +                                                              
> &dom->local_port[CONSOLE_TYPE_VCON],
> +                                                              
> &dom->remote_port[CONSOLE_TYPE_VCON]);

Indentation.

> +             if (err)
> +             {
> +                     xenevtchn_close(dom->xce_handle);
> +                     goto out;
> +             }
> +     }
> +
> +     if (dom->master_fd[CONSOLE_TYPE_PV] == -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;
> +                     dom->local_port[CONSOLE_TYPE_PV] = -1;
> +                     dom->remote_port[CONSOLE_TYPE_PV] = -1;
> +                     dom->local_port[CONSOLE_TYPE_VCON] = -1;
> +                     dom->remote_port[CONSOLE_TYPE_VCON] = -1;
> +                     dom->vcon_enabled = false;
>                       goto out;
>               }
>       }
>  
> -     if (log_guest && (dom->log_fd == -1))
> -             dom->log_fd = create_domain_log(dom);
> +     if (log_guest && (dom->log_fd[CONSOLE_TYPE_PV] == -1))
> +             dom->log_fd[CONSOLE_TYPE_PV] = create_domain_log(dom, 
> CONSOLE_TYPE_PV);
> +
> +     if (log_guest && dom->vcon_enabled && (dom->log_fd[CONSOLE_TYPE_VCON] 
> == -1))
> +             dom->log_fd[CONSOLE_TYPE_VCON] = create_domain_log(dom, 
> CONSOLE_TYPE_VCON);
>  
>   out:
>       return err;
> @@ -681,17 +811,24 @@ static struct domain *create_domain(int domid)
>       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->master_fd[CONSOLE_TYPE_PV] = -1;
> +     dom->master_pollfd_idx[CONSOLE_TYPE_PV] = -1;
> +     dom->slave_fd[CONSOLE_TYPE_PV] = -1;
> +     dom->master_fd[CONSOLE_TYPE_VCON] = -1;
> +     dom->master_pollfd_idx[CONSOLE_TYPE_VCON] = -1;
> +     dom->slave_fd[CONSOLE_TYPE_VCON] = -1;
> +     dom->log_fd[CONSOLE_TYPE_PV] = -1;
> +     dom->log_fd[CONSOLE_TYPE_VCON] = -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;
> +     dom->ring_ref[CONSOLE_TYPE_PV] = -1;
> +     dom->ring_ref[CONSOLE_TYPE_VCON] = -1;
> +     dom->local_port[CONSOLE_TYPE_PV] = -1;
> +     dom->remote_port[CONSOLE_TYPE_PV] = -1;
> +     dom->local_port[CONSOLE_TYPE_VCON] = -1;
> +     dom->remote_port[CONSOLE_TYPE_VCON] = -1;
>  
>       if (!watch_domain(dom, true))
>               goto out;
> @@ -737,13 +874,21 @@ static void cleanup_domain(struct domain *d)
>  {
>       domain_close_tty(d);
>  
> -     if (d->log_fd != -1) {
> -             close(d->log_fd);
> -             d->log_fd = -1;
> +     if (d->log_fd[CONSOLE_TYPE_PV] != -1) {
> +             close(d->log_fd[CONSOLE_TYPE_PV]);
> +             d->log_fd[CONSOLE_TYPE_PV] = -1;
> +     }
> +
> +     if (d->log_fd[CONSOLE_TYPE_VCON] != -1) {
> +             close(d->log_fd[CONSOLE_TYPE_VCON]);
> +             d->log_fd[CONSOLE_TYPE_VCON] = -1;
>       }
>  
> -     free(d->buffer.data);
> -     d->buffer.data = NULL;
> +     free(d->buffer[CONSOLE_TYPE_PV].data);
> +     free(d->buffer[CONSOLE_TYPE_VCON].data);
> +
> +     d->buffer[CONSOLE_TYPE_PV].data = NULL;
> +     d->buffer[CONSOLE_TYPE_VCON].data = NULL;
>  
>       free(d->conspath);
>       d->conspath = NULL;
> @@ -755,7 +900,8 @@ static void shutdown_domain(struct domain *d)
>  {
>       d->is_dead = true;
>       watch_domain(d, false);
> -     domain_unmap_interface(d);
> +     domain_unmap_interface(d, CONSOLE_TYPE_PV);
> +     domain_unmap_interface(d, CONSOLE_TYPE_VCON);
>       if (d->xce_handle != NULL)
>               xenevtchn_close(d->xce_handle);
>       d->xce_handle = NULL;
> @@ -786,9 +932,9 @@ static void enum_domains(void)
>       }
>  }
>  
> -static int ring_free_bytes(struct domain *dom)
> +static int ring_free_bytes(struct domain *dom, int console_type)
>  {
> -     struct xencons_interface *intf = dom->interface;
> +     struct xencons_interface *intf = dom->interface[console_type];
>       XENCONS_RING_IDX cons, prod, space;
>  
>       cons = intf->in_cons;
> @@ -813,25 +959,26 @@ 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 domain *dom, int console_type)
>  {
>       ssize_t len = 0;
>       char msg[80];
>       int i;
> -     struct xencons_interface *intf = dom->interface;
>       XENCONS_RING_IDX prod;
> +     struct xencons_interface *intf=dom->interface[console_type];
> +     xenevtchn_port_or_error_t port=dom->local_port[console_type];
>  
>       if (dom->is_dead)
>               return;
>  
> -     len = ring_free_bytes(dom);
> +     len = ring_free_bytes(dom, console_type);
>       if (len == 0)
>               return;
>  
>       if (len > sizeof(msg))
>               len = sizeof(msg);
>  
> -     len = read(dom->master_fd, msg, len);
> +     len = read(dom->master_fd[console_type], 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
> @@ -847,28 +994,29 @@ 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 domain *dom, int console_type)
>  {
>       ssize_t len;
>  
>       if (dom->is_dead)
>               return;
>  
> -     len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed,
> -                 dom->buffer.size - dom->buffer.consumed);
> +     len = write(dom->master_fd[console_type],
> +                             dom->buffer[console_type].data + 
> dom->buffer[console_type].consumed,
> +                             dom->buffer[console_type].size - 
> dom->buffer[console_type].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(&dom->buffer[console_type], len);
>       }
>  }
>  
> @@ -884,7 +1032,10 @@ static void handle_ring_read(struct domain *dom)
>  
>       dom->event_count++;
>  
> -     buffer_append(dom);
> +     if (port == dom->local_port[CONSOLE_TYPE_VCON])
> +             buffer_append(dom, CONSOLE_TYPE_VCON);
> +     else
> +             buffer_append(dom, CONSOLE_TYPE_PV);
>  
>       if (dom->event_count < RATE_LIMIT_ALLOWANCE)
>               (void)xenevtchn_unmask(dom->xce_handle, port);
> @@ -954,9 +1105,16 @@ 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);
> +                     if (d->log_fd[CONSOLE_TYPE_PV] != -1)
> +                             close(d->log_fd[CONSOLE_TYPE_PV]);
> +                     d->log_fd[CONSOLE_TYPE_PV] = create_domain_log(d, 
> CONSOLE_TYPE_PV);
> +
> +                     if (d->vcon_enabled)
> +                     {
> +                             if (d->log_fd[CONSOLE_TYPE_VCON] != -1)
> +                                     close(d->log_fd[CONSOLE_TYPE_VCON]);
> +                             d->log_fd[CONSOLE_TYPE_PV] = 
> create_domain_log(d, CONSOLE_TYPE_VCON);

Line too long.

> +                     }
>               }
>       }
>  
> @@ -1074,7 +1232,9 @@ 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);
> +                                     (void)xenevtchn_unmask(d->xce_handle, 
> d->local_port[CONSOLE_TYPE_PV]);
> +                                     if (d->vcon_enabled)
> +                                             
> (void)xenevtchn_unmask(d->xce_handle, d->local_port[CONSOLE_TYPE_VCON]);

Line too long.

>                               }
>                               d->event_count = 0;
>                       }
> @@ -1087,27 +1247,47 @@ 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 (buffer_available(d, CONSOLE_TYPE_PV)) {
> +                                     int evtchn_fd = 
> xenevtchn_fd(d->xce_handle);
> +                                     d->xce_pollfd_idx = set_fds(evtchn_fd,
> +                                                                             
>                 POLLIN|POLLPRI);

Indentation.

> +                             }
> +
> +                             if (buffer_available(d, CONSOLE_TYPE_VCON ) )

Extraneous space after CONSOLE_TYPE_VCON.

> +                             {
>                                       int evtchn_fd = 
> xenevtchn_fd(d->xce_handle);
>                                       d->xce_pollfd_idx = set_fds(evtchn_fd,
> -                                                                 
> POLLIN|POLLPRI);
> +                                                                             
>                 POLLIN|POLLPRI);

Indentation.

>                               }
>                       }
>  
> -                     if (d->master_fd != -1) {
> +                     if (d->master_fd[CONSOLE_TYPE_PV] != -1) {
>                               short events = 0;
> -                             if (!d->is_dead && ring_free_bytes(d))
> +                             if (!d->is_dead && ring_free_bytes(d, 
> CONSOLE_TYPE_PV))
>                                       events |= POLLIN;
>  
> -                             if (!buffer_empty(&d->buffer))
> +                             if (!buffer_empty(&d->buffer[CONSOLE_TYPE_PV]))
>                                       events |= POLLOUT;
>  
>                               if (events)
> -                                     d->master_pollfd_idx =
> -                                             set_fds(d->master_fd,
> -                                                     events|POLLPRI);
> +                                     d->master_pollfd_idx[CONSOLE_TYPE_PV] =
> +                                             
> set_fds(d->master_fd[CONSOLE_TYPE_PV],
> +                                                             events|POLLPRI);
> +                     }
> +
> +                     if (d->vcon_enabled && d->master_fd[CONSOLE_TYPE_VCON] 
> != -1) {
> +                             short events = 0;
> +                             if (!d->is_dead && ring_free_bytes(d, 
> CONSOLE_TYPE_VCON))
> +                                     events |= POLLIN;
> +
> +                             if 
> (!buffer_empty(&d->buffer[CONSOLE_TYPE_VCON]))
> +                                     events |= POLLOUT;
> +
> +                             if (events)
> +                                     d->master_pollfd_idx[CONSOLE_TYPE_VCON] 
> =
> +                                             
> set_fds(d->master_fd[CONSOLE_TYPE_VCON],
> +                                                             events|POLLPRI);
>                       }
>               }
>  
> @@ -1166,6 +1346,16 @@ void handle_io(void)
>  
>               for (d = dom_head; d; d = n) {
>                       n = d->next;
> +
> +                     /*
> +                      * Check if the data pending flag is set for any of the 
> consoles.
> +                      * If yes then service those first.
> +                      */
> +                     if ( d->console_data_pending & (1<<CONSOLE_TYPE_PV) )
> +                             buffer_append(d, CONSOLE_TYPE_PV);
> +                     else if ( d->console_data_pending & 
> (1<<CONSOLE_TYPE_VCON) )
> +                             buffer_append(d, CONSOLE_TYPE_VCON);
> +

Why? You seem to have skipped the ratelimit check here.

>                       if (d->event_count < RATE_LIMIT_ALLOWANCE) {
>                               if (d->xce_handle != NULL &&
>                                   d->xce_pollfd_idx != -1 &&
> @@ -1176,22 +1366,36 @@ 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 &
> +                     if (d->master_fd[CONSOLE_TYPE_PV] != -1 && 
> d->master_pollfd_idx[CONSOLE_TYPE_PV] != -1) {
> +                             if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_PV]].revents &
> +                                 ~(POLLIN|POLLOUT|POLLPRI))
> +                                     domain_handle_broken_tty(d, 
> domain_is_valid(d->domid));
> +                             else {
> +                                     if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_PV]].revents &
> +                                         POLLIN)
> +                                             handle_tty_read(d, 
> CONSOLE_TYPE_PV);
> +                                     if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_PV]].revents &
> +                                         POLLOUT)
> +                                             handle_tty_write(d, 
> CONSOLE_TYPE_PV);
> +                             }
> +                     }
> +
> +                     if (d->master_fd[CONSOLE_TYPE_VCON] != -1 && 
> d->master_pollfd_idx[CONSOLE_TYPE_VCON] != -1) {
> +                             if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_VCON]].revents &
>                                   ~(POLLIN|POLLOUT|POLLPRI))
> -                                     domain_handle_broken_tty(d,
> -                                                domain_is_valid(d->domid));
> +                                     domain_handle_broken_tty(d, 
> domain_is_valid(d->domid));
>                               else {
> -                                     if (fds[d->master_pollfd_idx].revents &
> +                                     if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_VCON]].revents &
>                                           POLLIN)
> -                                             handle_tty_read(d);
> -                                     if (fds[d->master_pollfd_idx].revents &
> +                                             handle_tty_read(d, 
> CONSOLE_TYPE_VCON);
> +                                     if 
> (fds[d->master_pollfd_idx[CONSOLE_TYPE_VCON]].revents &
>                                           POLLOUT)
> -                                             handle_tty_write(d);
> +                                             handle_tty_write(d, 
> CONSOLE_TYPE_VCON);
>                               }
>                       }
>  
> -                     d->xce_pollfd_idx = d->master_pollfd_idx = -1;
> +                     d->xce_pollfd_idx = 
> d->master_pollfd_idx[CONSOLE_TYPE_PV] =
> +                                                                     
> d->master_pollfd_idx[CONSOLE_TYPE_VCON] = -1;
>  

Indentation and line too long.

>                       if (d->last_seen != enum_pass)
>                               shutdown_domain(d);
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.