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

Re: [libvirt] ESX network functionality

Sorry, I forgot to reply to your follow up question on the users list.

2011/4/6 James Barkley <james barkley gmail com>:
> Greetings:
> I've added code to the ESX driver to support some basic network
> functionality. I'm pretty new to this list, so please tell me how to proceed
> with code review and patch submission (yes I've read the contributor
> guidelines on the wiki). It seems like people are emailing a patch file for
> every file they've changed, each in separate emails with [1 of N] in the
> subject, is that right? Or is it better to paste in code, get some feedback,
> and eventually attach the patch to the bug tracker item?

The normal approach is to have one commit/patch per logical
self-contained change. After each commit/patch the codebase has to be
in a compilable stage. For example you cannot add the code using the
generated SOAP bindings, before you actually added them to the
esx_vi_generator.input file.

The [1 of N] style patch series are typically used for large changes
that are split in several logical, self-contained parts. Splitting
like this simplifies reviewing and later on figuring out bugs using
git bisect.

You typically don't split on a per file basis.

In your case I'd suggest to create a single commit/patch for your
addition, as it is one logical piece of work. You _could_ (but I don't
recommend to) split it in multiple patches. For example one for the
esx_vi_generator.input addition, one for the VI helper function
additions and one for the actual driver functions, but I'd consider
this to be too fine grained.

> I've updated the code for the ESX driver to be able to handle the following
> functions :
> - virNetworkLookupByName
> - virConnectNumOfDefinedNetworks
> - virConnectNumOfNetworks
> - virConnectListNetworks

Is this sufficient to make virsh net-list work?

> I basically mapped the VMware Managed Object Reference for networks into a
> few data structures and added the following functions to the internal driver

You mean the managed object called Network? They _seem_ to be the
natural fit. But I'm not sure if that's the correct approach, as you
cannot directly create/destroy such objects and they are bound in some
way to the port groups on a virtual switch. Also I'm not sure about
the exact semantics of networks and port groups.

That's what I meant as I said about the mapping between libvirt and
VMware. We need to be sure to use the right approach from the start to
avoid making breaking changes across releases later on.

> API:
> - esxNumOfNetworks
> - esxListNetworks
> - esxNumOfDefinedNetworks

I assume you made esxNumOfDefinedNetworks return 0 to get virsh
net-list working as there network in the VMware context are always

> - esxNetworkLookupByName
> - esxVI_LookupNetworkList
> - esxVI_LookupNetworkByName
> These functions were modeled after existing functions from the domain and
> storage libraries. The following files were touched:
> - esx_network_driver.c
> - esx_vi.c
> - esx_vi.h
> - esx_vi_generator.input

Apart from the question whether the Network managed object is the
correct match, the next step is to send your patch to this list to get
a review. The recommended way for sending patches is git send-email.


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