On Thu, May 21, 2009 at 02:54:46AM +0200, Matthias Bolte wrote:
> 2009/5/19 Daniel Veillard <veillard@xxxxxxxxxx>:
> >> The driver uses the SOAP based VMware VI API. We've tried to generate
> >> SOAP client code with gSOAP and looked at other SOAP libraries, but
> >> the VI API uses inheritance in a way that gSOAP fails to generate C
> >
> > Yes, I have tried that before, and it looked nasty, basically if you
> > use the C generator on the bindings you get something enormous, plus the
> > requirement on the soap library, etc ... 10 times as big as libvirt
> > itself.
>
> IIRC approx. 17 MB generated C code in one file as we tested gSOAP for
> the VMware VI API.
paphio:~/VMWare/gsoaptest -> ls -l soapC.c
-rw-rw-r-- 1 veillard vcsa 17397783 2008-04-29 13:42 soapC.c
As you can see I didn't touched it for more than a year >:->
> > The other possibility was to use the libraries they provide to
> > interconnect, but there was a licencing problem, plus you can't expect
> > to have thing installed on the server, so this was a dead end too.
>
> We looked at this closed source communication library provided by
> VMware too, but dropped it because of the licensing problems.
Yup, too bad, I'm pretty sure it does most of what you're doing now
and even use libxml2 for doing so too. By not open sourcing it this
means double effort ...
> >> The next item on the todo list is domain creation, because currently
> >> the driver can only deal with prior existing domains.
> >
> > Actually make sure you have dumpXML first, that's probably more
> > important as a first step since it finalize the XML format.
>
> We need the XML format first, before we can use it to define or create
> domains, so that was implied when saying domain creation is next.
Yup, let's start discussing the format !
> > maybe we should use an esx/ subdirectory like for vbox
>
> Will be done.
thanks :-)
> >> +#define ESX_ERROR(conn, code, fmt...) \
> >> + /* FIXME: remove this VIR_DEBUG0 line when libvirt provides */ \
> >> + /* file and line information in error messages */ \
> >> + VIR_DEBUG0 ("this is a dummy message to provide a source " \
> >> + "position for the following error..."); \
> >> + virReportErrorHelper (conn, VIR_FROM_ESX, code, __FILE__, \
> >> + __FUNCTION__, __LINE__, fmt)
> >
> > interesting, what's the problem ?
>
> virLogMessage() takes filename, funcname and linenr parameters and
> outputs them along with the message. This works fine for the VIR_DEBUG
> macro. virReportErrorHelper() also takes filename, funcname and linenr
> parameters but doesn't use them, because virRaiseError() doesn't
> handle this debugging information, because the virError struct has no
> place to store them. virRaiseError() also contains a comment "TODO:
> pass function name and lineno".
>
> The VIR_DEBUG0 is part of the ESX_ERROR macro just to provide the
> filename, funcname and linenr information for debugging, because this
> information currently gets lost in virReportErrorHelper() before the
> error is reported/logged. Also virLogMessage() only outputs this
> information for VIR_LOG_DEBUG level.
I guess Dan's patch will be commited shortly which fixes the problem.
> >> +static virDrvOpenStatus
> >> +esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED)
> >> +{
> >
> > Any idea if there is a notion of read-only connection in ESX, or is
> > everything role based ?
>
> It seems everything is account/group/role/privilege based. So there is
That was my recollection but ...
> no simple common way for a read-only connection. The user may login
> via a very restricted account that has been manually preconfigured for
> the ESX host, but there seems to be no special, default read-only
> access that the driver could use if the read-only flag is set for the
> open call.
Too bad that's useful for monitoring applications. I guess we can just
keep the OpenReadOnly routine non-implemented.
> >> +static int
> >> +esxDomainMigratePrepare(virConnectPtr dconn,
> >> + char **cookie ATTRIBUTE_UNUSED,
> >> + int *cookielen ATTRIBUTE_UNUSED,
> >> + const char *uri_in,
> >> + char **uri_out,
> >> + unsigned long flags ATTRIBUTE_UNUSED,
> >> + const char *dname ATTRIBUTE_UNUSED,
> >> + unsigned long resource ATTRIBUTE_UNUSED)
> >> +{
> >> + if (uri_in == NULL) {
> >> + if (virAsprintf(uri_out, "%s://%s%s",
> >> + dconn->uri->scheme + 4, dconn->uri->server,
> >> + dconn->uri->path) < 0) {
> >> + virReportOOMError(dconn);
> >> + return -1;
> >> + }
> >> + }
> >> +
> >> + return 0;
> >> +}
> >
> > Is there really no way to check for compatibility for example ?
>
> As in checking if a proposed migration can succeed before performing
> the actual migration? Yes there is, it's just not implemented yet and
> a note about it is missing in the code :)
Okay :-)
> >> +char *
> >> +esxUtil_MigrateStringFromLibXML(xmlChar *xmlString)
> >> +{
> >> + char *esxString = NULL;
> >> + size_t length = 0;
> >> +
> >> + if (xmlString == NULL) {
> >> + return NULL;
> >> + }
> >> +
> >> + length = xmlStrlen(xmlString);
> >> +
> >> + if (VIR_ALLOC_N(esxString, length + 1) >= 0) {
> >> + strncpy(esxString, (const char *) xmlString, length);
> >> + }
> >> +
> >> + xmlFree(xmlString);
> >> +
> >> + return esxString;
> >> +}
> >
> > Hum, this is a bit over the top, xmlChar * is just an UTF-8 C string,
> > and unless someone registered specific libxml2 allocators you don't need
> > to use xmlFree, free() is sufficient. Since other pieces of libvirt
> > don't xmlFree their libxml2 allocated strings this is superfluous IMHO.
>
> Well, the memory handling is currently written in a way that it uses
> the matching alloc/free function pairs and doesn't make any
> assumptions about miscibility of these pairs. Because the objects of
> the VI API client use VIR_ALLOC/VIR_FREE, all elements of them must
> also be allocated using VIR_ALLOC. If you say its okay to free a
> pointer allocated by libxml2 with VIR_FREE then
> esxUtil_MigrateStringFromLibXML() and esxUtil_Strdup() can be removed,
> because their sole purpose is to guarantee the use of matching
> alloc/free functions.
since we don't implement that separation between xmlFree() and free()
in other part of libvirt, while it's the proper thing to do usually, in
that case, unless we change all other XML string access it's useless,
i.e. we really can't remap libxml2 allocation functions. It's a lesson
from my early libxml2 days, you can't provide features in a library
depending on a global variable setup, if I had a context for all libxml2
entry points then that would have been possible :-\
> >> + virMutexUnlock(&ctx->curl_lock);
> >
> > Okay one serial operation per connection probably good enough ...
>
> As I understand libcurl a curl handle can only be used by one thread
> at the same time. Allowing multiple concurrent operations per
> connection would involve using multiple curl handles.
Not worth chasing, at least now.
> >> + (*remoteResponse)->xpathContext =
> >> + xmlXPathNewContext((*remoteResponse)->document);
> >
> > if you can deal only with one operation per connection that could be
> > optimized and the context you be persistent, just attach to the new
> > document, but not worth worring
>
> I'll note this on the todo list with low priority.
yup, it's insignificant.
> >> + if ((*remoteResponse)->code != 200) {
> >> + (*remoteResponse)->xpathObject =
> >> + xmlXPathEval(BAD_CAST
> >> + "/soapenv:Envelope/soapenv:Body/soapenv:Fault",
> >> + (*remoteResponse)->xpathContext);
> >
> > And libxml2 static XPath experssion could be precompiled, but it's not
> > worth optimizing at this point :-)
>
> I'll note this on the todo list with low priority too.
> > compared to XPAth processing, a doubly linked list might not be worth
> > the time it saves...
>
> Are you referring to the O(n) runtime for appending to the list? No
> need for a double linked list at this point, single link is good
> enough. I assume (but I'm not 100% sure) that the order of items in
> lists in SOAP requests and responses is arbitrary, so
> esxVI_List_Append() could be replaced with esxVI_List_Prepend() with
> O(1) runtime.
okay
> >> + next->_next = item;
> >> +
> >> + return 0;
> >> +}
> >
> > an awful lot of type handling, even with the mcro helpers this looks heavy.
>
> Well, yes. SOAP doesn't really map very well to C, if you want to have
> a proper layer around the (de)serialization an transport code.
Way too many Java focused people/org in that working group at the time :-\
[...]
> >> +enum _esxVI_Boolean {
> >> + esxVI_Boolean_Undefined = 0,
> >> + esxVI_Boolean_True,
> >> + esxVI_Boolean_False,
> >> +};
> >
> > True == 1, False == 2, that's funky !
>
> Many types in the VI API contain optional boolean values and having an
> undefined value is a simple way to express them. If a boolean value is
> set to undefined it's interpreted as not set. Defining undefined as 0
> makes all boolean values in newly allocated VI API objects (allocated
> by calloc in VIR_ALLOC) being undefined by default. On the other hand
> it results in this funky values for true and false.
Ah, okay, makes sense :-)
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list