8:38 a.m.
On 12/07/2011 07:18 AM, Wayne Xia wrote:
These patches would do following things: building up the network
system CIM
model, building up basic framework and functions below to implement the
configuration, add the bridge and vlan802.1.q configuration capabilities in
CIM model and library below.
Now this patch implement the functionalities with commandline style, which
means it depends on command ip, ifconfig, brctl and /proc/net/vlan/. In this
way it is very sensitive to these command's output format. Maybe another
implemetion could avoid that in the future.
What exactly do you need with those commands. If we're talking about
listing network interfaces, obtaining IP/MAC addresses and like, this
all should be done via standard system calls instead of parsing command
line outputs. This is the basics of UNIX IPC mechanism. For instance see
socket(7), ip(7), netdevice(7) and related man pages, especially the
Ioctls sections.
Since libvirt-cim is not a daemon program that would always be
brought up
when the emulator was up, so there lacks a mechnism to save the configuration
and set them when system reboot, so these patch uses system directory
/etc/sysconfig/network-script/
to store the settings.
The things above means that a linux kernel later than 2.6 and RedHat
distribution is needed. Other distriution may work but not tested. If
the implemention goes to libvirt the restrict would be removed.
Depending on a specific distro is very very bad. Again, what kind of
configuration are you trying to store? Could the infostore
implementation in libvirt-cim which could be used?
And this line about implementation going to libvirt really makes me sad.
IMHO, if there is work to be done in libvirt, so why don't start there
instead of implementing everything in house and having to do everything
once again when the necessary bits are there?
[snip]
Wayne Xia (15):
vlan extension - makefile change
vlan extension - CIM model - add class VESS
vlan extension - CIM model - add class VESSSD
vlan extension - CIM model - add class EthernetPort
vlan extension - CIM model - add class
EthernetPortAllocationSettingData
vlan extension - CIM model - add association Net_SettingsDeineState
vlan extension - CIM model - add association Net_SystemDevice
vlan extension - CIM model - add association Net_VESSSDComponent
vlan extension - CIM model - add association Net_ElementSettingData
vlan extension - CIM model - add core class VESSMS
vlan extension - CIM model - add help functions
vlan extension - function lib - add the API
vlan extension - function lib - add the core structure
vlan extension - function lib - add the helper functions
vlan extension - function lib - add the implemention of commandline
For the series, looks like you sent the patches in the opposite order,
from last to first. If I apply the fist patch the build breaks. The
first review for this whole series is a -1 only because of that.
I think it would help a lot if you publish your code in gitorious for
instance. I have set up the mirrors for libvirt-cim repositories there:
http://gitorious.org/libvirt-cim
Finally, for future cases which involve huge amount of work, like this
one, *please please please* be gentle with us and avoid one big code
drop like this one. Small and incremental changes makes the life of
everyone, including yours, easier. :)
Best regards, Etrunko.
Makefile.am | 16 +-
libxkutil/Makefile.am | 11 +-
libxkutil/host_network_API.c | 150 ++
libxkutil/host_network_API.h | 32 +
libxkutil/host_network_basic.c | 639 +++++++
libxkutil/host_network_basic.h | 166 ++
libxkutil/host_network_error.h | 28 +
libxkutil/host_network_helper.c | 659 +++++++
libxkutil/host_network_helper.h | 192 ++
libxkutil/host_network_implement_cmdline.c | 2002 ++++++++++++++++++++
libxkutil/host_network_implement_cmdline.h | 55 +
libxkutil/network_model.c | 466 +++++
libxkutil/network_model.h | 105 +
P.S.:
I haven't had the chance to review the patches yet. But I have a sudden
cold feeling in my spine by looking at these first stats.
--
Eduardo de Barros Lima
Software Engineer, Open Virtualization
Linux Technology Center - IBM/Brazil
eblima(a)br.ibm.com