[ovs-dev] [PATCH 1/3] Windows installer

[Gurucharan Shetty]

On Mon, Apr 20, 2015 at 11:58 AM, Alin Serdean
<aserdean at cloudbasesolutions.com> wrote:
> This commit adds the windows installer to the OVS tree.
>
> Requirements are the following:
> Visual Studio Community 2013
> WiX Toolset 3.9
> Microsoft_VC120_CRT_x86.msm
>
> More detailed information on the requirements and build instructions
> can be found under:
> https://github.com/cloudbase/ovs-windows-installer/blob/master/README.rst
>
> Signed-off-by: Alessandro Pilotti <apilotti at cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Thank you so much for doing this! You rock!
The Signed-off-by chain is a little odd. Who is the Author? If it is
Alessandro, you should change this patch and make him the author of
the commit. If it was you, you should add your Signed-off-by. If it
was both, you should add a Co-authored-by from Alessandro and
Signed-off-by from you. Can you please re-post with the correct
submission chain?

I haven't looked at the the entire commit closely yet (or tried to
build this), but I have a couple of high level comments.

---snip----
> diff --git a/windows/LICENSE b/windows/LICENSE
> new file mode 100644
> index 0000000..68c771a
> --- /dev/null
Does the below LICENSE get installed somewhere after package
installation? Or is it the LICENSE for the installer code?

> +++ b/windows/LICENSE
> @@ -0,0 +1,176 @@
> +
> +                                 Apache License
> +                           Version 2.0, January 2004
> +                        http://www.apache.org/licenses/
> +
> +   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
> +



---snip---
> +    <Component Id="OvsdbServerService" Directory="BINARIESDIR" Guid="{280201D5-35E7-45D6-83B9-293F1A4F7F0E}">
> +      <File Id="ovsdbserver.exe" Source="Services\ovsdb-server.exe" Checksum="yes" />
> +      <ServiceInstall
> +                Arguments='--log-file="[LOGSDIR]ovsdb-server.log" --pidfile --service --service-monitor --unixctl="[APPDATADIR]ovsdb-server.ctl" --remote=punix:"[APPDATADIR]db.sock" "[CONFDIR]conf.db"'

I think the above will write the pidfile in the configured directory
which would be wrong. Probably it needs the entire path? Have you seen
where it gets generated? Same for ovs-vswitchd.