Re: [PATCH 18/28] libxl: ocaml: add xen_console_read

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/03/2013 11:48, David Scott wrote:
> On 25/03/13 14:45, Rob Hoes wrote:
>> Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
>> ---
>>   tools/ocaml/libs/xl/xenlight.ml.in   |    1 +
>>   tools/ocaml/libs/xl/xenlight.mli.in  |    1 +
>>   tools/ocaml/libs/xl/xenlight_stubs.c |   27 +++++++++++++++++++++++++++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/tools/ocaml/libs/xl/xenlight.ml.in b/tools/ocaml/libs/xl/xenlight.ml.in
>> index 991b2bf..63b8bf8 100644
>> --- a/tools/ocaml/libs/xl/xenlight.ml.in
>> +++ b/tools/ocaml/libs/xl/xenlight.ml.in
>> @@ -78,5 +78,6 @@ type devid = int
>>   external send_trigger : ctx -> domid -> trigger -> int -> unit = "stub_xl_send_trigger"
>>   external send_sysrq : ctx -> domid -> char -> unit = "stub_xl_send_sysrq"
>>   external send_debug_keys : ctx -> string -> unit = "stub_xl_send_debug_keys"
>> +external xen_console_read : ctx -> string list = "stub_xl_xen_console_read"
>>
>>   let _ = Callback.register_exception "Xenlight.Error" (Error(Fail, ""))
>> diff --git a/tools/ocaml/libs/xl/xenlight.mli.in b/tools/ocaml/libs/xl/xenlight.mli.in
>> index 12568ca..24064fc 100644
>> --- a/tools/ocaml/libs/xl/xenlight.mli.in
>> +++ b/tools/ocaml/libs/xl/xenlight.mli.in
>> @@ -50,3 +50,4 @@ type devid = int
>>   external send_trigger : ctx -> domid -> trigger -> int -> unit = "stub_xl_send_trigger"
>>   external send_sysrq : ctx -> domid -> char -> unit = "stub_xl_send_sysrq"
>>   external send_debug_keys : ctx -> string -> unit = "stub_xl_send_debug_keys"
>> +external xen_console_read : ctx -> string list = "stub_xl_xen_console_read"
>> diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
>> index f4fa520..939e993 100644
>> --- a/tools/ocaml/libs/xl/xenlight_stubs.c
>> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
>> @@ -484,6 +484,33 @@ value stub_xl_send_debug_keys(value ctx, value keys)
>>   	CAMLreturn(Val_unit);
>>   }
>>
>> +value stub_xl_xen_console_read(value ctx)
>> +{
>> +	CAMLparam1(ctx);
>> +	CAMLlocal3(list, cons, ml_line);
>> +	int i = 0, ret;
>> +	char *console[32768], *line;
>> +	libxl_xen_console_reader *cr;
>> +
>> +	cr = libxl_xen_console_read_start(CTX, 0);
>> +	if (cr)
>> +		for (i = 0; libxl_xen_console_read_line(CTX, cr, &line) > 0; i++)
>  > +			console[i] = strdup(line);
>
> Is it obvious why i is always < 32768? Or is 32768 "too big to fail"? 
> (Sorry, couldn't resist)

This is 32K entries of 4 or 8 bytes (for 32/64bit system), meaning 128K
or 256K of data on the stack.

I am surprised that this didn't segfault instantly, but this does seem
an unreasonably large amount of data, especially for some library bindings.

If you want to end up with a list of lines, I would suggest using my
patch series to grab the entire console ring at once, counting the
number of '\n's present and working with that.

~Andrew

>
>> +	libxl_xen_console_read_finish(CTX, cr);
>> +
>> +	list = Val_emptylist;
>> +	for (; i > 0; i--) {
>> +		ml_line = caml_copy_string(console[i - 1]);
>> +		free(console[i - 1]);
>> +		cons = caml_alloc(2, 0);
>> +		Store_field(cons, 0, ml_line);  // head
>> +		Store_field(cons, 1, list);     // tail
>> +		list = cons;
>> +	}
>> +
>> +	CAMLreturn(list);
>> +}
>> +
>>   /*
>>    * Local variables:
>>    *  indent-tabs-mode: t
>>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel