[PATCH 0 of 5] tools: console ring fixes

[PATCH 0 of 5] tools: console ring fixes

[Andrew Cooper]

Hello,

The following set of patches came about when trying to fix the 32K limitation
with stub_xc_readconsolering() in the ocaml bindings.

Patch 1 is a hypervisor patch adding a new SYSCTL hypercall to query the size
of the console ring, which is fixed but otherwise inaccessible after boot.

Patches 2 thru 5 are tools patches:
 * Patch 2 adds a libxc function to use the new SYSCTL hypercall.
 * Patch 3 implements an alternative xc_readconsolering() to prevent bounce
   buffering.
 * Patch 4 uses the preceding patches to fix the implementation of
   stub_xc_readconsolering() in the ocaml bindings.
 * Patch 5 is a misc threading fix in the ocaml bindings, discovered when
   trying to fix the console functionality.

This patch set has been compile-tested on unstable, and functionally tested
via backport to 4.2 (which was only one whitespace fuzz issue and no code
change).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

[PATCH 1 of 5] common/sysctl: Introduce hypercall to query the console ring size

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 66f563be41d9 -r a380b2169803 xen/common/sysctl.c
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -357,6 +357,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
     }
     break;
 
+    case XEN_SYSCTL_consoleringsize:
+        ret = console_ring_size(&op->u.consoleringsize);
+        break;
 
     default:
         ret = arch_do_sysctl(op, u_sysctl);
diff -r 66f563be41d9 -r a380b2169803 xen/drivers/char/console.c
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -226,6 +226,12 @@ long read_console_ring(struct xen_sysctl
     return 0;
 }
 
+long console_ring_size(struct xen_sysctl_consoleringsize * op)
+{
+    op->size = conring_size;
+    return 0;
+}
+
 
 /*
  * *******************************************************
diff -r 66f563be41d9 -r a380b2169803 xen/include/public/sysctl.h
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -596,6 +596,14 @@ struct xen_sysctl_scheduler_op {
 typedef struct xen_sysctl_scheduler_op xen_sysctl_scheduler_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_scheduler_op_t);
 
+/* XEN_SYSCTL_consoleringsize */
+/* Get the size of the hypervisor console ring. */
+struct xen_sysctl_consoleringsize {
+    uint64_t size; /* Written by Xen. */
+};
+typedef struct xen_sysctl_consoleringsize xen_sysctl_consoleringsize_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_consoleringsize_t);
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -616,6 +624,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_numainfo                      17
 #define XEN_SYSCTL_cpupool_op                    18
 #define XEN_SYSCTL_scheduler_op                  19
+#define XEN_SYSCTL_consoleringsize               20
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -636,6 +645,7 @@ struct xen_sysctl {
         struct xen_sysctl_lockprof_op       lockprof_op;
         struct xen_sysctl_cpupool_op        cpupool_op;
         struct xen_sysctl_scheduler_op      scheduler_op;
+        struct xen_sysctl_consoleringsize   consoleringsize;
         uint8_t                             pad[128];
     } u;
 };
diff -r 66f563be41d9 -r a380b2169803 xen/include/xen/console.h
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -12,6 +12,8 @@
 
 struct xen_sysctl_readconsole;
 long read_console_ring(struct xen_sysctl_readconsole *op);
+struct xen_sysctl_consoleringsize;
+long console_ring_size(struct xen_sysctl_consoleringsize *op);
 
 void console_init_preirq(void);
 void console_init_postirq(void);

[PATCH 2 of 5] tools/libxc: Helper function for XEN_SYSCTL_consoleringsize

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r a380b2169803 -r 40d1bab1b996 tools/libxc/xc_misc.c
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -100,6 +100,26 @@ int xc_readconsolering(xc_interface *xch
     return ret;
 }
 
+int xc_consoleringsize(xc_interface *xch, uint64_t * psize)
+{
+    int ret = -1;
+    DECLARE_SYSCTL;
+
+    if ( ! psize )
+    {
+        errno = EFAULT;
+        return ret;
+    }
+
+    sysctl.cmd = XEN_SYSCTL_consoleringsize;
+    ret = do_sysctl(xch, &sysctl);
+
+    if ( ! ret )
+        *psize = sysctl.u.consoleringsize.size;
+
+    return ret;
+}
+
 int xc_send_debug_keys(xc_interface *xch, char *keys)
 {
     int ret, len = strlen(keys);
diff -r a380b2169803 -r 40d1bab1b996 tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -998,6 +998,7 @@ int xc_readconsolering(xc_interface *xch
                        char *buffer,
                        unsigned int *pnr_chars,
                        int clear, int incremental, uint32_t *pindex);
+int xc_consoleringsize(xc_interface *xch, uint64_t * psize);
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);

