Xen devel - Mar 2006 - [PATCH] XML-RPC: Cope with large integers on x86-64 systems

Attached patch fixes a problem in marshaling large integers.  It seems 
to only occur on systems with very large memory.  However, on those 
systems, it''s a show-stopper.  I see this as a temporary solution until
we can sanitize the XML-RPC functions and get rid of all the 
S-Expression stuff.

I''ll run xm-test tomorrow against it.  Right now I''ve only
done basic
testing.

Regards,

Anthony Liguori


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 28 Mar 2006, at 03:29, Anthony Liguori wrote:
> Attached patch fixes a problem in marshaling large integers.  It seems 
> to only occur on systems with very large memory.  However, on those 
> systems, it''s a show-stopper.  I see this as a temporary solution 
> until we can sanitize the XML-RPC functions and get rid of all the 
> S-Expression stuff.
I thought the main reason to push the XML-RPC stuff before 3.0.2 was to 
give a measure of stability to that interface, and provide a more 
supportable interface for vendors to lock onto? That reasoning doesn''t 
really hold if the core of the interface needs more refactoring and 
cleaning up post 3.0.2.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Mon, Mar 27, 2006 at 08:29:13PM -0600, Anthony Liguori wrote:
> Attached patch fixes a problem in marshaling large integers.  It seems 
> to only occur on systems with very large memory.  However, on those 
> systems, it''s a show-stopper.  I see this as a temporary solution
until
> we can sanitize the XML-RPC functions and get rid of all the 
> S-Expression stuff.
Anthony,

This code is clearly plagiarised from Lib/xmlrpclib.py in the Python
distribution, without appropriate credit, copyright and licence statements in
the code, and without a patch to the documentation as per the licence
agreement.  I can''t accept this patch as it stands.

Regardless, I don''t see what is to be gained in marshalling ints
>32bits as
strings, but those <=32 bits as integers; the receiving code is going to have
to convert the incoming argument unconditionally, so why not just send all
integers as strings?  They''re going to be stringified on the wire
anyway, so
there''s no efficiency loss, and I don''t think that anyone
fancies auditing
Xend to figure out which integers are strict 32 bit integers on all platforms
and which aren''t, so it would be safer just to pass them all as
strings.

Ewan.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ewan Mellor wrote:
> On Mon, Mar 27, 2006 at 08:29:13PM -0600, Anthony Liguori wrote:
>
>   
>> Attached patch fixes a problem in marshaling large integers.  It seems 
>> to only occur on systems with very large memory.  However, on those 
>> systems, it''s a show-stopper.  I see this as a temporary
solution until
>> we can sanitize the XML-RPC functions and get rid of all the 
>> S-Expression stuff.
>>     
>
> Anthony,
>
> This code is clearly plagiarised from Lib/xmlrpclib.py in the Python
> distribution, without appropriate credit, copyright and licence statements
in
> the code, and without a patch to the documentation as per the licence
> agreement.  I can''t accept this patch as it stands.
>   
Sorry, I didn''t realize I hadn''t put that in there.  Attached
is another
patch with the appropriate references.
> Regardless, I don''t see what is to be gained in marshalling ints
>32bits as
> strings, but those <=32 bits as integers; the receiving code is going to
have
> to convert the incoming argument unconditionally, so why not just send all
> integers as strings?  They''re going to be stringified on the wire
anyway, so
> there''s no efficiency loss, and I don''t think that anyone
fancies auditing
> Xend to figure out which integers are strict 32 bit integers on all
platforms
> and which aren''t, so it would be safer just to pass them all as
strings.
>   
This is a pretty big hack which based on Aravindh''s note may not be as 
important as I thought it was.  Aravindh bumped NR_CPUS which increased 
the size of vcpu_avail.  I''ve audited the interface and the only place 
that large ints can be passed around are in the S-Expressions.  The 
S-Expressions are untyped so the user always has to manually do type 
conversion.  Passing the S-Expressions as strings is always safe.

