Re: [PATCH 08/12 v3] xen/arm: vpl011: Modify xenconsole to support multiple consoles

[Stefano Stabellini <sstabellini@kernel.org>]

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

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