Re: [Libvirt-cim] [PATCH 00/15] vlan extension

First and foremost, thank you, Wayne. The design looks well thought-out the code appears complete. I took a quick first pass to review and respond to some of the patches. However, it occurred to me we should break this work into a few phases. We did something like this with the ACL code and it helped the review, test, and integration for the most part. Given the amount and complexity of the code, here's my suggestion: Phase 1: read-only support for existing bridges and VLANs Re-factor and reduce the current patch-set to only include the read-only aspects of the libraries and providers. This will cut the code to review significantly and can act as the foundation to the follow-on patch sets. Example: patch 0 - Summary of you changes and quick overview of your implementation. It's a good idea to also include some sample wbemcli commands that folks can use to verify the function. patch 1 - libxkutil updates & additions (read-only operations) patch 2 - schema updates & additions patch 3 - provider updates & additions (read-only operations) patch 4 - cimtest case Phase 2: read-write support for bridge and VLANs This is simply incremental to support Create/Modify/Delete instance as needed and InvokeMethod Example: patch 0 - Summary of additions/changes including new wbemcli examples. patch 1 - libxkutil updates & additions (write operations) patch 2 - provider updates & additions (write operations) patch 3 - cimtest case Phase 3: Associations (linking VLANs/bridge and network to VM) Incremental support for reading, and if needed writing, Associatotions Example: patch 0 - Summary of additions/changes including new wbemcli examples. patch 1 - libxkutil updates & additions (new write operations?) patch 2 - provider updates & additions (association operations) patch 3- cimtest case I'd like to at least get the phase 0 (see below) code reviewed and pushed next week. Let me know if you think that is reasonable given that some low-level code needs to be updated to use OS APIs (as opposed to command line parsing).

