Re: [libvirt] [PATCH 1/2] ESX: Add routines to interface driver
2012/7/23 Laine Stump <laine(a)laine.org>:
On 07/20/2012 05:20 PM, Ata E Husain Bohra wrote:
> Add following routines to esx_interface_driver:
> esxNumOfInterfaces,
> esxNumOfDefinedInterfaces,
> esxListInterfaces,
> esxListDefinedInterfaces,
> esxInterfaceLookupByMACString,
> esxInterfaceGetXMLDesc,
> esxInterfaceUndefine,
> esxInterfaceCreate,
> esxInterfaceDestroy
>
> Signed-off-by: Ata E Husain Bohra <ata.husain(a)hotmail.com>
> ---
> src/esx/esx_interface_driver.c | 506 +++++++++++++++++++++++++++++++++++++++-
> src/esx/esx_vi.c | 126 ++++++++++
> src/esx/esx_vi.h | 10 +
> src/esx/esx_vi_generator.input | 227 ++++++++++++++++++
> src/esx/esx_vi_generator.py | 31 ++-
> src/esx/esx_vi_types.c | 18 +-
> 6 files changed, 913 insertions(+), 5 deletions(-)
>
> diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c
> index 5713137..b1ba5e2 100644
> --- a/src/esx/esx_interface_driver.c
> +++ b/src/esx/esx_interface_driver.c
> @@ -23,6 +23,9 @@
> */
>
> #include <config.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
>
> #include "internal.h"
> #include "util.h"
> @@ -34,6 +37,7 @@
> #include "esx_vi.h"
> #include "esx_vi_methods.h"
> #include "esx_util.h"
> +#include "interface_conf.h"
>
> #define VIR_FROM_THIS VIR_FROM_ESX
>
> @@ -67,10 +71,508 @@ esxInterfaceClose(virConnectPtr conn)
>
>
>
> +static int
> +esxNumOfInterfaces(virConnectPtr conn)
> +{
> + esxPrivate *priv = conn->interfacePrivateData;
> + esxVI_HostVirtualNic *virtualNicList = NULL;
> + const esxVI_HostVirtualNic *virtualNic = NULL;
It's a bit disconcerting to see these called "virtual" NICs, when
virInterface is all about configuring interfaces on the *physical* host.
This i my biggest concern here too, and also that's the reason why I
didn't implement the network driver yet. Because I'm not sure what is
the proper mapping between libvirt and vSphere API here.
As far as I understand this, a HostVirtualNic seems to be the best
match for a virInterface. A HostVirtualNic is connected to
HostVirtualSwitch, that is connected to the physical network via a
PhysicalNic. There is not much one can do about the PhysicalNic. It
just sits there and is connected to the HostVirtualSwitch. The
HostVirtualNic is also the one that has the external IP address of the
ESX server assigned.
Also a HostVirtualNic is not part of the virtual hardware of a VM. A
VirtualEthernetCard is the type in the vSphere API representing the
virtual network interface of a virtual machine
Being unfamiliar with the vmware API, I'm not sure which of
these
functions are things you've added to libvirt and which are part of
vmware, but please rename any that are defined in libvirt to use
"physical" or "host" or "p/h" as appropriate, rather than
"virtual"
and/or "v". We want to make sure nobody gets the wrong idea about these
functions.
I don't think that this is a good idea, we should really stick exactly
to the naming of things in the vSphere API otherwise there will be
much more confusion.
Therefore,
esxVI_HostVirtualNic *virtualNicList = NULL;
should be
esxVI_HostVirtualNic *hostVirtualNicList = NULL;
Apart from that I'll do a more detailed review soon.
--
Matthias Bolte
http://photron.blogspot.com