[PATCH 3 of 5] tools/libxc: Implement of xc_readconsolering_buffer

[Andrew Cooper]

Functions identically to xc_readconsolering(), but uses a user-provided
xc_hypercall_buffer_t to save using a bounce buffer.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 40d1bab1b996 -r 6b8c513cff4f tools/libxc/xc_misc.c
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -100,6 +100,37 @@ int xc_readconsolering(xc_interface *xch
     return ret;
 }
 
+int xc_readconsolering_buffer(xc_interface *xch,
+                              xc_hypercall_buffer_t *buffer,
+                              unsigned int *pnr_chars,
+                              int clear, int incremental, uint32_t *pindex)
+{
+    int ret;
+    unsigned int nr_chars = *pnr_chars;
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BUFFER_ARGUMENT(buffer);
+
+    sysctl.cmd = XEN_SYSCTL_readconsole;
+    set_xen_guest_handle(sysctl.u.readconsole.buffer, buffer);
+    sysctl.u.readconsole.count = nr_chars;
+    sysctl.u.readconsole.clear = clear;
+    sysctl.u.readconsole.incremental = 0;
+    if ( pindex )
+    {
+        sysctl.u.readconsole.index = *pindex;
+        sysctl.u.readconsole.incremental = incremental;
+    }
+
+    if ( (ret = do_sysctl(xch, &sysctl)) == 0 )
+    {
+        *pnr_chars = sysctl.u.readconsole.count;
+        if ( pindex )
+            *pindex = sysctl.u.readconsole.index;
+    }
+
+    return ret;
+}
+
 int xc_consoleringsize(xc_interface *xch, uint64_t * psize)
 {
     int ret = -1;
diff -r 40d1bab1b996 -r 6b8c513cff4f tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -998,6 +998,10 @@ int xc_readconsolering(xc_interface *xch
                        char *buffer,
                        unsigned int *pnr_chars,
                        int clear, int incremental, uint32_t *pindex);
+int xc_readconsolering_buffer(xc_interface *xch,
+                              xc_hypercall_buffer_t *buffer,
+                              unsigned int *pnr_chars,
+                              int clear, int incremental, uint32_t *pindex);
 int xc_consoleringsize(xc_interface *xch, uint64_t * psize);
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);

Re: [PATCH 3 of 5] tools/libxc: Implement of xc_readconsolering_buffer

[Ian Campbell]

On Wed, 2013-02-20 at 18:05 +0000, Andrew Cooper wrote:
> Functions identically to xc_readconsolering(), but uses a user-provided
> xc_hypercall_buffer_t to save using a bounce buffer.

Can we now trivially reimplement xc_readconsolering in terms of this
function?

Should just be a case of replacing all the code between bounce_pre and
bounce_post with a call
	xc_readconsolering_buffer(xch, HYPERCALL_BUFFER_AS_ARG(buffer),
                                  pnr_chars, clear, incremental, pindex);
?

We should probably also deprecate the old one and at least have a vague
idea that we might eventually switch all the callers over (you don't
need to here though, unless you are super keen).

Ian.

Re: [PATCH 3 of 5] tools/libxc: Implement of xc_readconsolering_buffer

[Andrew Cooper]

On 21/02/13 10:01, Ian Campbell wrote:
> On Wed, 2013-02-20 at 18:05 +0000, Andrew Cooper wrote:
>> Functions identically to xc_readconsolering(), but uses a user-provided
>> xc_hypercall_buffer_t to save using a bounce buffer.
> Can we now trivially reimplement xc_readconsolering in terms of this
> function?
>
> Should just be a case of replacing all the code between bounce_pre and
> bounce_post with a call
> 	xc_readconsolering_buffer(xch, HYPERCALL_BUFFER_AS_ARG(buffer),
>                                   pnr_chars, clear, incremental, pindex);
> ?

Ok

>
> We should probably also deprecate the old one and at least have a vague
> idea that we might eventually switch all the callers over (you don't
> need to here though, unless you are super keen).
>
> Ian.
>

Depending on the quantity of callers, I may or may not due to time.

~Andrew

[PATCH 4 of 5] tools/ocaml: libxc bindings: Fix stub_xc_readconsolering()

[Andrew Cooper]

There are two problems with this function:
  * The caml_{enter,leave}_blocking_section() calls mean that two threads can
    compete for use of the static ring[] buffer.
  * It fails to retrieve the entire buffer if the hypervisor has used 32K or
    more of its console ring.

