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

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

 



2012/7/23 Laine Stump <laine@xxxxxxxxx>:
> 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@xxxxxxxxxxx>
>> ---
>>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]