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

Re: [libvirt] [PATCH] - added mapping to Network object - added implementation of select networking functions - virsh net-list and net-info commands now work for esx

Long summary line. You could reformat it like this:

esx: Support virsh net-list and net-info

Add mapping of Network object and implementation of select
networking functions.

This makes virsh net-list and net-info commands work for ESX.

2011/4/12 jbarkley <james barkley gmail com>:
> From: jbarkely <jbarkley willo (none)>

You should tell git your name and email address to get it properly
recorded in a patch. See git config for this:

git config --global user.name "your name"
git config --global user.email "your email"

> ---
>  src/esx/esx_network_driver.c   |  181 +++++++++++++++++++++++++++++++++++++++-
>  src/esx/esx_vi.c               |  104 ++++++++++++++++++++++-
>  src/esx/esx_vi.h               |    9 ++
>  src/esx/esx_vi_generator.input |   78 +++++++++++++++++
>  4 files changed, 366 insertions(+), 6 deletions(-)

As I'm a bit short of time right now just some general comments.

- C style comments (/* */) are preferred over C++ style ones (//).
- Remove code that you have commented out.
- I'm not sure if using the MD5 sum of the name of a network it's UUID
is the stablest possible source. I'd like to find a better option for
this, but I'm not sure that it exists.
- Instead of adding many empty objects for the members of
HostNetworkSystem and HostNetworkInfo that aren't used yet you could
mark them as ignored (i) instead in the .input file.

Overall the patch looks okay. I'll give it a detailed review and test
it over the weekend.

In the meantime I'd like to delay the inclusion of this patch into the
codebase until we are confident that this approach is the correct one.


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