I think this is the least invasive and safest way to address this 
problem.  Since it will only occur if you bump NR_CPUS, we could perhaps 
take the time to think of a more clever solution.

Regards,

Anthony Liguori
> Ewan.
>   


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
>
> On 28 Mar 2006, at 03:29, Anthony Liguori wrote:
>
>> Attached patch fixes a problem in marshaling large integers.  It 
>> seems to only occur on systems with very large memory.  However, on 
>> those systems, it''s a show-stopper.  I see this as a temporary
>> solution until we can sanitize the XML-RPC functions and get rid of 
>> all the S-Expression stuff.
>
> I thought the main reason to push the XML-RPC stuff before 3.0.2 was 
> to give a measure of stability to that interface, and provide a more 
> supportable interface for vendors to lock onto? That reasoning
doesn''t
> really hold if the core of the interface needs more refactoring and 
> cleaning up post 3.0.2.
The approach for the XML-RPC work was to first get XML-RPC support into 
the tree and then build a standard interface.  Building a standard 
interface requires refactoring (which will be simplification) to get rid 
of things like S-Expression configs.  To get rid of that, we have to get 
rid of the S-Expression/HTTP layer.

This whole reasoning is outlined on the wiki page.

http://wiki.xensource.com/xenwiki/Xend/XML-RPC

Regards,

Anthony Liguori
>  -- Keir
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 28 Mar 2006, at 16:18, Anthony Liguori wrote:
> The approach for the XML-RPC work was to first get XML-RPC support 
> into the tree and then build a standard interface.  Building a 
> standard interface requires refactoring (which will be simplification) 
> to get rid of things like S-Expression configs.  To get rid of that, 
> we have to get rid of the S-Expression/HTTP layer.
>
> This whole reasoning is outlined on the wiki page.
Thanks.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Ewan,

I''m not opposed to keeping this out of 3.0.2.  This is a pretty skanky 
hack and since you have to manually modify the tree to even see this 
problem, having to manually apply a patch doesn''t seem so bad.

I can write a patch shortly after 3.0.2 that changes vcpu_avail to 
something a bit more sane and update xm accordingly.

Regards,

Anthony Liguori

Anthony Liguori wrote:
> Ewan Mellor wrote:
>> On Mon, Mar 27, 2006 at 08:29:13PM -0600, Anthony Liguori wrote:
>>
>>  
>>> Attached patch fixes a problem in marshaling large integers.  It 
>>> seems to only occur on systems with very large memory.  However, on
>>> those systems, it''s a show-stopper.  I see this as a
temporary
>>> solution until we can sanitize the XML-RPC functions and get rid of
>>> all the S-Expression stuff.
>>>     
>>
>> Anthony,
>>
>> This code is clearly plagiarised from Lib/xmlrpclib.py in the Python
>> distribution, without appropriate credit, copyright and licence 
>> statements in
>> the code, and without a patch to the documentation as per the licence
>> agreement.  I can''t accept this patch as it stands.
>>   
>
> Sorry, I didn''t realize I hadn''t put that in there. 
Attached is
> another patch with the appropriate references.
>
>> Regardless, I don''t see what is to be gained in marshalling
ints
>> >32bits as
>> strings, but those <=32 bits as integers; the receiving code is
going
>> to have
>> to convert the incoming argument unconditionally, so why not just 
>> send all
>> integers as strings?  They''re going to be stringified on the
wire
>> anyway, so
>> there''s no efficiency loss, and I don''t think that
anyone fancies
>> auditing
>> Xend to figure out which integers are strict 32 bit integers on all 
>> platforms
>> and which aren''t, so it would be safer just to pass them all
as strings.
>>   
>
> This is a pretty big hack which based on Aravindh''s note may not
be as
> important as I thought it was.  Aravindh bumped NR_CPUS which 
> increased the size of vcpu_avail.  I''ve audited the interface and
the
> only place that large ints can be passed around are in the 
> S-Expressions.  The S-Expressions are untyped so the user always has 
> to manually do type conversion.  Passing the S-Expressions as strings 
> is always safe.
>
> I think this is the least invasive and safest way to address this 
> problem.  Since it will only occur if you bump NR_CPUS, we could 
> perhaps take the time to think of a more clever solution.
>
> Regards,
>
> Anthony Liguori
>
>> Ewan.
>>   
>
> ------------------------------------------------------------------------
>
> # HG changeset patch
> # User anthony@rhesis.austin.ibm.com
> # Node ID 16b3267df4ebbfeeab91aadb5a48ed89609e3622
> # Parent  8b5a752167a17785b998a080a5c1ce1991b9379b
> In some cases, Xend can return a very large integer.  This so far only
happens
> on 64bit systems where the sizeof the Python integer is > 32bit. 
Presumably,
> this is occuring because of the fact that we return PFNs as part of the
> domain info so on a system with greater than 2G of memory we get into the
> invalid integer range.
>
> For now, the work-around is to overload the xmlrpclib Marshaller and
instead
> of throwing an exception when we get out-of-range integers, we marshal them
> as strings.  In the future, we can handle this in a more elegant way by
> introducing higher-level types.  We also won''t have to deal with
things like
> PFNs once we clean up the XML-RPC interface.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> diff -r 8b5a752167a1 -r 16b3267df4eb tools/python/xen/util/xmlrpclib2.py
> --- a/tools/python/xen/util/xmlrpclib2.py	Mon Mar 27 10:16:36 2006
> +++ b/tools/python/xen/util/xmlrpclib2.py	Tue Mar 28 15:02:39 2006
> @@ -15,15 +15,45 @@
>  # Copyright (C) 2006 Anthony Liguori <aliguori@us.ibm.com>
>  # Copyright (C) 2006 XenSource Ltd.
> 
#===========================================================================>
+#
> +# This file contains portions of the xmlrpclib.py that is distributed with
> +# Python.  Those portions are:
> +#
> +# Copyright (c) 1999-2002 by Secret Labs AB
> +# Copyright (c) 1999-2002 by Fredrik Lundh
> +#
> +# By obtaining, using, and/or copying this software and/or its
> +# associated documentation, you agree that you have read, understood,
> +# and will comply with the following terms and conditions:
> +#
> +# Permission to use, copy, modify, and distribute this software and
> +# its associated documentation for any purpose and without fee is
> +# hereby granted, provided that the above copyright notice appears in
> +# all copies, and that both that copyright notice and this permission
> +# notice appear in supporting documentation, and that the name of
> +# Secret Labs AB or the author not be used in advertising or publicity
> +# pertaining to distribution of the software without specific, written
> +# prior permission.
> +#
> +# SECRET LABS AB AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH REGARD
> +# TO THIS SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANT-
> +# ABILITY AND FITNESS.  IN NO EVENT SHALL SECRET LABS AB OR THE AUTHOR
> +# BE LIABLE FOR ANY SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY
> +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
> +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
> +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> +# OF THIS SOFTWARE.
> +# --------------------------------------------------------------------
>  
>  """
>  An enhanced XML-RPC client/server interface for Python.
>  """
>  
>  from httplib import HTTPConnection, HTTP
> -from xmlrpclib import Transport
> +from xmlrpclib import Transport, Fault
>  from SimpleXMLRPCServer import SimpleXMLRPCServer,
SimpleXMLRPCRequestHandler
> -import xmlrpclib, socket, os, traceback
> +import xmlrpclib, socket, os, traceback, string
> +from types import *
>  
>  # A new ServerProxy that also supports httpu urls.  An http URL comes in
the
>  # form:
> @@ -31,6 +61,9 @@
>  # httpu:///absolute/path/to/socket.sock
>  #
>  # It assumes that the RPC handler is /RPC2.  This probably needs to be
improved
> +
> +MAXINT =  2L**31-1
> +MININT = -2L**31
>  
>  class HTTPUnixConnection(HTTPConnection):
>      def connect(self):
> @@ -58,6 +91,67 @@
>          xmlrpclib.ServerProxy.__init__(self, uri, transport, encoding,
>                                         verbose, allow_none)
>  
> +class Marshaller(xmlrpclib.Marshaller):
> +    def __init__(self, encoding=None, allow_none=0):
> +        xmlrpclib.Marshaller.__init__(self, encoding, allow_none)
> +        self.dispatch[IntType] = Marshaller.dump_int
> +        self.dispatch[LongType] = Marshaller.dump_long
> +        
> +    def dump_int(self, value, write):
> +        if value > MAXINT or value < MININT:
> +            self.dispatch[StringType](self, str(value), write)
> +        else:
> +            xmlrpclib.Marshaller.dump_int(self, value, write)
> +
> +    def dump_long(self, value, write):
> +        if value > MAXINT or value < MININT:
> +            self.dispatch[StringType](self, str(value), write)
> +        else:
> +            xmlrpclib.Marshaller.dump_long(self, value, write)
> +
> +def dumps(params, methodname=None, methodresponse=None, encoding=None,
> +          allow_none=0):
> +    assert isinstance(params, TupleType) or isinstance(params, Fault),\
> +           "argument must be tuple or Fault instance"
> +
> +    if isinstance(params, Fault):
> +        methodresponse = 1
> +    elif methodresponse and isinstance(params, TupleType):
> +        assert len(params) == 1, "response tuple must be a
singleton"
> +
> +    if not encoding:
> +        encoding = "utf-8"
> +
> +    m = Marshaller(encoding, allow_none)
> +
> +    data = m.dumps(params)
> +
> +    if encoding != "utf-8":
> +        xmlheader = "<?xml version=''1.0''
encoding=''%s''?>\n" % str(encoding)
> +    else:
> +        xmlheader = "<?xml
version=''1.0''?>\n" # utf-8 is default
> +
> +    if methodname:
> +        if not isinstance(methodname, StringType):
> +            methodname = methodname.encode(encoding)
> +        data = (
> +            xmlheader,
> +            "<methodCall>\n"
> +            "<methodName>", methodname,
"</methodName>\n",
> +            data,
> +            "</methodCall>\n"
> +            )
> +    elif methodresponse:
> +        data = (
> +            xmlheader,
> +            "<methodResponse>\n",
> +            data,
> +            "</methodResponse>\n"
> +            )
> +    else:
> +        return data
> +    return string.join(data, "")
> +
>  # This is a base XML-RPC server for TCP.  It sets allow_reuse_address to
>  # true, and has an improved marshaller that serializes unknown exceptions
>  # with full traceback information.
> @@ -74,13 +168,13 @@
>                  response = self._dispatch(method, params)
>  
>              response = (response,)
> -            response = xmlrpclib.dumps(response,
> -                                       methodresponse=1,
> -                                       allow_none=1)
> +            response = dumps(response,
> +                             methodresponse=1,
> +                             allow_none=1)
>          except xmlrpclib.Fault, fault:
> -            response = xmlrpclib.dumps(fault)
> +            response = dumps(fault)
>          except:
> -            response = xmlrpclib.dumps(
> +            response = dumps(
>                  xmlrpclib.Fault(1, traceback.format_exc())
>                  )
>  
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>   

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> This is a pretty big hack which based on Aravindh''s note may not
be as
> important as I thought it was.  Aravindh bumped NR_CPUS which
increased
> the size of vcpu_avail.  I''ve audited the interface and the only
place
> that large ints can be passed around are in the S-Expressions.  The
> S-Expressions are untyped so the user always has to manually do type
> conversion.  Passing the S-Expressions as strings is always safe.
I changed the DomU NR_CPUS to 8 and tried "xm list" without your
patch.
I still hit the issue. I think it has to do with the fact that we
running on a 32-way system. So this is going to be an issue on our big
ES7000 systems.

Aravindh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel