> > We would like to simplify Xend by utilizing more of the standard > > Python library and relying on less of our own code. The > most obvious > > thing here is the current S-Expression/HTTP RPC interface. > We would > > like to replace this with XML-RPC using Python''s builtin support > > (xmlrpclib and SimpleXMLRPCServer). > > > > I''ve got some initial code and more details on the wiki > > (http://wiki.xensource.com/xenwiki/Xend/XML-RPC). Early > estimates are > > that this would reduce the code in Xend by about 33% (5k > slocs). We > > would also like to standardize this XML-RPC interface so that > > third-parties write apps to this interface without worrying about > > massive breakage.Anothony, have you an updated version of this patch? How much testing has it had? Should we be looking to get it in prior to 3.0.2? In principle I think this would be a good thing, but we may have left it too late. I expect 2.6.16 final will be out fairly soon, and then we''ll want to be pushing a release out in the following week or so. Thanks, Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Pratt wrote:>>> would also like to standardize this XML-RPC interface so that >>> third-parties write apps to this interface without worrying about >>> massive breakage. >>> > > Anothony, have you an updated version of this patch? How much testing > has it had? >Yes, I sent those to xen-devel just the other day. They are attached to this note for your convenience. I''ve done a bunch of testing (via xm-test) on x86-32 and limited on x86-64.> Should we be looking to get it in prior to 3.0.2?I think the patch that adds an XML-RPC interface to Xend ought to go in. It doesn''t change any of the normally executed code paths for xm and only listens on a domain socket. This would give us a chance to start supporting XML-RPC in libvirt and to bang on the interface a little more. If 3.0.2 is going out shortly, I''d say hold off on the second patch until after it goes out. There''s no real benefit with the second patch other than getting more people to exercise the code so it''s probably not worth taking the risk (although as I said, I''ve tested it pretty heavily).> In principle I think > this would be a good thing, but we may have left it too late. I expect > 2.6.16 final will be out fairly soon, and then we''ll want to be pushing > a release out in the following week or so. >I split up the patches specifically with this in mind. The first one should be very low-risk. Regards, Anthony Liguori> Thanks, > Ian >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony, I''ve reviewed your XML-RPC patches. It all looks good, so I''m in favour of putting them in straight away. I''d like to get the second patch in straight away as well as the first, that is (for those watching at home) to change xm to using XML-RPC, because I would like to deprecate the old protocol sooner, rather than later. Making the XML-RPC interface the primary control path for Xen 3.0.2 will encourage third-parties to code to that rather than to the s-expression protocol, and that''s a good thing. Here are some specific comments. There''s quite a lot here, but nothing that would prevent both patches going in. We can discuss these things, and if we''re still in disagreement or regard my suggestions as high risk, then we can put in the two patches as they stand now.> # HG changeset patch > # User anthony@rhesis.austin.ibm.com > # Node ID 095ac0d95d9cc154ec8fc3dba1a67f02f79771ac > # Parent c369d960f96ba4fd7c9c6920cfa60c46a764323c > Add an XML-RPC interface to Xend. > > This introduces a enhanced client library (xmlrpclib2) that supports XML-RPC > over a domain socket (also working around a bug in Python''s handling of NIL). > > This also introduces a new server that runs along side of the existing > S-Expression/HTTP server. > > This changeset makes no change to the normal operation of Xend. xm still goes > through S-Expression/HTTP -- there''s just another option for interacting with > Xend. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/xend/server/SrvServer.py > --- a/tools/python/xen/xend/server/SrvServer.py Tue Feb 28 16:45:20 2006 > +++ b/tools/python/xen/xend/server/SrvServer.py Tue Feb 28 22:08:47 2006 > @@ -51,6 +51,7 @@ > from xen.web.SrvDir import SrvDir > > from SrvRoot import SrvRoot > +from XMLRPCServer import XMLRPCServer > > > xroot = XendRoot.instance() > @@ -113,4 +114,5 @@ > path = xroot.get_xend_unix_path() > log.info(''unix path='' + path) > servers.add(UnixHttpServer(path=path, root=root)) > + servers.add(XMLRPCServer()) > return serversIt would be prudent to make it possible to turn the XMLRPCServer off, just as we can turn the other servers off.> diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/util/xmlrpclib2.py > --- /dev/null Tue Feb 28 16:45:20 2006 > +++ b/tools/python/xen/util/xmlrpclib2.py Tue Feb 28 22:08:47 2006 > > [Snip lots] > > +class ServerProxy(xmlrpclib.ServerProxy): > + def __init__(self, uri, transport=None, encoding=None, verbose=0, > + allow_none=1): > + if transport == None: > + protocol = uri.split('':'')[0] > + if protocol == ''httpu'': > + uri = ''http:'' + '':''.join(uri.split('':'')[1:]) > + transport = UnixTransport()How about (protocol, rest) = uri.split('':'', 1) if protocol == ''httpu'': uri = ''http:'' + rest transport = UnixTransport()> > [Snip lots more] > > diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/xend/server/XMLRPCServer.py > --- /dev/null Tue Feb 28 16:45:20 2006 > +++ b/tools/python/xen/xend/server/XMLRPCServer.py Tue Feb 28 22:08:47 2006 > @@ -0,0 +1,97 @@ > +#===========================================================================> +# This library is free software; you can redistribute it and/or > +# modify it under the terms of version 2.1 of the GNU Lesser General Public > +# License as published by the Free Software Foundation. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# License along with this library; if not, write to the Free Software > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > +#===========================================================================> +# Copyright (C) 2006 Anthony Liguori <aliguori@us.ibm.com> > +#===========================================================================> + > +from xen.xend import (XendDomain, XendDomainInfo, XendNode, > + XendLogging, XendDmesg)This syntax is only in Python 2.4+, so we have to use from xen.xend import XendDomain, XendDomainInfo, XendNode, \ XendLogging, XendDmesg> [Snip] > > +def get_log(): > + f = open(XendLogging.getLogFilename(), ''r'') > + ret = f.read() > + f.close() > + return retThis will leak a file descriptor if f.read() throws an exception. We need f = open(XendLogging.getLogFIlename(), ''r'') try: return f.read() finally: f.close()> +methods = [''device_create'', ''destroyDevice'', ''getDeviceSxprs'', > + ''setMemoryTarget'', ''setName'', ''setVCpuCount'', ''shutdown'', > + ''send_sysrq'', ''getVCPUInfo'', ''waitForDevices''] > + > +exclude = [''domain_create'', ''domain_restore''] > + > +class XMLRPCServer: > + def __init__(self): > + self.ready = False > + > + def run(self): > + self.server = UnixXMLRPCServer("/var/run/xend-xmlrpc.sock")This filename constant needs to go somewhere else; XendClient is probably the best place for it. (I''ve never liked the way that files in xen/xm have to go poking around in xen/xend for things that they need, but I don''t have the stomach for a major file rearrangement at the moment, so XendClient will have to do.)> + # Functions in XendDomainInfo > + for name in methods: > + fn = eval("lambda domid, *args: dispatch(domid, ''%s'', > args)"%name) > + self.server.register_function(fn, "xend.domain.%s" % > name) > + > + # Functions in XendDomain > + inst = XendDomain.instance() > + for name in dir(inst): > + fn = getattr(inst, name) > + if name.startswith("domain_") and callable(fn): > + if name not in exclude: > + self.server.register_function(fn, > "xend.domain.%s" % name[7:]) > + > + # Functions in XendNode and XendDmesg > + for type, lst, n in [(XendNode, [''info'', > ''cpu_bvt_slice_set''], ''node''), > + (XendDmesg, [''info'', ''clear''], > ''node.dmesg'')]: > + inst = type.instance() > + for name in lst: > + self.server.register_function(getattr(inst, name), > + "xend.%s.%s" % (n, > name)) > +This is all a bit skanky, and could be easily cleaned up by introducing a naming convention for XendDomain, XendNode, etc. How about if we prefixed every function that we wish to expose to the messaging layer with "public_"? So for example XendDomainInfo.send_sysrq would be named public_send_sysrq instead. Then, we could use that to guide the function registration, rather than having exclude lists and inline lists of callable methods. This would make your patch a bit more invasive, in that the renaming would cause it to touch more files, but I don''t have a problem with that -- I think the change is justified.> [Snip the rest of this patch] > > # HG changeset patch > # User anthony@rhesis.austin.ibm.com > # Node ID 951f0d589164e0cffc785cf23d82118b3e0b0495 > # Parent 095ac0d95d9cc154ec8fc3dba1a67f02f79771ac > This changeset is a major refactoring of XendClient to use the XML-RPC > transport for Xend and also to make a few changes to xm so that it knows about > the new Exception types. > > xm-test has been passing with this changeset for the past couple of weeks. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xend/XendClient.py > --- a/tools/python/xen/xend/XendClient.py Tue Feb 28 22:08:47 2006 > +++ b/tools/python/xen/xend/XendClient.py Tue Feb 28 22:10:14 2006 > > [Snip lots] > > def xend_node_get_dmesg(self): > - return self.xendGet(self.nodeurl(''dmesg'')) > + return self.srv.xend.node.dmesg.info()What you''ve done here is slipped the new XML-RPC layer under the existing XendClient API. I don''t think that we need to support that API at all. All the functions here just turn into stubs onto ServerProxy, and I don''t think that those stubs buy us anything. What I''d do here is just this: XendClient.py: XEND_XMLRPC_UNIX_SOCKET = ''/var/run/xend-xmlrpc.sock'' server = ServerProxy(''httpu://'' + XEND_XMLRPC_UNIX_SOCKET) and then push _all_ the other code into the client (main.py, create.py, etc). So these would change from from xen.xend.XendClient import server print server.xend_node_get_dmesg() to from xen.xend.XendClient import server print server.xend.node.dmesg.info() XendClient.py just dissolves into nothingness, and the clients are no more complicated.> def xend_domain_sysrq(self, id, key): > - return self.xendPost(self.domainurl(id), > - {''op'' : ''sysrq'', > - ''key'' : key}) > + return self.srv.xend.domain.send_sysrq(self.lookup(id), key)For calls like this, where we currently look up the domain ID before making the real call, I would push the lookup to the server; there''s no need for two roundtrips for this kind of call. I think that if you remove the lookup call, it will just work now in any case, because XMLRPCServer.lookup uses XendDomain.domain_lookup_by_name_or_id.> + # FIXME > + def xend_domain_restore(self, filename): > + return self.srv.xend.domain.restore(filename)What does this FIXME mean?> diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xend/XendDomain.py > --- a/tools/python/xen/xend/XendDomain.py Tue Feb 28 22:08:47 2006 > +++ b/tools/python/xen/xend/XendDomain.py Tue Feb 28 22:10:14 2006 > @@ -385,7 +385,7 @@ > val = dominfo.destroy() > else: > try: > - val = xc.domain_destroy(domid) > + val = xc.domain_destroy(int(domid)) > except Exception, ex: > raise XendError(str(ex)) > return valI can see why you''ve done this, but it just led me to wonder why XendDomain.domain_destroy exists at all. All it''s doing is looking up a domain ID, checking that it''s not the privileged domain (in the wrong order!) calling XendDomainInfo.destroy, and calling xc.domain_destroy in the case of an exception. We should move the check and the exception handling into XendDomainInfo.destroy, and then we can dispatch to that method straight away, without needing XendDomain.domain_destroy at all. The messaging layer already has the capability to lookup domain IDs -- we should use it. The reason I noticed is that I have been trying to make it so that XendDomain and XendDomainInfo only receive arguments of the correct type, with the casts to integers etc handled by the messaging layer. This change you''ve made highlights the fact that there is now nowhere for that type conversion to take place, because the messaging layer is more generic, and doesn''t understand the calls that it is dispatching. That''s OK -- it''s a consequence of the removal of all that code in SrvDomain -- but it''s something to be aware of. If we adopt the naming convention that I suggest above, then all methods accept only arguments of the correct type _except_ for those named "public_xyz" -- those methods must validate and convert their arguments appropriately.> diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xm/create.py > --- a/tools/python/xen/xm/create.py Tue Feb 28 22:08:47 2006 > +++ b/tools/python/xen/xm/create.py Tue Feb 28 22:10:14 2006 > @@ -28,11 +28,9 @@ > import time > import re > > -import xen.lowlevel.xc > - > from xen.xend import sxp > from xen.xend import PrettyPrint > -from xen.xend.XendClient import server, XendError > +from xen.xend.XendClient import server > from xen.xend.XendBootloader import bootloader > from xen.util import blkif > > @@ -416,6 +414,8 @@ > def err(msg): > """Print an error to stderr and exit. > """ > + import traceback > + traceback.print_exc() > print >>sys.stderr, "Error:", msg > sys.exit(1)This looks like debug code -- I''m not even sure how this traceback helps you very much. It should be disabled by default, or removed altogether.> @@ -810,7 +810,7 @@ > dominfo = server.xend_domain_restore(filename, config) > else: > dominfo = server.xend_domain_create(config) > - except XendError, ex: > + except Exception, ex: > import signal > if vncpid: > os.kill(vncpid, signal.SIGKILL) > diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xm/main.py > --- a/tools/python/xen/xm/main.py Tue Feb 28 22:08:47 2006 > +++ b/tools/python/xen/xm/main.py Tue Feb 28 22:10:14 2006 > @@ -29,8 +29,8 @@ > import socket > import warnings > warnings.filterwarnings(''ignore'', category=FutureWarning) > - > -import xen.xend.XendError > +import xmlrpclib > + > import xen.xend.XendProtocol > > from xen.xend import PrettyPrint > @@ -1036,23 +1036,13 @@ > else: > err("Error connecting to xend: %s." % ex[1]) > sys.exit(1) > - except xen.xend.XendError.XendError, ex: > - if len(args) > 0: > - handle_xend_error(argv[1], args, ex) > - else: > - print "Unexpected error:", sys.exc_info()[0] > - print > - print "Please report to xen-devel@lists.xensource.com" > - raise > - except xen.xend.XendProtocol.XendError, ex: > - if len(args) > 0: > - handle_xend_error(argv[1], args, ex) > - else: > - print "Unexpected error:", sys.exc_info()[0] > - print > - print "Please report to xen-devel@lists.xensource.com" > - raise > except SystemExit: > + sys.exit(1) > + except xmlrpclib.Fault, ex: > +# print "Xend generated an internal fault:" > +# sys.stderr.write(ex.faultString) > +# sys.exit(1) > + print "Error: Internal Xend error"I expect we can do better than just printing "Internal Xend Error". How much structure and useful information is there in an xmlrpclib.Fault at the moment?> sys.exit(1) > except: > print "Unexpected error:", sys.exc_info()[0]Looks like that''s it! Thanks for all your hard work, Anthony, this is going to make a big difference to Xend''s usefulness and maintainability, and you''ve done a good job of it. Let''s get it into 3.0.2. Cheers, Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sat, Mar 11, 2006 at 08:55:10PM +0000, Ewan Mellor wrote:> Anthony, I''ve reviewed your XML-RPC patches. It all looks good, so I''m in > favour of putting them in straight away. I''d like to get the second patch in > straight away as well as the first, that is (for those watching at home) to > change xm to using XML-RPC, because I would like to deprecate the old > protocol sooner, rather than later. Making the XML-RPC interface the primary > control path for Xen 3.0.2 will encourage third-parties to code to that rather > than to the s-expression protocol, and that''s a good thing.+1 we will try to switch ASAP too at the libvirt level.> It would be prudent to make it possible to turn the XMLRPCServer off, just as we > can turn the other servers off.Big dilemna :-) Currently with http on anybody can manipulate any Xen instance as soon as one has Xen0 access (I run my libvirt regression test from my normal user login but except for the convenience it''s a bit scary :-). I assume It will be exactly the same with XML-RPC. I would feel way better if we could have a policy model which is not all on or all off implemented, for example associating users with domains, and controlling per user resource usage. I know that the obvious answer would be "only root should do that" except that a frustated user needing his VM could then launch a qemu instance instead and that wouldn''t be any better for the server. IMHO a smarter, more fine grained approach is needed, it will also require authentication at connection time to the service, and even if not trivial this should be doable and we use authenticated XML-RPC call all the time for the Red Hat Network framework, so this has to be possible locally and remotely.> > diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/util/xmlrpclib2.py > > --- /dev/null Tue Feb 28 16:45:20 2006 > > +++ b/tools/python/xen/util/xmlrpclib2.py Tue Feb 28 22:08:47 2006 > > > > [Snip lots] > > > > +class ServerProxy(xmlrpclib.ServerProxy): > > + def __init__(self, uri, transport=None, encoding=None, verbose=0, > > + allow_none=1): > > + if transport == None: > > + protocol = uri.split('':'')[0] > > + if protocol == ''httpu'': > > + uri = ''http:'' + '':''.join(uri.split('':'')[1:]) > > + transport = UnixTransport() > > How about > > (protocol, rest) = uri.split('':'', 1) > if protocol == ''httpu'': > uri = ''http:'' + rest > transport = UnixTransport()hum, do we really need to invent a new transport for local access ? I don''t remember seeing ''httpu'' anywhere, sounds weird to me [...]> > except SystemExit: > > + sys.exit(1) > > + except xmlrpclib.Fault, ex: > > +# print "Xend generated an internal fault:" > > +# sys.stderr.write(ex.faultString) > > +# sys.exit(1) > > + print "Error: Internal Xend error" > > I expect we can do better than just printing "Internal Xend Error". How > much structure and useful information is there in an xmlrpclib.Fault at > the moment?Another good question, as we are defining an API to Xend I think the error handling question will become more and more important. The client will need more structure and informations about processing error, the full stack trace within xend might not be that useful (except for low level debugging) to the clients, but currently I just extract a string like "No such domain test" which is a bit hard to process correctly even in context. A list of predefined error code and meaning could help quite a bit along with just the error message.> > sys.exit(1) > > except: > > print "Unexpected error:", sys.exc_info()[0] > > Looks like that''s it! Thanks for all your hard work, Anthony, this is > going to make a big difference to Xend''s usefulness and maintainability, > and you''ve done a good job of it. Let''s get it into 3.0.2.Yup, thanks Anthony :-) ! Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sat, Mar 11, 2006 at 04:36:57PM -0500, Daniel Veillard wrote:> > It would be prudent to make it possible to turn the XMLRPCServer off, just as we > > can turn the other servers off. > > Big dilemna :-) > Currently with http on anybody can manipulate any Xen instance as soon > as one has Xen0 access (I run my libvirt regression test from my normal > user login but except for the convenience it''s a bit scary :-). I assume > It will be exactly the same with XML-RPC. I would feel way better if we > could have a policy model which is not all on or all off implemented, > for example associating users with domains, and controlling per user > resource usage. I know that the obvious answer would be "only root should > do that" except that a frustated user needing his VM could then launch > a qemu instance instead and that wouldn''t be any better for the server. > IMHO a smarter, more fine grained approach is needed, it will also require > authentication at connection time to the service, and even if not trivial > this should be doable and we use authenticated XML-RPC call all the time > for the Red Hat Network framework, so this has to be possible locally and > remotely.Turning the server on and off is just for basic sanity -- the admin gets to choose which ports they want open so that they don''t have to audit aspects in which they aren''t interested. Of course, a full user / permissions system for the protocol would be a good idea, but like you say, it''s not trivial work. We could kick that discussion off if you want, but it''s going to need someone to design the permissions semantics, management of users and roles, etc. Is anyone interested in starting this? At the moment, the XML-RPC is over a Unix domain socket, so only root can use it anyway (as controlled by the permission on the socket file).> > > diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/util/xmlrpclib2.py > > > --- /dev/null Tue Feb 28 16:45:20 2006 > > > +++ b/tools/python/xen/util/xmlrpclib2.py Tue Feb 28 22:08:47 2006 > > > > > > [Snip lots] > > > > > > +class ServerProxy(xmlrpclib.ServerProxy): > > > + def __init__(self, uri, transport=None, encoding=None, verbose=0, > > > + allow_none=1): > > > + if transport == None: > > > + protocol = uri.split('':'')[0] > > > + if protocol == ''httpu'': > > > + uri = ''http:'' + '':''.join(uri.split('':'')[1:]) > > > + transport = UnixTransport() > > > > How about > > > > (protocol, rest) = uri.split('':'', 1) > > if protocol == ''httpu'': > > uri = ''http:'' + rest > > transport = UnixTransport() > > hum, do we really need to invent a new transport for local access ? > I don''t remember seeing ''httpu'' anywhere, sounds weird to mehttpu is HTTP over a unix domain socket, as opposed to over TCP. It''s used elsewhere (though not widely, obviously). It gives you basic protection from non-root users (see your complaint above).> [...] > > > except SystemExit: > > > + sys.exit(1) > > > + except xmlrpclib.Fault, ex: > > > +# print "Xend generated an internal fault:" > > > +# sys.stderr.write(ex.faultString) > > > +# sys.exit(1) > > > + print "Error: Internal Xend error" > > > > I expect we can do better than just printing "Internal Xend Error". How > > much structure and useful information is there in an xmlrpclib.Fault at > > the moment? > > Another good question, as we are defining an API to Xend I think > the error handling question will become more and more important. The client > will need more structure and informations about processing error, the > full stack trace within xend might not be that useful (except for low level > debugging) to the clients, but currently I just extract a string like > "No such domain test" > which is a bit hard to process correctly even in context. A list of predefined > error code and meaning could help quite a bit along with just the error message.Sure. Do you have a list of error codes already (for libvirt, for example)? Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ewan Mellor wrote:> Anthony, I''ve reviewed your XML-RPC patches. It all looks good, so I''m in > favour of putting them in straight away.Thanks!> I''d like to get the second patch in > straight away as well as the first, that is (for those watching at home) to > change xm to using XML-RPC, because I would like to deprecate the old > protocol sooner, rather than later. Making the XML-RPC interface the primary > control path for Xen 3.0.2 will encourage third-parties to code to that rather > than to the s-expression protocol, and that''s a good thing. >Okay. So I''m going to take this to mean that we would deprecate S-Expression/HTTP for 3.0.2 and remove it in 3.0.3? That would be really nice.>> xroot = XendRoot.instance() >> @@ -113,4 +114,5 @@ >> path = xroot.get_xend_unix_path() >> log.info(''unix path='' + path) >> servers.add(UnixHttpServer(path=path, root=root)) >> + servers.add(XMLRPCServer()) >> return servers >> > > It would be prudent to make it possible to turn the XMLRPCServer off, just as we > can turn the other servers off. >Yeah, this is certainly sane.>> +class ServerProxy(xmlrpclib.ServerProxy): >> + def __init__(self, uri, transport=None, encoding=None, verbose=0, >> + allow_none=1): >> + if transport == None: >> + protocol = uri.split('':'')[0] >> + if protocol == ''httpu'': >> + uri = ''http:'' + '':''.join(uri.split('':'')[1:]) >> + transport = UnixTransport() >> > > How about > > (protocol, rest) = uri.split('':'', 1) > if protocol == ''httpu'': > uri = ''http:'' + rest > transport = UnixTransport() >Haven''t seen that before. Quite useful :-)>> +from xen.xend import (XendDomain, XendDomainInfo, XendNode, >> + XendLogging, XendDmesg) >> > > This syntax is only in Python 2.4+, so we have to use > > from xen.xend import XendDomain, XendDomainInfo, XendNode, \ > XendLogging, XendDmesg >Yeah, thanks for the catch.>> [Snip] >> >> +def get_log(): >> + f = open(XendLogging.getLogFilename(), ''r'') >> + ret = f.read() >> + f.close() >> + return ret >> > > This will leak a file descriptor if f.read() throws an exception. We need > > f = open(XendLogging.getLogFIlename(), ''r'') > try: > return f.read() > finally: > f.close() >Yeah, again, thanks for the catch :-)>> + def run(self): >> + self.server = UnixXMLRPCServer("/var/run/xend-xmlrpc.sock") >> > > This filename constant needs to go somewhere else; XendClient is probably the > best place for it. (I''ve never liked the way that files in xen/xm have to go > poking around in xen/xend for things that they need, but I don''t have the > stomach for a major file rearrangement at the moment, so XendClient will have > to do.) >Okay. I''m fine with that.>> + # Functions in XendNode and XendDmesg >> + for type, lst, n in [(XendNode, [''info'', >> ''cpu_bvt_slice_set''], ''node''), >> + (XendDmesg, [''info'', ''clear''], >> ''node.dmesg'')]: >> + inst = type.instance() >> + for name in lst: >> + self.server.register_function(getattr(inst, name), >> + "xend.%s.%s" % (n, >> name)) >> + >> > > This is all a bit skanky, and could be easily cleaned up by introducing a > naming convention for XendDomain, XendNode, etc.Yes, skanky is very much an understatement. I found XendDomain and XendDomainInfo particularly nasty to work with considering that they use different naming conventions too.> How about if we prefixed > every function that we wish to expose to the messaging layer with > "public_"? So for example XendDomainInfo.send_sysrq would be named > public_send_sysrq instead. Then, we could use that to guide the > function registration, rather than having exclude lists and inline lists > of callable methods. >Isn''t using an underscore a convention for making methods private in Python? I think, at least, pydoc ignores functions that start with an underscore.> This would make your patch a bit more invasive, in that the renaming > would cause it to touch more files, but I don''t have a problem with that > -- I think the change is justified. >Yes, that would make things much nicer so I''m happy to do it. I clearly wanted to avoid making more changes than necessary at first :-)>> [Snip the rest of this patch] >> >> # HG changeset patch >> # User anthony@rhesis.austin.ibm.com >> # Node ID 951f0d589164e0cffc785cf23d82118b3e0b0495 >> # Parent 095ac0d95d9cc154ec8fc3dba1a67f02f79771ac >> This changeset is a major refactoring of XendClient to use the XML-RPC >> transport for Xend and also to make a few changes to xm so that it knows about >> the new Exception types. >> >> xm-test has been passing with this changeset for the past couple of weeks. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> >> diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xend/XendClient.py >> --- a/tools/python/xen/xend/XendClient.py Tue Feb 28 22:08:47 2006 >> +++ b/tools/python/xen/xend/XendClient.py Tue Feb 28 22:10:14 2006 >> >> [Snip lots] >> >> def xend_node_get_dmesg(self): >> - return self.xendGet(self.nodeurl(''dmesg'')) >> + return self.srv.xend.node.dmesg.info() >> > > What you''ve done here is slipped the new XML-RPC layer under the existing > XendClient API. I don''t think that we need to support that API at all. All > the functions here just turn into stubs onto ServerProxy, and I don''t think > that those stubs buy us anything. >Nope. Just compatibility. I know there are people using the XendClient API. I also know we''ve never declared that to be a fixed API. I didn''t want to be the one to break it though :-)> What I''d do here is just this: > > XendClient.py: > > XEND_XMLRPC_UNIX_SOCKET = ''/var/run/xend-xmlrpc.sock'' > > server = ServerProxy(''httpu://'' + XEND_XMLRPC_UNIX_SOCKET) > > and then push _all_ the other code into the client (main.py, create.py, etc). > So these would change from > > from xen.xend.XendClient import server > print server.xend_node_get_dmesg() > > to > > from xen.xend.XendClient import server > print server.xend.node.dmesg.info() > > XendClient.py just dissolves into nothingness, and the clients are no more > complicated. >Yes, this would be great. If we don''t mind breaking XendClient, I''m happy to do it.>> def xend_domain_sysrq(self, id, key): >> - return self.xendPost(self.domainurl(id), >> - {''op'' : ''sysrq'', >> - ''key'' : key}) >> + return self.srv.xend.domain.send_sysrq(self.lookup(id), key) >> > > For calls like this, where we currently look up the domain ID before making the > real call, I would push the lookup to the server; there''s no need for two > roundtrips for this kind of call. I think that if you remove the lookup call, > it will just work now in any case, because XMLRPCServer.lookup uses > XendDomain.domain_lookup_by_name_or_id. >Yeah, lookup() is a bit of a hack. I can go through and audit the functions to make sure that they all accept either names or IDs.>> + # FIXME >> + def xend_domain_restore(self, filename): >> + return self.srv.xend.domain.restore(filename) >> > > What does this FIXME mean? >Sorry, that shouldn''t have been there. I already fixed that one :-)>> diff -r 095ac0d95d9c -r 951f0d589164 tools/python/xen/xend/XendDomain.py >> --- a/tools/python/xen/xend/XendDomain.py Tue Feb 28 22:08:47 2006 >> +++ b/tools/python/xen/xend/XendDomain.py Tue Feb 28 22:10:14 2006 >> @@ -385,7 +385,7 @@ >> val = dominfo.destroy() >> else: >> try: >> - val = xc.domain_destroy(domid) >> + val = xc.domain_destroy(int(domid)) >> except Exception, ex: >> raise XendError(str(ex)) >> return val >> > > I can see why you''ve done this, but it just led me to wonder why > XendDomain.domain_destroy exists at all. All it''s doing is looking up a > domain ID, checking that it''s not the privileged domain (in the wrong > order!) calling XendDomainInfo.destroy, and calling xc.domain_destroy in > the case of an exception. We should move the check and the exception > handling into XendDomainInfo.destroy, and then we can dispatch to that > method straight away, without needing XendDomain.domain_destroy at all. > The messaging layer already has the capability to lookup domain IDs -- > we should use it. >Ok, sounds sane to me.> The reason I noticed is that I have been trying to make it so that > XendDomain and XendDomainInfo only receive arguments of the correct type, > with the casts to integers etc handled by the messaging layer. This > change you''ve made highlights the fact that there is now nowhere for > that type conversion to take place, because the messaging layer is more > generic, and doesn''t understand the calls that it is dispatching. > That''s OK -- it''s a consequence of the removal of all that code in > SrvDomain -- but it''s something to be aware of. If we adopt the naming > convention that I suggest above, then all methods accept only arguments > of the correct type _except_ for those named "public_xyz" -- those > methods must validate and convert their arguments appropriately. >Why? If a public_ function expects a domid and gets passed a string, it ought to throw an exception and the client should get it as an error. This particular hack was to account for a place in xm where it was passing a string version of the domain ID. The real problem here was the xm code. I wanted to avoid modifying that code though to demonstrate that the XML-RPC stuff could be a drop-in-replacement. Of course, now that that exercise is complete, there''s no reason not to go in and fix the original source of the problem.>> @@ -416,6 +414,8 @@ >> def err(msg): >> """Print an error to stderr and exit. >> """ >> + import traceback >> + traceback.print_exc() >> print >>sys.stderr, "Error:", msg >> sys.exit(1) >> > > This looks like debug code -- I''m not even sure how this traceback helps > you very much. It should be disabled by default, or removed altogether. >Yeah, sorry, I missed that.>> from xen.xend import PrettyPrint >> @@ -1036,23 +1036,13 @@ >> else: >> err("Error connecting to xend: %s." % ex[1]) >> sys.exit(1) >> - except xen.xend.XendError.XendError, ex: >> - if len(args) > 0: >> - handle_xend_error(argv[1], args, ex) >> - else: >> - print "Unexpected error:", sys.exc_info()[0] >> - print >> - print "Please report to xen-devel@lists.xensource.com" >> - raise >> - except xen.xend.XendProtocol.XendError, ex: >> - if len(args) > 0: >> - handle_xend_error(argv[1], args, ex) >> - else: >> - print "Unexpected error:", sys.exc_info()[0] >> - print >> - print "Please report to xen-devel@lists.xensource.com" >> - raise >> except SystemExit: >> + sys.exit(1) >> + except xmlrpclib.Fault, ex: >> +# print "Xend generated an internal fault:" >> +# sys.stderr.write(ex.faultString) >> +# sys.exit(1) >> + print "Error: Internal Xend error" >> > > I expect we can do better than just printing "Internal Xend Error". How > much structure and useful information is there in an xmlrpclib.Fault at > the moment? >We can pass whatever we want. xmlrpclib.Fault contain an integer code and a string message. For now, I''m just always passing a 1 for the faultCode and sending the Xend traceback as the faultMessage. I''m not sure how fancy we want to get for the first drop of this code. Any suggestions? BTW, it''s printing ''Internal Xend Error'' because xm-test has a test case that checks for that specific result :-)> >> sys.exit(1) >> except: >> print "Unexpected error:", sys.exc_info()[0] >> > > Looks like that''s it! Thanks for all your hard work, Anthony, this is > going to make a big difference to Xend''s usefulness and maintainability, > and you''ve done a good job of it. Let''s get it into 3.0.2. >Thanks for looking at the code! I don''t think there''s that much change necessary. I should be able to turn around another patch in a couple days. Regards, Anthony Liguori> Cheers, > > Ewan. >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ewan Mellor wrote:> On Sat, Mar 11, 2006 at 04:36:57PM -0500, Daniel Veillard wrote: > > Turning the server on and off is just for basic sanity -- the admin gets > to choose which ports they want open so that they don''t have to audit > aspects in which they aren''t interested. > > Of course, a full user / permissions system for the protocol would be a > good idea, but like you say, it''s not trivial work. We could kick that > discussion off if you want, but it''s going to need someone to design > the permissions semantics, management of users and roles, etc. Is > anyone interested in starting this? >We can do basic authentication and require root credentials as a start. That''s really easy to do but I''m not sure sending the root password as clear text really makes things much better :-) Python, unfortunately, doesn''t have server-side SSL bindings so basic authentication over SSL is not an option without some custom OpenSSL bindings. Regards, Anthony Liguori> At the moment, the XML-RPC is over a Unix domain socket, so only root > can use it anyway (as controlled by the permission on the socket file). > > >>>> diff -r c369d960f96b -r 095ac0d95d9c tools/python/xen/util/xmlrpclib2.py >>>> --- /dev/null Tue Feb 28 16:45:20 2006 >>>> +++ b/tools/python/xen/util/xmlrpclib2.py Tue Feb 28 22:08:47 2006 >>>> >>>> [Snip lots] >>>> >>>> +class ServerProxy(xmlrpclib.ServerProxy): >>>> + def __init__(self, uri, transport=None, encoding=None, verbose=0, >>>> + allow_none=1): >>>> + if transport == None: >>>> + protocol = uri.split('':'')[0] >>>> + if protocol == ''httpu'': >>>> + uri = ''http:'' + '':''.join(uri.split('':'')[1:]) >>>> + transport = UnixTransport() >>>> >>> How about >>> >>> (protocol, rest) = uri.split('':'', 1) >>> if protocol == ''httpu'': >>> uri = ''http:'' + rest >>> transport = UnixTransport() >>> >> hum, do we really need to invent a new transport for local access ? >> I don''t remember seeing ''httpu'' anywhere, sounds weird to me >> > > httpu is HTTP over a unix domain socket, as opposed to over TCP. It''s > used elsewhere (though not widely, obviously). It gives you basic > protection from non-root users (see your complaint above). > > >> [...] >> >>>> except SystemExit: >>>> + sys.exit(1) >>>> + except xmlrpclib.Fault, ex: >>>> +# print "Xend generated an internal fault:" >>>> +# sys.stderr.write(ex.faultString) >>>> +# sys.exit(1) >>>> + print "Error: Internal Xend error" >>>> >>> I expect we can do better than just printing "Internal Xend Error". How >>> much structure and useful information is there in an xmlrpclib.Fault at >>> the moment? >>> >> Another good question, as we are defining an API to Xend I think >> the error handling question will become more and more important. The client >> will need more structure and informations about processing error, the >> full stack trace within xend might not be that useful (except for low level >> debugging) to the clients, but currently I just extract a string like >> "No such domain test" >> which is a bit hard to process correctly even in context. A list of predefined >> error code and meaning could help quite a bit along with just the error message. >> > > Sure. Do you have a list of error codes already (for libvirt, for > example)? > > Ewan. >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sat, Mar 11, 2006 at 07:46:35PM -0600, Anthony Liguori wrote:> Ewan Mellor wrote: > >On Sat, Mar 11, 2006 at 04:36:57PM -0500, Daniel Veillard wrote: > > > >Turning the server on and off is just for basic sanity -- the admin gets > >to choose which ports they want open so that they don''t have to audit > >aspects in which they aren''t interested. > > > >Of course, a full user / permissions system for the protocol would be a > >good idea, but like you say, it''s not trivial work. We could kick that > >discussion off if you want, but it''s going to need someone to design > >the permissions semantics, management of users and roles, etc. Is > >anyone interested in starting this? > > > We can do basic authentication and require root credentials as a start. > That''s really easy to do but I''m not sure sending the root password as > clear text really makes things much better :-) > > Python, unfortunately, doesn''t have server-side SSL bindings so basic > authentication over SSL is not an option without some custom OpenSSL > bindings.Hum, I could check how much "custom" is required, I remember doing that a long time ago but I could ask around :-) if interested. Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sat, Mar 11, 2006 at 10:20:53PM +0000, Ewan Mellor wrote:> Of course, a full user / permissions system for the protocol would be a > good idea, but like you say, it''s not trivial work. We could kick that > discussion off if you want, but it''s going to need someone to design > the permissions semantics, management of users and roles, etc. Is > anyone interested in starting this?We need to make sure we can have authentication based on the transport used, otherwise this need to be added at the RPC level (and hence changes the API).> At the moment, the XML-RPC is over a Unix domain socket, so only root > can use it anyway (as controlled by the permission on the socket file).To me that''s a big regression. That mean libvirt can''t be used anymore to just monitor the Xen instance(s) without priviledged access. Monitoring should not require root priviledge IMHO, ps aux and top are not restricted to root on Unix. And if you make Xen virtualization a sysadmin only feature you restrain massively the area of usage IMHO, people will just use something else (as stated QEmu requires no sysadmin priviledge, and if using Xen can''t be integrated seamlessly in the user workflow, they will switch to something else). Well over (modern) unix socket one can extract the UID of the connecting client, can we extract that from Python ? (c.f. LOCAL_CREDS/SO_PEERCRED) If yes then that''s a good first step toward local authentication without messing with https and credentials.> httpu is HTTP over a unix domain socket, as opposed to over TCP. It''s > used elsewhere (though not widely, obviously). It gives you basic > protection from non-root users (see your complaint above).It''s not a complaint, I''m raising a problem, which IMHO is different. Adding core piece of OS machinery without thinking of security and authentication aspects from the start is a mistake that had been done on Unix already, it would be great to not go though the mess again just because we are eager to push a solution, the subsequent costs could reduce the benefits from the effort getting there ;-)> > > I expect we can do better than just printing "Internal Xend Error". How > > > much structure and useful information is there in an xmlrpclib.Fault at > > > the moment? > > > > Another good question, as we are defining an API to Xend I think > > the error handling question will become more and more important. The client > > will need more structure and informations about processing error, the > > full stack trace within xend might not be that useful (except for low level > > debugging) to the clients, but currently I just extract a string like > > "No such domain test" > > which is a bit hard to process correctly even in context. A list of predefined > > error code and meaning could help quite a bit along with just the error message. > > Sure. Do you have a list of error codes already (for libvirt, for > example)?Yes but for the moment they can''t capture what''s happening within Xend, so if the RPC fails they show up just as being from within Xend, with either a GET or POST information and the string found in the content of the HTTP answer: http://libvirt.org/html/libvirt-virterror.html#virErrorNumber associated docs: http://libvirt.org/errors.html Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sun, 12 Mar 2006 04:57:02 -0500, Daniel Veillard wrote:> On Sat, Mar 11, 2006 at 10:20:53PM +0000, Ewan Mellor wrote: >> Of course, a full user / permissions system for the protocol would be a >> good idea, but like you say, it''s not trivial work. We could kick that >> discussion off if you want, but it''s going to need someone to design >> the permissions semantics, management of users and roles, etc. Is >> anyone interested in starting this? > > We need to make sure we can have authentication based on the transport > used, otherwise this need to be added at the RPC level (and hence changes > the API).Here''s my proposal: We make a simple program that connects to a domain socket and funnels the stdin/stdout to that socket. One can then build an XML-RPC transport on top of it via SSH. The client side simply invokes ssh and treats it''s stdio as it would a normal socket. I''ve used this approach before with great success. We get PAM authentication for free. An advanced admin can suid that executable to delegate privileges. I still think we ought to have a less privileged socket too but one that''s remotable in a similar manner.>> At the moment, the XML-RPC is over a Unix domain socket, so only root >> can use it anyway (as controlled by the permission on the socket file). > > To me that''s a big regression. That mean libvirt can''t be used anymore > to just monitor the Xen instance(s) without priviledged access.The code is there for TCP. It''s just hard coded to use a domain socket right now. When I make the change Ewan requested to allow it to be enabled/disabled I''ll make it possible to choose the TCP version.> Well over (modern) unix socket one can extract the UID of the > connecting > client, can we extract that from Python ? (c.f. LOCAL_CREDS/SO_PEERCRED) > If yes then that''s a good first step toward local authentication without > messing with https and credentials.Yeah, but that''s a mess and requires specially constructed packets on the client side. I think the ssh tunnel approach is a lot easier. Regards, Anthony Liguori>> httpu is HTTP over a unix domain socket, as opposed to over TCP. It''s >> used elsewhere (though not widely, obviously). It gives you basic >> protection from non-root users (see your complaint above). > > It''s not a complaint, I''m raising a problem, which IMHO is different. > Adding core piece of OS machinery without thinking of security and > authentication aspects from the start is a mistake that had been done on > Unix already, it would be great to not go though the mess again just > because we are eager to push a solution, the subsequent costs could > reduce the benefits from the effort getting there ;-) > >> > > I expect we can do better than just printing "Internal Xend Error". >> > > How much structure and useful information is there in an >> > > xmlrpclib.Fault at the moment? >> > >> > Another good question, as we are defining an API to Xend I think >> > the error handling question will become more and more important. The >> > client will need more structure and informations about processing >> > error, the full stack trace within xend might not be that useful >> > (except for low level debugging) to the clients, but currently I just >> > extract a string like >> > "No such domain test" >> > which is a bit hard to process correctly even in context. A list of >> > predefined error code and meaning could help quite a bit along with >> > just the error message. >> >> Sure. Do you have a list of error codes already (for libvirt, for >> example)? > > Yes but for the moment they can''t capture what''s happening within > Xend, > so if the RPC fails they show up just as being from within Xend, with > either a GET or POST information and the string found in the content of > the HTTP answer: > http://libvirt.org/html/libvirt-virterror.html#virErrorNumber > associated docs: > http://libvirt.org/errors.html > > Daniel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sun, Mar 12, 2006 at 11:41:58AM -0600, Anthony Liguori wrote:> On Sun, 12 Mar 2006 04:57:02 -0500, Daniel Veillard wrote: > > > On Sat, Mar 11, 2006 at 10:20:53PM +0000, Ewan Mellor wrote: > >> Of course, a full user / permissions system for the protocol would be a > >> good idea, but like you say, it''s not trivial work. We could kick that > >> discussion off if you want, but it''s going to need someone to design > >> the permissions semantics, management of users and roles, etc. Is > >> anyone interested in starting this? > > > > We need to make sure we can have authentication based on the transport > > used, otherwise this need to be added at the RPC level (and hence changes > > the API). > > Here''s my proposal: > > We make a simple program that connects to a domain socket and funnels the > stdin/stdout to that socket. > > One can then build an XML-RPC transport on top of it via SSH. The client > side simply invokes ssh and treats it''s stdio as it would a normal socket. > I''ve used this approach before with great success.You mean you''re ready to force 4 context switches to make the RPC because you don''t want to handle authentication at the initialization of the protocol ? I have a hard time thinking it''s a "great" solution. I probably misunderstood something.> We get PAM authentication for free. An advanced admin can suid that > executable to delegate privileges. > > I still think we ought to have a less privileged socket too but one that''s > remotable in a similar manner. > > >> At the moment, the XML-RPC is over a Unix domain socket, so only root > >> can use it anyway (as controlled by the permission on the socket file). > > > > To me that''s a big regression. That mean libvirt can''t be used anymore > > to just monitor the Xen instance(s) without priviledged access. > > The code is there for TCP. It''s just hard coded to use a domain socket > right now. When I make the change Ewan requested to allow it to be > enabled/disabled I''ll make it possible to choose the TCP version.yeah I think I''m lost. I don''t see how you could get to a good solution without separating the various entry points based on different level of credentials.> > Well over (modern) unix socket one can extract the UID of the > > connecting > > client, can we extract that from Python ? (c.f. LOCAL_CREDS/SO_PEERCRED) > > If yes then that''s a good first step toward local authentication without > > messing with https and credentials. > > Yeah, but that''s a mess and requires specially constructed packets on the > client side. I think the ssh tunnel approach is a lot easier.For writing/running a client ? I''m lost, say I want virsh the shell based on libvirt to list the running domains how do you get that working ? virsh is a binary launched from an user shell and linked with libvirt what do you think should happen in this scenario ? Where is the ssh authentication handled ? when does it take place ? I''m lost. Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sun, 12 Mar 2006 13:29:41 -0500, Daniel Veillard wrote:>> One can then build an XML-RPC transport on top of it via SSH. The client >> side simply invokes ssh and treats it''s stdio as it would a normal socket. >> I''ve used this approach before with great success. > > You mean you''re ready to force 4 context switches to make the RPC because > you don''t want to handle authentication at the initialization of the protocol ?The cost of context switching is going to be negligible compared to the cost of encrypting the traffic (not to mention the cost of byte code translation within Xend).> I have a hard time thinking it''s a "great" solution. I probably misunderstood > something.It''s really about where you do policy. Do you do it within Xend, or do you do it outside of Xend. In this case, you would have Xend only accessible to root, and then you have another application with suid privileges that can implement a policy. Those roles are separate and well defined. Using ssh is kind of orthogonal to the above separation so you can think about it independently. The advantage to using ssh as a transport is that it totally punts the problem of authentication/authorization and privacy. It''s easy from an administrative point of view (no new ports need to be opened up) plus it ties into enterprise-identity systems quite well.>> The code is there for TCP. It''s just hard coded to use a domain socket >> right now. When I make the change Ewan requested to allow it to be >> enabled/disabled I''ll make it possible to choose the TCP version. > > yeah I think I''m lost. > I don''t see how you could get to a good solution without separating the > various entry points based on different level of credentials.Yup, let me be a bit more concrete. Let''s say we have xend and xend-remote. Xend is the daemon that listens on a domain socket for XML-RPC. xend-remote is a command line tool that is an XML-RPC proxy. It proxies stdio to a domain socket. This tool can be setup to be SUID in which case it also reads a policy file that in the simplest case, is a white list of which calls can be executed by a particular user/group. libvirt exec()''s a single instance of ssh with the -S option to set it up to be in server mode. This is when authentication occurs. Then, to execute individual XML-RPC calls, libvirt exec()''s ssh (again with -S but this time as a client) which then uses the ssh running in server mode as a proxy. If you didn''t want to go through the hassle of ssh, you could exec() xend-remote directly (assuming you''re only connecting to local host). Then you don''t need any authentication at all. The alternative approach to this would be to have clients to Xend pass SO_CRED stuff directly. Xend would then have to perform PAM authentication (every time too since XML-RPC isn''t stateful). If we wanted privacy, we would have to implement server-side support for SSL. There are advantages to this approach but I think it has the disadvantage of making Xend more complex and having less flexible policy mechanisms. For instance, using xend-remote, the XenSE guys could implement a much more sophisticated proxy that was SSID-aware. If the standard policies we provide didn''t meet an admin''s requirements, she could easily implement a new xend-remote and everything would just work.>> > Well over (modern) unix socket one can extract the UID of the >> > connecting >> > client, can we extract that from Python ? (c.f. LOCAL_CREDS/SO_PEERCRED) >> > If yes then that''s a good first step toward local authentication without >> > messing with https and credentials. >> >> Yeah, but that''s a mess and requires specially constructed packets on the >> client side. I think the ssh tunnel approach is a lot easier. > > For writing/running a client ?Oh, I''m not familiar with SO_PEERCRED. A quick googling seems to suggest it''s Linux specific. The mechanism to authentication over domain sockets that I''m aware of is SCM_CREDS which requires that the client actually send the credential information (via sendmsg).> I''m lost, say I want virsh the shell > based on libvirt to list the running domains how do you get that working ? > virsh is a binary launched from an user shell and linked with libvirt > what do you think should happen in this scenario ? Where is the ssh > authentication handled ? when does it take place ? I''m lost.I think I covered it above. If this is localhost management, virsh can simply exec() xend-remote (once at library initialization) and use that as a proxy SUID proxy to Xend. Does this sound sane? This has been my long term vision for how things ought to work. One could actually implement xend-remote pretty easily right now. Of course, I''m flexible and open to alternatives. Regards, Anthony Liguori> > Daniel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Veillard
2006-Mar-12 21:49 UTC
Re: [Xen-devel] Re: Re: [RFC] Xend XML-RPC Refactoring
On Sun, Mar 12, 2006 at 02:28:24PM -0600, Anthony Liguori wrote:> Does this sound sane? This has been my long term vision for how > things ought to work. One could actually implement xend-remote pretty > easily right now. Of course, I''m flexible and open to alternatives.It''s Sunday, it''s late, I think I understand your viewpoint (especially after the exchange on IRC). Still it decouples completely authentication and right checking from the API. And I feel like we are trying to create a solution which may not be adequate. I really feel of rights over Xen operations to best reflected by tokens or capacities to use the old term. For me to create a domain on a node then you need the capacity for that node, as a result you get a capacity for that domain. Now once you have the capacity for the domain you can pause/unpause/save or reduce its resource allocations. To list domains you don''t need a capacity. To shudown/destroy a domain you need the node or domain capacity. To migrate a domain to a new node you need both the domain and remote node capacities, etc ... So I really think of the authentication and security checkings in a very different model a priori than what you are suggesting, maybe the model I would like to see is just too complex, or doesn''t fit the tools available. That''s probably why using a separate controller which is unlikely to understand the API and auth at the pure connection layer feels strange too me. I find that way too coarse, while at the same time probably expensive to run. I certainly need to think more about this, other should probably tell me how wrong I am too, that should not block going forward with the current plan anyway :-) Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Sat, Mar 11, 2006 at 07:44:20PM -0600, Anthony Liguori wrote:> Ewan Mellor wrote: > >Anthony, I''ve reviewed your XML-RPC patches. It all looks good, so I''m in > >favour of putting them in straight away. > Thanks! > >I''d like to get the second patch in > >straight away as well as the first, that is (for those watching at home) to > >change xm to using XML-RPC, because I would like to deprecate the old > >protocol sooner, rather than later. Making the XML-RPC interface the > >primary > >control path for Xen 3.0.2 will encourage third-parties to code to that > >rather > >than to the s-expression protocol, and that''s a good thing. > > > Okay. So I''m going to take this to mean that we would deprecate > S-Expression/HTTP for 3.0.2 and remove it in 3.0.3? That would be > really nice.Yes, I hope so. That''s assuming that a) no-one objects and b) we get the config file formats (both the main Xend config file and the xm create config file) into XML too. I don''t see any point removing the s-expression protocol support unless we can remove the s-expression parser/generator too, but I certainly think that we should deprecate it for 3.0.2.> > How about if we prefixed > >every function that we wish to expose to the messaging layer with > >"public_"? So for example XendDomainInfo.send_sysrq would be named > >public_send_sysrq instead. Then, we could use that to guide the > >function registration, rather than having exclude lists and inline lists > >of callable methods. > > > Isn''t using an underscore a convention for making methods private in > Python? I think, at least, pydoc ignores functions that start with an > underscore.A preceding underscore indicates that "from Module import *" should not import the function, i.e. it''s a weak "private" declaration. There''s nothing special about embedded underscores though. I chose public_get_name over publicGetName because I thought that, at the XML-RPC level, it would be conventional to have "xend.domain.get_name" as the message name, rather than "xend.domain.GetName". That''s personal taste though, I don''t have a strong opinion here.> >> > >> def xend_node_get_dmesg(self): > >>- return self.xendGet(self.nodeurl(''dmesg'')) > >>+ return self.srv.xend.node.dmesg.info() > >> > > > >What you''ve done here is slipped the new XML-RPC layer under the existing > >XendClient API. I don''t think that we need to support that API at all. > >All > >the functions here just turn into stubs onto ServerProxy, and I don''t think > >that those stubs buy us anything. > > > Nope. Just compatibility. I know there are people using the XendClient > API.There are? Who? Anyone who''s using the XendClient API, please speak up!> >The reason I noticed is that I have been trying to make it so that > >XendDomain and XendDomainInfo only receive arguments of the correct type, > >with the casts to integers etc handled by the messaging layer. This > >change you''ve made highlights the fact that there is now nowhere for > >that type conversion to take place, because the messaging layer is more > >generic, and doesn''t understand the calls that it is dispatching. > >That''s OK -- it''s a consequence of the removal of all that code in > >SrvDomain -- but it''s something to be aware of. If we adopt the naming > >convention that I suggest above, then all methods accept only arguments > >of the correct type _except_ for those named "public_xyz" -- those > >methods must validate and convert their arguments appropriately. > > > Why? If a public_ function expects a domid and gets passed a string, it > ought to throw an exception and the client should get it as an error.I don''t think that there''s any user-facing call which should only accept a domid -- they should all accept either a domain ID or a domain name. Of course, xm cannot tell whether it is passing a domain ID or a domain name (because names may be strings of digits too, and these are typed at the command line) so I think that any public function must be able to cope with either case. That said, if there is some call that accepts only an integer, yes, it ought to be able to throw an exception. (I think, actually, that we''re talking at cross purposes, and obliquely agreeing with each other). Cheers, Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi Ewan, I''ve made the changes you''ve requested except where I''ve described below. After all the regressions complete, I''ll push it out to the list (sometime tomorrow as long as nothing fails miserably). Ewan Mellor wrote:> This is all a bit skanky, and could be easily cleaned up by introducing a > naming convention for XendDomain, XendNode, etc. How about if we prefixed > every function that we wish to expose to the messaging layer with > "public_"? So for example XendDomainInfo.send_sysrq would be named > public_send_sysrq instead. Then, we could use that to guide the > function registration, rather than having exclude lists and inline lists > of callable methods. >I went ahead and started using the public_ naming convention for XendDomainInfo. It was fine for a couple of the methods but it got ugly real quick for things like deviceDestroy as it''s called in a bunch of other places within Xend. So, I''ve got two possible solutions for this. We could just keep the white list or we could introduce new public_ methods within XendDomainInfo that were just simple wrappers for the underlying methods. It''s seems a bit unnatural but less unnatural than peppering calls to public_ functions through Xend. Thoughts?> I can see why you''ve done this, but it just led me to wonder why > XendDomain.domain_destroy exists at all. All it''s doing is looking up a > domain ID, checking that it''s not the privileged domain (in the wrong > order!) calling XendDomainInfo.destroy, and calling xc.domain_destroy in > the case of an exception. We should move the check and the exception > handling into XendDomainInfo.destroy, and then we can dispatch to that > method straight away, without needing XendDomain.domain_destroy at all. > The messaging layer already has the capability to lookup domain IDs -- > we should use it. >I''ve gone through and made sure that all XendDomain functions can accept either domids or names so that effectively takes care of this hack. I didn''t refactor away the domain_destroy function though as it seems independent of the XML-RPC stuff.> I expect we can do better than just printing "Internal Xend Error". How > much structure and useful information is there in an xmlrpclib.Fault at > the moment? >This one is a bit tricky. We use faultCode to encode particular exception types but I''m not sure what that buys us. I don''t think encoding OSError as a faultCode (which is what I think the most common exception we toss other than XendError) really buys us much right now. I think what we need to do is audit the methods in Xend, figure out what the common errors are, *catch them within Xend*, and then rethrow them as standardized Faults. Thoughts? Regards, Anthony Liguori>> sys.exit(1) >> except: >> print "Unexpected error:", sys.exc_info()[0] >> > > Looks like that''s it! Thanks for all your hard work, Anthony, this is > going to make a big difference to Xend''s usefulness and maintainability, > and you''ve done a good job of it. Let''s get it into 3.0.2. > > Cheers, > > Ewan. >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Mar 14, 2006 at 12:58:48AM -0600, Anthony Liguori wrote:> Hi Ewan, > > I''ve made the changes you''ve requested except where I''ve described > below. After all the regressions complete, I''ll push it out to the list > (sometime tomorrow as long as nothing fails miserably). > > Ewan Mellor wrote: > >This is all a bit skanky, and could be easily cleaned up by introducing a > >naming convention for XendDomain, XendNode, etc. How about if we prefixed > >every function that we wish to expose to the messaging layer with > >"public_"? So for example XendDomainInfo.send_sysrq would be named > >public_send_sysrq instead. Then, we could use that to guide the > >function registration, rather than having exclude lists and inline lists > >of callable methods. > > > I went ahead and started using the public_ naming convention for > XendDomainInfo. It was fine for a couple of the methods but it got ugly > real quick for things like deviceDestroy as it''s called in a bunch of > other places within Xend. > > So, I''ve got two possible solutions for this. We could just keep the > white list or we could introduce new public_ methods within > XendDomainInfo that were just simple wrappers for the underlying > methods. It''s seems a bit unnatural but less unnatural than peppering > calls to public_ functions through Xend. Thoughts?Yes, I''d go with the latter. public_device_destroy does the type conversion, argument validation (with errors thrown if necessary) and then just calls deviceDestroy. All the other calls to deviceDestroy from within XendDomainInfo itself don''t need the type conversion etc, so just point at deviceDestroy directly. Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel