[ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Alin Serdean aserdean at cloudbasesolutions.com
Fri Aug 15 16:26:53 UTC 2014


Nithin,

Thanks for the review I will send out a V2 as soon as possible.

Thanks,
Alin.

-----Mesaj original-----
De la: Nithin Raju [mailto:nithin at vmware.com] 
Trimis: Friday, August 15, 2014 3:52 AM
Către: Alin Serdean
Cc: dev at 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



More information about the dev mailing list