Re: [libvirt] VMware ESX driver announcement

2009/5/19 Daniel Veillard <veillard(a)redhat.com&gt;: >> diff --git a/src/esx_driver.c b/src/esx_driver.c >> new file mode 100644 >> index 0000000..28500b8 >> --- /dev/null >> +++ b/src/esx_driver.c >> @@ -0,0 +1,1385 @@ > [...] >> +#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.

>> +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 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.

>> +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 :)

>  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.