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

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi,

On 17 May 2017 at 05:11, Stefano Stabellini <sstabellini@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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

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