Re: [Libguestfs] [PATCH libnbd] ocaml: Implement sockaddr type

On 11/03/22 16:16, Richard W.M. Jones wrote: > On Thu, Nov 03, 2022 at 04:03:42PM +0100, Laszlo Ersek wrote: >> On 11/02/22 22:10, Richard W.M. Jones wrote: >>> +open Unix >>> +open Printf >>> + >>> +let () = >>> + let nbd = NBD.create () in >>> + >>> + (* Unlike other tests, we're going to run nbdkit as a subprocess >>> + * by hand and have it listening on a randomly named socket >>> + * that we create. >>> + *) >>> + let sock = Filename.temp_file "580-" ".sock" in >>> + unlink sock; >>> + let pidfile = Filename.temp_file "580-" ".pid" in >>> + unlink pidfile; >>> + let cmd = >>> + sprintf "nbdkit -U %s -P %s --exit-with-parent memory size=512 &" >>> + (Filename.quote sock) (Filename.quote pidfile) in >>> + if Sys.command cmd <> 0 then >>> + failwith "nbdkit command failed"; >>> + let rec loop i = >>> + if i > 60 then >>> + failwith "nbdkit subcommand did not start up"; >>> + if not (Sys.file_exists pidfile) then ( >>> + sleep 1; >>> + loop (i+1) >>> + ) >>> + in >>> + loop 0; >>> + >>> + (* Connect to the subprocess using a Unix.sockaddr. *) >>> + let sa = ADDR_UNIX sock in >>> + NBD.aio_connect nbd sa; >>> + while NBD.aio_is_connecting nbd do >>> + ignore (NBD.poll nbd 1) >>> + done; >>> + assert (NBD.aio_is_ready nbd); >>> + NBD.close nbd; >>> + >>> + (* Kill the nbdkit subprocess. *) >>> + let chan = open_in pidfile in >>> + let pid = int_of_string (input_line chan) in >>> + kill pid Sys.sigint; >> >> I think it's more customary to send SIGTERM in such situations; SIGINT >> is more like an interactive interrupt signal (usually sent by the >> terminal driver when the INTR character is entered on the terminal). >> POSIX calls SIGINT "Terminal interrupt signal", and SIGTERM "Termination >> signal". But it's really tangential. > > I changed it to SIGTERM now (commit eb13699a75). > > The whole test is very unsatisfactory though. Compare it to the > elegance of the equivalent Python test: > > https://gitlab.com/nbdkit/libnbd/-/blob/master/python/python-aio-connect-... Can you perhaps introduce the test not under ML_TESTS but TESTS (called "test_580_aio_connect.sh")? Then use the "captive nbdkit" pattern (--run) just like in the python example. Now, ocaml doesn't support "-c" (I think), so the "$PYTHON -c" idea won't work identically, but if you also introduced "test_580_aio_connect.ml", and began that with an "ocaml shebang", that should work, shouldn't it? nbdkit [...] --run 'test_580_aio_connect.ml "$unixsocket"' (Not sure if it's worth the churn though.)