Re: [libvirt] [PATCH 1/2] ESX: Add routines to interface driver

2012/7/23 Laine Stump <laine(a)laine.org&gt;: > 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&gt; >> --- >> 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.