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

Re: [libvirt] ESX network functionality

2011/4/6 James Barkley <james barkley gmail com>:
> Yes, I used the managed object called Network. It does get a little dicey
> with VMware due to the addition of port groups and virtual switches
> (admittedly, not sure how this works in most other hypervisors). You
> certainly can't have transient network objects, and any creation/destruction
> of a network object seems like it must necessarily involve either
> creation/destruction of a port group or creation/destruction of a vswitch. I
> believe using the Network managed object is the right approach, but am not
> yet 100% confident. My next task is to facilitate the creation of a new
> network through virNetworkDefineXML or virNetworkCreateXML. If you'd prefer,
> I can wait until I've completed this functionality to submit the patch, as
> it may drive me to adopt a different approach.

The docs about the Network object say that they are created and
destroyed automatically as needed. Did you try if a port group without
virtual machines attached to it has a corresponding Network object?

You could send your current code for an initial review, as the
implementation for virNetworkDefineXML should go into a second patch.

I think you cannot implement virNetworkCreateXML as that's supposed to
create a transient network and that's not how network work.

On the other hand, what would be the semantic of virNetworkCreate and
virNetworkDestroy? Those require that there is some kind of power
switch on the networks, like the power state of the virtual machines.
So the life cycle of a network is not entirely clear to me yet.

>>I assume you made esxNumOfDefinedNetworks return 0 to get virsh
>>net-list working as there network in the VMware context are always
> Correct. I was debating on whether or not to have it loop through the
> network list and check the "active/inactive" just for thoroughness, but if
> you're fine with just returning 0 and commenting why, then I'll leave it.

Is there actually something like an active/inactive state for a network?

>>The recommended way for sending patches is git send-email.
> Barring any other recommendations I'll figure out git send email and submit
> the patch to the list shortly.

You can also use git format-patch and attach the patch file as
plaintext to an email, but sending patches inline in email (like git
send email does) is the preferred way.


> thanks,
> -jb
> On Wed, Apr 6, 2011 at 2:37 AM, Matthias Bolte
> <matthias bolte googlemail com> wrote:
>> 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
>> active.
>> > - 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.
>> Matthias

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