Libguestfs - Nov 2022 - [PATCH libnbd] ocaml: Implement sockaddr type

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-unix.sh

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

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-unix.sh
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.)

Laszlo