On 12/07/2011 04: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. > 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. > Related profiles: the most important are DSP1050, DSP1097, DSP > 2025(draft), > Other are: DSP1041, DSP1014. > Typical usage: assuming you have pNIC eth0. Use following xml files > passed > to wbemexec could create bridge testbr with eth0.10 attached, which > means all > VMs connecting to testbr would have access to IEEE802.1.q vlan 10. > > XML 1 > > <?xml version="1.0" encoding="utf-8" ?> > <CIM CIMVERSION="2.0" DTDVERSION="2.0"> > <MESSAGE ID="4711" PROTOCOLVERSION="1.0"> > <SIMPLEREQ> > <METHODCALL NAME="DefineSystem"> > <LOCALINSTANCEPATH> > <LOCALNAMESPACEPATH> > <NAMESPACE NAME="root"></NAMESPACE><NAMESPACE NAME="virt"></NAMESPACE> > </LOCALNAMESPACEPATH> > <INSTANCENAME > CLASSNAME="Net_VirtualEthernetSwitchSystemManagementService"> > <KEYBINDING NAME="CreationClassName"><KEYVALUE > VALUETYPE="string">Net_VirtualEthernetSwitchSystemManagementService</KEYVALUE></KEYBINDING> > > <KEYBINDING NAME="Name"><KEYVALUE VALUETYPE="string">Management > Service</KEYVALUE></KEYBINDING> > <KEYBINDING NAME="SystemCreationClassName"><KEYVALUE > VALUETYPE="string">KVM_HostSystem</KEYVALUE></KEYBINDING> > <KEYBINDING NAME="SystemName"><KEYVALUE > VALUETYPE="string">unknown</KEYVALUE></KEYBINDING> > </INSTANCENAME> > </LOCALINSTANCEPATH> > > <PARAMVALUE NAME="SystemSettings" PARAMTYPE="string"> > <VALUE> > instance of Net_VirtualEthernetSwitchSystemSettingData { > VirtualSystemIdentifier ="VS_testbr1"; > STP = 1; > }; > </VALUE> > </PARAMVALUE> > > </METHODCALL> > </SIMPLEREQ> > </MESSAGE> > </CIM> > > XML 2 > > <?xml version="1.0" encoding="utf-8" ?> > <CIM CIMVERSION="2.0" DTDVERSION="2.0"> > <MESSAGE ID="4711" PROTOCOLVERSION="1.0"> > <SIMPLEREQ> > <METHODCALL NAME="AddResourceSettings"> > <LOCALINSTANCEPATH> > <LOCALNAMESPACEPATH> > <NAMESPACE NAME="root"></NAMESPACE><NAMESPACE NAME="virt"></NAMESPACE> > </LOCALNAMESPACEPATH> > <INSTANCENAME > CLASSNAME="Net_VirtualEthernetSwitchSystemManagementService"> > <KEYBINDING NAME="CreationClassName"><KEYVALUE > VALUETYPE="string">Net_VirtualEthernetSwitchSystemManagementService</KEYVALUE></KEYBINDING> > > <KEYBINDING NAME="Name"><KEYVALUE VALUETYPE="string">Management > Service</KEYVALUE></KEYBINDING> > <KEYBINDING NAME="SystemCreationClassName"><KEYVALUE > VALUETYPE="string">KVM_HostSystem</KEYVALUE></KEYBINDING> > <KEYBINDING NAME="SystemName"><KEYVALUE > VALUETYPE="string">unknown</KEYVALUE></KEYBINDING> > </INSTANCENAME> > </LOCALINSTANCEPATH> > > <PARAMVALUE NAME="AffectedConfiguration"> > <VALUE.REFERENCE> > <INSTANCENAME CLASSNAME="Net_VirtualEthernetSwitchSettingData"> > <KEYBINDING NAME="InstanceID"><KEYVALUE > VALUETYPE="string">Virt:VS_eth0</KEYVALUE></KEYBINDING> > </INSTANCENAME> > </VALUE.REFERENCE> > </PARAMVALUE> > > <PARAMVALUE NAME="ResourceSettings" PARAMTYPE="string"> > <VALUE.ARRAY> > <VALUE> > instance of Net_EthernetPortAllocationSettingData { > InstanceID = "VS_eth0/EA_eth0.10"; > ElementName = "EA_eth0.10"; > VLANType = 1; > Connection = {"VLAN10"}; > ReorderHdr = 0; > }; > </VALUE> > <VALUE> > instance of Net_EthernetPortAllocationSettingData { > InstanceID = "VS_eth0/EC_eth0.10"; > HostResource = {"VS_testbr1"}; > Parent = "EA_eth0.10"; > }; > </VALUE> > </VALUE.ARRAY> > </PARAMVALUE> > > </METHODCALL> > </SIMPLEREQ> > </MESSAGE> > </CIM> > > 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 > > 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 + > schema/ElementSettingData.mof | 16 +- > schema/ElementSettingData.registration | 4 +- > schema/EthernetPort.mof | 4 + > schema/EthernetPort.registration | 3 + > schema/EthernetPortAllocationSettingData.mof | 21 + > .../EthernetPortAllocationSettingData.registration | 3 + > schema/SettingsDefineState.mof | 9 +- > schema/SettingsDefineState.registration | 3 +- > schema/SystemDevice.mof | 18 +- > schema/SystemDevice.registration | 3 +- > schema/VESSSDComponent.mof | 8 + > schema/VESSSDComponent.registration | 3 + > schema/VirtualEthernetSwitchSystem.mof | 10 + > schema/VirtualEthernetSwitchSystem.registration | 3 + > ...irtualEthernetSwitchSystemManagementService.mof | 14 + > ...ernetSwitchSystemManagementService.registration | 3 + > schema/VirtualEthernetSwitchSystemSettingData.mof | 27 + > ...ualEthernetSwitchSystemSettingData.registration | 3 + > src/Makefile.am | 49 +- > src/Virt_EASD.c | 733 +++++++ > src/Virt_EASD.h | 61 + > src/Virt_ElementSettingData.c | 229 +++- > src/Virt_EthernetPort.c | 565 ++++++ > src/Virt_EthernetPort.h | 58 + > src/Virt_HostSystem.c | 2 +- > src/Virt_HostSystem.h | 2 + > src/Virt_SettingsDefineState.c | 256 +++ > src/Virt_SystemDevice.c | 126 ++ > src/Virt_VESSSD.c | 372 ++++ > src/Virt_VESSSD.h | 39 + > src/Virt_VESSSDComponent.c | 181 ++ > src/Virt_VirtualEthernetSwitchSystem.c | 483 +++++ > src/Virt_VirtualEthernetSwitchSystem.h | 52 + > ..._VirtualEthernetSwitchSystemManagementService.c | 1342 +++++++++++++ > ..._VirtualEthernetSwitchSystemManagementService.h | 31 + > src/Virt_VirtualSystemManagementService.c | 17 +- > src/Virt_VirtualSystemManagementService.h | 10 + > 50 files changed, 9255 insertions(+), 29 deletions(-) > create mode 100644 libxkutil/host_network_API.c > create mode 100644 libxkutil/host_network_API.h > create mode 100644 libxkutil/host_network_basic.c > create mode 100644 libxkutil/host_network_basic.h > create mode 100644 libxkutil/host_network_error.h > create mode 100644 libxkutil/host_network_helper.c > create mode 100644 libxkutil/host_network_helper.h > create mode 100644 libxkutil/host_network_implement_cmdline.c > create mode 100644 libxkutil/host_network_implement_cmdline.h > create mode 100644 libxkutil/network_model.c > create mode 100644 libxkutil/network_model.h > create mode 100644 schema/EthernetPort.mof > create mode 100644 schema/EthernetPort.registration > create mode 100644 schema/EthernetPortAllocationSettingData.mof > create mode 100644 schema/EthernetPortAllocationSettingData.registration > create mode 100644 schema/VESSSDComponent.mof > create mode 100644 schema/VESSSDComponent.registration > create mode 100644 schema/VirtualEthernetSwitchSystem.mof > create mode 100644 schema/VirtualEthernetSwitchSystem.registration > create mode 100644 > schema/VirtualEthernetSwitchSystemManagementService.mof > create mode 100644 > schema/VirtualEthernetSwitchSystemManagementService.registration > create mode 100644 schema/VirtualEthernetSwitchSystemSettingData.mof > create mode 100644 > schema/VirtualEthernetSwitchSystemSettingData.registration > create mode 100644 src/Virt_EASD.c > create mode 100644 src/Virt_EASD.h > create mode 100644 src/Virt_EthernetPort.c > create mode 100644 src/Virt_EthernetPort.h > create mode 100644 src/Virt_VESSSD.c > create mode 100644 src/Virt_VESSSD.h > create mode 100644 src/Virt_VESSSDComponent.c > create mode 100644 src/Virt_VirtualEthernetSwitchSystem.c > create mode 100644 src/Virt_VirtualEthernetSwitchSystem.h > create mode 100644 > src/Virt_VirtualEthernetSwitchSystemManagementService.c > create mode 100644 > src/Virt_VirtualEthernetSwitchSystemManagementService.h >