Nithin, Thanks for the review I will send out a V2 as soon as possible.
Thanks, Alin. -----Mesaj original----- De la: Nithin Raju [mailto:nit...@vmware.com] Trimis: Friday, August 15, 2014 3:52 AM Către: Alin Serdean Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild Alin, Thanks for making this change. It is great that we can build via command line now. Some comments I had are: 1. Somewhere at the top we should mention that the 'forwarding extension' is a 'kernel driver'. For someone now familiar with Windows, it might make it more obvious as to which is the kernel component. 2. Now 'make' has a dependency on WDK 8.1. If we can have 'configure' check for this, it would be very helpful. I took your change and compiled both userspace and kernel binaries. I copied over the kernel binaries to my debug client machine, and was able to do live debugging with the binaries build on the debug server machine. So, this validates the dev-test-debug workflow as well. --- Some cosmetic comments about the patch: > -* Run make for the ported executables in the top source directory, e.g.: > +* Run make for the ported executables and the forwarding extension in the > top minor: extra white space between 'executables' and 'and'. > -* Run make for the ported executables. > +* Run make for the ported executables and the forwarding extension in the > top minor: extra white space between 'executables' and 'and'. > -Installing the Kernel module > +Installing the forwarding extension > ---------------------------- minor: you need to have the underscoring using '---' for the entire line above. > + ./datapath-windows/x64/Win8Debug/package/ovsext.inf > + ./datapath-windows/x64/Win8Debug/package/OVSExt.sys > + ./datapath-windows/x64/Win8Debug/package/ovsext.cat At the top, we put a requirement of WDK 8.1. Can we build in 'Win8Debug' if we only have WDK 8.1 installed? ie. no WDK 8.0? This is the reason why we wanted to remove the 'Win8[Debug|Release]' mode. We haven't gotten around to doing it yet. Is there any reason why you wanted to use 'Win8' mode instead of 'Win8.1'? > -Steps to install the module > +Steps to install the forwarding extension > --------------------------- minor: you need to have the underscoring using '---' for the entire line above. thanks, Nithin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
