1. I will respin the patches to fix the author chain.
2. The license is used in a dialog. The user must accept the license before it 
can be installed.
3. The pid file gets generated in the binaries directory I can change to a 
temporary one if needed.

Alin.

-----Mesaj original-----
De la: Gurucharan Shetty [mailto:shet...@nicira.com] 
Trimis: Thursday, April 23, 2015 4:33 AM
Către: Alin Serdean
Cc: dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH 1/3] Windows installer

On Mon, Apr 20, 2015 at 11:58 AM, Alin Serdean 
<aserd...@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 <apilo...@cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserd...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to