4:22 a.m.
On Thu, May 21, 2009 at 02:54:46AM +0200, Matthias Bolte wrote:
2009/5/19 Daniel Veillard <veillard(a)redhat.com>:
>> 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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/