Rewrite the function using the new xc_consoleringsize() and
xc_readconsolering_buffer() functions to efficiently grab the entire console
ring.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 6b8c513cff4f -r a73d0c5a5b24 tools/ocaml/libs/xc/xenctrl_stubs.c
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -523,27 +523,61 @@ CAMLprim value stub_xc_evtchn_reset(valu
 	CAMLreturn(Val_unit);
 }
 
-
-#define RING_SIZE 32768
-static char ring[RING_SIZE];
-
+/* Maximum size of console ring we are prepared to read, 16MB */
+#define MAX_CONSOLE_RING_SIZE (1U << 24)
 CAMLprim value stub_xc_readconsolering(value xch)
 {
-	unsigned int size = RING_SIZE - 1;
-	char *ring_ptr = ring;
-	int retval;
+	uint64_t conring_size;
+	DECLARE_HYPERCALL_BUFFER(char, ring);
+	unsigned int nr_chars;
+	int r;
 
 	CAMLparam1(xch);
+	CAMLlocal1(conring);
+
+	r = xc_consoleringsize(_H(xch), &conring_size);
+
+	if ( r )
+	{
+		failwith_xc(_H(xch));
+		CAMLreturn(Val_unit);
+	}
+
+	/* Round conring_size up to the next page, for NULL terminator and
+	   slop from a race with printk() in the hypervisor. */
+	conring_size = (conring_size + XC_PAGE_SIZE) & XC_PAGE_MASK;
+	nr_chars = conring_size-1;
+
+	if ( conring_size > MAX_CONSOLE_RING_SIZE )
+	{
+		caml_failwith("Console ring too large");
+		CAMLreturn(Val_unit);
+	}
+
+	ring = xc_hypercall_buffer_alloc(_H(xch), ring, conring_size);
+
+	if ( ! ring )
+	{
+		failwith_xc(_H(xch));
+		CAMLreturn(Val_unit);
+	}
 
 	caml_enter_blocking_section();
-	retval = xc_readconsolering(_H(xch), ring_ptr, &size, 0, 0, NULL);
+	r = xc_readconsolering_buffer(_H(xch), HYPERCALL_BUFFER(ring),
+				      &nr_chars, 0, 0, NULL);
 	caml_leave_blocking_section();
 
-	if (retval)
+	if ( r )
+	{
 		failwith_xc(_H(xch));
+		xc_hypercall_buffer_free(_H(xch), ring);
+		CAMLreturn(Val_unit);
+	}
 
-	ring[size] = '\0';
-	CAMLreturn(caml_copy_string(ring));
+	ring[nr_chars] = '\0';
+	conring = caml_copy_string(ring);
+	xc_hypercall_buffer_free(_H(xch), ring);
+	CAMLreturn(conring);
 }
 
 CAMLprim value stub_xc_send_debug_keys(value xch, value keys)

Re: [PATCH 4 of 5] tools/ocaml: libxc bindings: Fix stub_xc_readconsolering()

[Ian Campbell]

On Wed, 2013-02-20 at 18:05 +0000, Andrew Cooper wrote:
> There are two problems with this function:
>   * The caml_{enter,leave}_blocking_section() calls mean that two threads can
>     compete for use of the static ring[] buffer.
>   * It fails to retrieve the entire buffer if the hypervisor has used 32K or
>     more of its console ring.
> 
> Rewrite the function using the new xc_consoleringsize() and
> xc_readconsolering_buffer() functions to efficiently grab the entire console
> ring.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> diff -r 6b8c513cff4f -r a73d0c5a5b24 tools/ocaml/libs/xc/xenctrl_stubs.c
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -523,27 +523,61 @@ CAMLprim value stub_xc_evtchn_reset(valu
>  	CAMLreturn(Val_unit);
>  }
>  
> -
> -#define RING_SIZE 32768
> -static char ring[RING_SIZE];
> -
> +/* Maximum size of console ring we are prepared to read, 16MB */
> +#define MAX_CONSOLE_RING_SIZE (1U << 24)

A bit arbitrary, why not just let it handle whatever the hypervisor
throws at it. If there is to be a limit it probably belongs on the
hypervisor side, but in the meantime someone who passes consring=1G can
keep all the pieces ;-)

