Re: [PATCH] Fix ocaml build in 4.1; problem with 4.1.6 release

[David Scott <dave.scott@eu.citrix.com>]

On 09/09/13 15:35, Ian Jackson wrote:
> (CC to xen-devel added)
>
> Ian Jackson writes ("Problem with 4.1.6 release"):
>> Jan Beulich writes ("please sign-and-tag 4.2.3 and 4.1.6"):
>>> Ian - could you also create tarballs as usual?
>>
>> The 4.1.6 tarball failed its build test.  This is due to an actual
>> build failure in 4.1.6.  See below.  This is probably a result of
>> 070ab4c505934951f86f42dd8403cf62bc5822f0 "oxenstored: Protect
>> oxenstored from malicious domains".
>
> Now confirmed by a local rebuild with 070ab4 reverted.
> Possible fix below.  This builds but I haven't tested it.
>
>> It appears that when I backported this change I this change from 4.2.x
>> to 4.1.x, I cannot have done a build test :-/.  (I have upgraded my
>> workstation in between but the error doesn't seem likely to be due to
>> a compiler version change.)
>>
>> The backport to 4.2.x involved conflicts, which I fixed up, and did do
>> a build test for so it looks like I resolved the conflict correctly.
>> The 4.1.x backport (of the 4.2.x patch) didn't involve textual
>> conflicts but it seems to have a semantic conflict.
>>
>> The osstest testing system doesn't install ocaml compilers, because
>> that makes it use oxenstored by default and the last time I attempted
>> to do this it produced regressions.  So the push gate didn't catch
>> this problem.
>>
>> I have already made and pushed a signed tag for 4.1.6.  That suggests
>> that we should abandon the 4.1.6 version number, in favour of 4.1.6.1
>> maybe.
>>
>> We also have to decide what to do with the code.  We shouldn't really
>> simply revert this fix, which is security-relevant.
>>
>> CC the authors of the patch which when backported became 070ab4c50593,
>> and Andrew Cooper who knows something about this code.
>>
>> Ian.
>>
>> make[5]: Entering directory `/u/iwj/work/xen.git/tools/ocaml/xenstored'
>>   MLI      symbol.cmi
>>   MLI      trie.cmi
>>   MLOPT    define.cmx
>>   MLOPT    stdext.cmx
>>   MLOPT    trie.cmx
>>   MLOPT    config.cmx
>>   MLOPT    logging.cmx
>> File "logging.ml", line 113, characters 3-4:
>> Warning 11: this match case is unused.
>>   MLOPT    quota.cmx
>>   MLOPT    perms.cmx
>>   MLOPT    symbol.cmx
>>   MLOPT    utils.cmx
>>   MLOPT    store.cmx
>>   MLOPT    disk.cmx
>>   MLOPT    transaction.cmx
>>   MLOPT    event.cmx
>>   MLOPT    domain.cmx
>>   MLOPT    domains.cmx
>>   MLOPT    connection.cmx
>>   MLOPT    connections.cmx
>>   MLOPT    parse_arg.cmx
>>   MLOPT    process.cmx
>> File "process.ml", line 375, characters 3-8:
>> Error: Unbound value error
>
> commit 7e792ffe54adc2d9fcc210baa8140f210a841c31
> Author: Ian Jackson <ian.jackson@eu.citrix.com>
> Date:   Mon Sep 9 15:30:42 2013 +0100
>
>      oxenstored: Fix process.ml build after 070ab4c50593
>
>      This change:
>        070ab4c505934951f86f42dd8403cf62bc5822f0
>        "oxenstored: Protect oxenstored from malicious domains"
>      broke the build because it had an unresolved semantic (but not
>      textual) conflict with
>        c69fddbd5dfa3004aaf2d0f2dde00c9ec3dd6d5d
>        "tools/ocaml: Remove log library from tools/ocaml/libs"
>      (which is in 4.2 but not 4.1)
>
>      Fix this by using the 4.1.x idiom in the new error handling introduced
>      in 070ab4c50593.
>
>      Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
>
> diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
> index bd87646..5c81755 100644
> --- a/tools/ocaml/xenstored/process.ml
> +++ b/tools/ocaml/xenstored/process.ml
> @@ -372,8 +372,8 @@ let do_input store cons doms con =
>   		try
>   			Connection.do_input con
>   		with Failure exp ->
> -			error "caught exception %s" exp;
> -			error "got a bad client %s" (sprintf "%-8s" (Connection.get_domstr con));
> +			Logs.error "general" "caught exception %s" exp;
> +			Logs.error "general" "got a bad client %s" (sprintf "%-8s" (Connection.get_domstr con));
>   			Connection.mark_as_bad con;
>   			false
>   	in

This looks good to me:

Acked-by: David Scott <dave.scott@eu.citrix.com>

Cheers,
Dave