[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] esx: Add vpx:// scheme to allow direct connection to a vCenter



On Mon, Jul 19, 2010 at 01:26:10AM +0200, Matthias Bolte wrote:
> Add a pointer to the primary context of a connection and use it in all
> driver functions that don't dependent on the context type. This includes
> almost all functions that deal with a virDomianPtr. Therefore, using
> a vpx:// connection allows you to perform all the usual domain related
> actions like start, destroy, suspend, resume, dumpxml etc.
> 
> Some functions that require an explicitly specified ESX server don't work
> yet. This includes the host UUID, the hostname, the general node info, the
> max vCPU count and the free memory. Also not working yet are migration and
> defining new domains.
> ---
>  docs/drvesx.html.in          |   21 +-
>  src/esx/esx_driver.c         |  931 ++++++++++++++++++++++++------------------
>  src/esx/esx_private.h        |    1 +
>  src/esx/esx_storage_driver.c |   33 +-
>  src/esx/esx_vmx.c            |    2 +
>  src/libvirt.c                |    1 +
>  6 files changed, 562 insertions(+), 427 deletions(-)
> 
> diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
> index e8cee77..1f2ae4e 100644
> --- a/docs/drvesx.html.in
> +++ b/docs/drvesx.html.in
> @@ -4,13 +4,14 @@
>      <p>
>          The libvirt VMware ESX driver can manage VMware ESX/ESXi 3.5/4.0 and
>          VMware GSX 2.0, also called VMware Server 2.0, and possibly later
> -        versions.
> +        versions. <span class="since">Since 0.8.3</span> the driver can also
> +        connect to a VMware vCenter 2.5/4.0 (VPX).
>      </p>
>  
>  
>      <h2><a name="prereq">Deployment pre-requisites</a></h2>
>      <p>
> -        None. Any out-of-the-box installation of ESX/GSX should work. No
> +        None. Any out-of-the-box installation of VPX/ESX(i)/GSX should work. No
>          preparations are required on the server side, no libvirtd must be
>          installed on the ESX server. The driver uses version 2.5 of the remote,
>          SOAP based
> @@ -27,10 +28,11 @@
>          Some example remote connection URIs for the driver are:
>      </p>
>  <pre>
> -esx://example.com                  (ESX over HTTPS)
> -gsx://example.com                  (GSX over HTTPS)
> -esx://example.com/?transport=http  (ESX over HTTP)
> -esx://example.com/?no_verify=1     (ESX over HTTPS, but doesn't verify the server's SSL certificate)
> +vpx://example-vcenter.com              (VPX over HTTPS)
> +esx://example-esx.com                  (ESX over HTTPS)
> +gsx://example-gsx.com                  (GSX over HTTPS)
> +esx://example-esx.com/?transport=http  (ESX over HTTP)
> +esx://example-esx.com/?no_verify=1     (ESX over HTTPS, but doesn't verify the server's SSL certificate)
>  </pre>
>      <p>
>          <strong>Note</strong>: In contrast to other drivers, the ESX driver is
> @@ -49,9 +51,9 @@ esx://example.com/?no_verify=1     (ESX over HTTPS, but doesn't verify the serve
>  type://[username ]hostname[:port]/[?extraparameters]


  May I suggest a small extra sentence here, explaining how to compose
extraparameters ? User may find from lookuing above that they need to
use:

   arg=value

but there is no indication on how to separate multiple parameter.

>  </pre>
>      <p>
[...]
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index 33f421d..5922cb6 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -67,6 +67,11 @@ esxSupportsLongMode(esxPrivate *priv)
>          return priv->supportsLongMode;
>      }
>  
> +    if (priv->host == NULL) {
> +        /* FIXME: Currently no host for a vpx:// connection */
> +        return esxVI_Boolean_False;
> +    }
> +

  What about emitting an error in those cases ?

[...]
> @@ -661,6 +729,11 @@ esxSupportsVMotion(esxPrivate *priv)
>          return priv->supportsVMotion;
>      }
>  
> +    if (priv->host == NULL) {
> +        /* FIXME: Currently no host for a vpx:// connection */
> +        return esxVI_Boolean_False;
> +    }
> +

  I feel ignorant here. I would assume that a vpx:// connection being to
a vCenter vould allow to migrate, care to explain :-) ?

[...]

> @@ -2173,6 +2283,7 @@ esxDomainDumpXML(virDomainPtr domain, int flags)
>      }
>  
>      esxVI_String_Free(&propertyNameList);
> +    esxVI_ObjectContent_Free(&datacenter);
>      esxVI_ObjectContent_Free(&virtualMachine);
>      VIR_FREE(datastoreName);
>      VIR_FREE(directoryName);

  I'm a bit suspicious, I see no virFree(datacenterName) here ... normal ?

Okay, overall a very large part of the patch is the change from
priv->host to priv->primary, and there is still some TODOs without
an error, I assume they will get fixed soon,

  based on this and after double checking the few points I raised, ACK :-)

    thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]