>  CAMLprim value stub_xc_readconsolering(value xch)
>  {
> -	unsigned int size = RING_SIZE - 1;
> -	char *ring_ptr = ring;
> -	int retval;
> +	uint64_t conring_size;
> +	DECLARE_HYPERCALL_BUFFER(char, ring);
> +	unsigned int nr_chars;
> +	int r;
>  
>  	CAMLparam1(xch);
> +	CAMLlocal1(conring);
> +
> +	r = xc_consoleringsize(_H(xch), &conring_size);

static int consoleringsize and only call this once the first time?

> +
> +	if ( r )
> +	{
> +		failwith_xc(_H(xch));
> +		CAMLreturn(Val_unit);
> +	}
> +
> +	/* Round conring_size up to the next page, for NULL terminator and
> +	   slop from a race with printk() in the hypervisor. */
> +	conring_size = (conring_size + XC_PAGE_SIZE) & XC_PAGE_MASK;
> +	nr_chars = conring_size-1;
> +
> +	if ( conring_size > MAX_CONSOLE_RING_SIZE )
> +	{
> +		caml_failwith("Console ring too large");
> +		CAMLreturn(Val_unit);
> +	}
> +
> +	ring = xc_hypercall_buffer_alloc(_H(xch), ring, conring_size);
> +
> +	if ( ! ring )
> +	{
> +		failwith_xc(_H(xch));
> +		CAMLreturn(Val_unit);
> +	}
>  
>  	caml_enter_blocking_section();
> -	retval = xc_readconsolering(_H(xch), ring_ptr, &size, 0, 0, NULL);
> +	r = xc_readconsolering_buffer(_H(xch), HYPERCALL_BUFFER(ring),
> +				      &nr_chars, 0, 0, NULL);
>  	caml_leave_blocking_section();
>  
> -	if (retval)
> +	if ( r )
> +	{
>  		failwith_xc(_H(xch));

Doesn't this (due to caml_raise...) end up exiting synchronously via a
longjmp back to the runtime? i.e. it leaks the buffer which is only
free'd after.

It's a good idea to cc xen-api@ with ocaml changes, for this sort of
reason...


> +		xc_hypercall_buffer_free(_H(xch), ring);
> +		CAMLreturn(Val_unit);

I think CAMLnoreturn here.

> +	}
>  
> -	ring[size] = '\0';
> -	CAMLreturn(caml_copy_string(ring));
> +	ring[nr_chars] = '\0';
> +	conring = caml_copy_string(ring);
> +	xc_hypercall_buffer_free(_H(xch), ring);
> +	CAMLreturn(conring);
>  }
>  
>  CAMLprim value stub_xc_send_debug_keys(value xch, value keys)

Re: [PATCH 4 of 5] tools/ocaml: libxc bindings: Fix stub_xc_readconsolering()

[Andrew Cooper]

On 21/02/13 10:14, Ian Campbell wrote:
> On Wed, 2013-02-20 at 18:05 +0000, Andrew Cooper wrote:
>> There are two problems with this function:
>>   * The caml_{enter,leave}_blocking_section() calls mean that two threads can
>>     compete for use of the static ring[] buffer.
>>   * It fails to retrieve the entire buffer if the hypervisor has used 32K or
>>     more of its console ring.
>>
>> Rewrite the function using the new xc_consoleringsize() and
>> xc_readconsolering_buffer() functions to efficiently grab the entire console
>> ring.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> diff -r 6b8c513cff4f -r a73d0c5a5b24 tools/ocaml/libs/xc/xenctrl_stubs.c
>> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
>> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> @@ -523,27 +523,61 @@ CAMLprim value stub_xc_evtchn_reset(valu
>>  	CAMLreturn(Val_unit);
>>  }
>>  
>> -
>> -#define RING_SIZE 32768
>> -static char ring[RING_SIZE];
>> -
>> +/* Maximum size of console ring we are prepared to read, 16MB */
>> +#define MAX_CONSOLE_RING_SIZE (1U << 24)
> A bit arbitrary, why not just let it handle whatever the hypervisor
> throws at it. If there is to be a limit it probably belongs on the
> hypervisor side, but in the meantime someone who passes consring=1G can
> keep all the pieces ;-)
>
>>  CAMLprim value stub_xc_readconsolering(value xch)
>>  {
>> -	unsigned int size = RING_SIZE - 1;
>> -	char *ring_ptr = ring;
>> -	int retval;
>> +	uint64_t conring_size;
>> +	DECLARE_HYPERCALL_BUFFER(char, ring);
>> +	unsigned int nr_chars;
>> +	int r;
>>  
>>  	CAMLparam1(xch);
>> +	CAMLlocal1(conring);
>> +
>> +	r = xc_consoleringsize(_H(xch), &conring_size);
> static int consoleringsize and only call this once the first time?

One of the point of this patch was to remove the static buffer for
thread safety reasons.  Having said that, even if you do race on this
static variable, it should be filled in with the same value, so is
probably safe?

>
>> +
>> +	if ( r )
>> +	{
>> +		failwith_xc(_H(xch));
>> +		CAMLreturn(Val_unit);
>> +	}
>> +
>> +	/* Round conring_size up to the next page, for NULL terminator and
>> +	   slop from a race with printk() in the hypervisor. */
>> +	conring_size = (conring_size + XC_PAGE_SIZE) & XC_PAGE_MASK;
>> +	nr_chars = conring_size-1;
>> +
>> +	if ( conring_size > MAX_CONSOLE_RING_SIZE )
>> +	{
>> +		caml_failwith("Console ring too large");
>> +		CAMLreturn(Val_unit);
>> +	}
>> +
>> +	ring = xc_hypercall_buffer_alloc(_H(xch), ring, conring_size);
>> +
>> +	if ( ! ring )
>> +	{
>> +		failwith_xc(_H(xch));
>> +		CAMLreturn(Val_unit);
>> +	}
>>  
>>  	caml_enter_blocking_section();
>> -	retval = xc_readconsolering(_H(xch), ring_ptr, &size, 0, 0, NULL);
>> +	r = xc_readconsolering_buffer(_H(xch), HYPERCALL_BUFFER(ring),
>> +				      &nr_chars, 0, 0, NULL);
>>  	caml_leave_blocking_section();
>>  
>> -	if (retval)
>> +	if ( r )
>> +	{
>>  		failwith_xc(_H(xch));
> Doesn't this (due to caml_raise...) end up exiting synchronously via a
> longjmp back to the runtime? i.e. it leaks the buffer which is only
> free'd after.
>
> It's a good idea to cc xen-api@ with ocaml changes, for this sort of
> reason...

I will double check that.  If so, then I have some memory leaks to fix
up in other bits of Ocaml/C bindings, which I was using as an example
here...

I will cc xen-api@ for the next submission of this series.

>
>> +		xc_hypercall_buffer_free(_H(xch), ring);
>> +		CAMLreturn(Val_unit);
> I think CAMLnoreturn here.

Ok.

~Andrew

>
>> +	}
>>  
>> -	ring[size] = '\0';
>> -	CAMLreturn(caml_copy_string(ring));
>> +	ring[nr_chars] = '\0';
>> +	conring = caml_copy_string(ring);
>> +	xc_hypercall_buffer_free(_H(xch), ring);
>> +	CAMLreturn(conring);
>>  }
>>  
>>  CAMLprim value stub_xc_send_debug_keys(value xch, value keys)
>

[PATCH 5 of 5] tools/ocaml: libxc bindings: Fix failwith_xc()

[Andrew Cooper]

The static error_str[] buffer is not thread-safe, and 1024 bytes is
unreasonably large.  Reduce to 256 bytes (which is still much larger than any
current use), and move it to being a stack variable.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r a73d0c5a5b24 -r 4ba606ff8cf9 tools/ocaml/libs/xc/xenctrl_stubs.c
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -51,21 +51,22 @@
 	i1 = (uint32_t) Int64_val(Field(input, 0)); \
 	i2 = ((Field(input, 1) == Val_none) ? 0xffffffff : (uint32_t) Int64_val(Field(Field(input, 1), 0)));
 
-#define ERROR_STRLEN 1024
 void failwith_xc(xc_interface *xch)
 {
-	static char error_str[ERROR_STRLEN];
+	char error_str[256];
 	if (xch) {
 		const xc_error *error = xc_get_last_error(xch);
 		if (error->code == XC_ERROR_NONE)
-                	snprintf(error_str, ERROR_STRLEN, "%d: %s", errno, strerror(errno));
+                	snprintf(error_str, sizeof(error_str),
+				 "%d: %s", errno, strerror(errno));
 		else
-			snprintf(error_str, ERROR_STRLEN, "%d: %s: %s",
-				 error->code,
+			snprintf(error_str, sizeof(error_str),
+				 "%d: %s: %s", error->code,
 				 xc_error_code_to_desc(error->code),
 				 error->message);
 	} else {
-		snprintf(error_str, ERROR_STRLEN, "Unable to open XC interface");
+		snprintf(error_str, sizeof(error_str),
+			 "Unable to open XC interface");
 	}
 	caml_raise_with_string(*caml_named_value("xc.error"), error_str);
 }