Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
On 09/06/22 11:19, Eric Blake wrote:
> On Mon, Sep 05, 2022 at 02:10:36PM +0200, Laszlo Ersek wrote:
>> On 09/03/22 00:14, Eric Blake wrote:
>>> This proves that the stat counters increment as desired, as well as
>>> proving that our RUInt32 generator type works.
>>
>> How is this related to RUint32?
>
> Typo in the commit message; this should be RUInt64. Oh well, the
> commit is already in.
>
>>
>>>
>>> This commit is also a showcase of whether I can do 64-bit math in
>>> various languages (C's terseness in 'a == b + (c > d)' is annoying to
>>> replicate in languages that don't like playing fast and loose with
>>> types). :)
>>
>> I don't understand; please elaborate.
>
> Sure. In C, uint64_t == uint64_t + bool is legal; making for a nice
> terse expression.
I interpreted your statement about 64-bit math and the formula
a == b + (c > d)
as if the formula were some means to implement *64-bit-safe* math (addition,
overflow etc something like that). I understood how the expression worked at
the lowest level, but didn't understand what it was good for -- what it
expressed.
The more concrete form is
cr3 == cr2 + (br3 > br2)
which seems to stand for: "chunks received #3" should equal "chunks received
#2, plus one chunk, if we received additional bytes (in br3) since setting br2".
Basically the section
+bs2 = h.stats_bytes_sent()
+cs2 = h.stats_chunks_sent()
+br2 = h.stats_bytes_received()
+cr2 = h.stats_chunks_received()
+
+[...]
+
+# Stats are still readable after the connection closes; we don't know if
+# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
+h.shutdown()
+
+bs3 = h.stats_bytes_sent()
+cs3 = h.stats_chunks_sent()
+br3 = h.stats_bytes_received()
+cr3 = h.stats_chunks_received()
+
+assert bs3 > bs2
+assert cs3 == cs2 + 1
+assert br3 >= br2
+assert cr3 == cr2 + (br3 > br2)
was a bit too hard for me to parse. I didn't notice the role of h.shutdown()
before.
Acked-by: Laszlo Ersek
>
>>> +++ b/python/t/620-stats.py
>
>>> +# Stats are still readable after the connection closes; we don't know if
>>> +# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
>>> +h.shutdown()
>>> +
>>> +bs3 = h.stats_bytes_sent()
>>> +cs3 = h.stats_chunks_sent()
>>> +br3 = h.stats_bytes_received()
>>> +cr3 = h.stats_chunks_received()
>>> +
>>> +assert bs3 > bs2
>>> +assert cs3 == cs2 + 1
>>> +assert br3 >= br2
>>> +assert cr3 == cr2 + (br3 > br2)
>
> And in python.
>
>>> +++ b/ocaml/tests/test_620_stats.ml
>>> + (* Stats are still readable after the connection closes; we don't know if
>>> + * the server sent reply bytes to our NBD_CMD_DISC, so don't insist on
>>> it.
>>> + *)
>>> + NBD.shutdown nbd;
>>> +
>>> + let bs3 = NBD.stats_bytes_sent nbd in
>>> + let cs3 = NBD.stats_chunks_sent nbd in
>>> + let br3 = NBD.stats_bytes_received nbd in
>>> + let cr3 = NBD.stats_chunks_received nbd in
>>> + let fudge = if cr2 = cr3 then 0L else 1L in
>>> + assert (bs3 > bs2);
>>> + assert (cs3 = (Int64.succ cs2));
>>> + assert (br3 >= br2);
>>> + assert (cr3 = (Int64.add cr2 fudge))
>
> Not so in OCaml, where + isn't even polymorphic to int64, so I have to
> break out manual calls to Int64.XXX methods. Here, I went with a
> helper variable 'fudge'.
>
>>> +++ b/golang/libnbd_620_stats.go
>>> + /* Stats are still readable after the connection closes; we don't know
>>> if
>>> +* the server sent reply bytes to our NBD_CMD_DISC, so don't insist on
>>> it.
>>> +*/
>>> + err = h.Shutdown(nil)
>>> + if err != nil {
>>> + t.Fatalf("%s", err)
>>> + }
>>> +
>>> + bs3, err := h.StatsBytesSent()
>>> + if err != nil {
>>> + t.Fatalf("%s", err)
>>> + }
>>> + cs3, err := h.StatsChunksSent()
>>> + if err != nil {
>>> + t.Fatalf("%s", err)
>>> + }
>>> + br3, err := h.StatsBytesReceived()
>>> + if err != nil {
>>> + t.Fatalf("%s", err)
>>> + }
>>> + cr3, err := h.StatsChunksReceived()
>>> + if err != nil {
>>> + t.Fatalf("%s", err)
>>> + }
>>> + slop := uint64(1)
>>> + if br2 == br3 {
>>> + slop = uint64(0)
>>> + }
>>> +
>>> + if bs3 <= bs2 {
>>> + t.Fatalf("unexpected value for bs3")
>>> + }
>>> + if cs3 != cs2 + 1 {
>>> + t.Fatalf("unexpected value for cs3")
>>> + }
>>> + if br3 < br2 {
>>> + t.Fatalf("unexpected value for br3")
>>> + }
>>> + if cr3 != cr2 + slop {
>>> + t.Fatalf("unexpected value for cr3")
>>> + }
>
> Nor in Go, where you can't even do uint64(bool), but HAVE to use an
> 'if' statement to populate 'slop' with the correct value. Hmm, maybe
> I should have used the same variable name between OCaml and Go,
> instead of 'slop'/'fudge'; oh well. (And Go is painfully verbose in
> the amount of boilerplate formatting it requires, compared to the
> other languages; although I will be the first to admit that I'm
> probably not an idiomatic Go coder)
>
Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
On Tue, Sep 06, 2022 at 04:19:40AM -0500, Eric Blake wrote: > > > + assert (cr3 = (Int64.add cr2 fudge)) > > Not so in OCaml, where + isn't even polymorphic to int64, so I have to > break out manual calls to Int64.XXX methods. Here, I went with a > helper variable 'fudge'. No implicit type conversions in OCaml! (as a deliberate choice) You don't need to the parens around Int64.add since function application always binds tightest. You could have written this if you'd wanted: let (+^) = Int64.add ... assert (cr3 = cr2 +^ fudge) Custom operators always have the same precedence as the normal operator with the same first character. Or in recent OCaml versions: assert Int64.(cr3 = cr2 + fudge) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
On Mon, Sep 05, 2022 at 02:10:36PM +0200, Laszlo Ersek wrote:
> On 09/03/22 00:14, Eric Blake wrote:
> > This proves that the stat counters increment as desired, as well as
> > proving that our RUInt32 generator type works.
>
> How is this related to RUint32?
Typo in the commit message; this should be RUInt64. Oh well, the
commit is already in.
>
> >
> > This commit is also a showcase of whether I can do 64-bit math in
> > various languages (C's terseness in 'a == b + (c > d)' is annoying to
> > replicate in languages that don't like playing fast and loose with
> > types). :)
>
> I don't understand; please elaborate.
Sure. In C, uint64_t == uint64_t + bool is legal; making for a nice
terse expression.
> > +++ b/python/t/620-stats.py
> > +# Stats are still readable after the connection closes; we don't know if
> > +# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
> > +h.shutdown()
> > +
> > +bs3 = h.stats_bytes_sent()
> > +cs3 = h.stats_chunks_sent()
> > +br3 = h.stats_bytes_received()
> > +cr3 = h.stats_chunks_received()
> > +
> > +assert bs3 > bs2
> > +assert cs3 == cs2 + 1
> > +assert br3 >= br2
> > +assert cr3 == cr2 + (br3 > br2)
And in python.
> > +++ b/ocaml/tests/test_620_stats.ml
> > + (* Stats are still readable after the connection closes; we don't know if
> > + * the server sent reply bytes to our NBD_CMD_DISC, so don't insist on
> > it.
> > + *)
> > + NBD.shutdown nbd;
> > +
> > + let bs3 = NBD.stats_bytes_sent nbd in
> > + let cs3 = NBD.stats_chunks_sent nbd in
> > + let br3 = NBD.stats_bytes_received nbd in
> > + let cr3 = NBD.stats_chunks_received nbd in
> > + let fudge = if cr2 = cr3 then 0L else 1L in
> > + assert (bs3 > bs2);
> > + assert (cs3 = (Int64.succ cs2));
> > + assert (br3 >= br2);
> > + assert (cr3 = (Int64.add cr2 fudge))
Not so in OCaml, where + isn't even polymorphic to int64, so I have to
break out manual calls to Int64.XXX methods. Here, I went with a
helper variable 'fudge'.
> > +++ b/golang/libnbd_620_stats.go
> > + /* Stats are still readable after the connection closes; we don't know
> > if
> > +* the server sent reply bytes to our NBD_CMD_DISC, so don't insist on
> > it.
> > +*/
> > + err = h.Shutdown(nil)
> > + if err != nil {
> > + t.Fatalf("%s", err)
> > + }
> > +
> > + bs3, err := h.StatsBytesSent()
> > + if err != nil {
> > + t.Fatalf("%s", err)
> > + }
> > + cs3, err := h.StatsChunksSent()
> > + if err != nil {
> > + t.Fatalf("%s", err)
> > + }
> > + br3, err := h.StatsBytesReceived()
> > + if err != nil {
> > + t.Fatalf("%s", err)
> > + }
> > + cr3, err := h.StatsChunksReceived()
> > + if err != nil {
> > + t.Fatalf("%s", err)
> > + }
> > + slop := uint64(1)
> > + if br2 == br3 {
> > + slop = uint64(0)
> > + }
> > +
> > + if bs3 <= bs2 {
> > + t.Fatalf("unexpected value for bs3")
> > + }
> > + if cs3 != cs2 + 1 {
> > + t.Fatalf("unexpected value for cs3")
> > + }
> > + if br3 < br2 {
> > + t.Fatalf("unexpected value for br3")
> > + }
> > + if cr3 != cr2 + slop {
> > + t.Fatalf("unexpected value for cr3")
> > + }
Nor in Go, where you can't even do uint64(bool), but HAVE to use an
'if' statement to populate 'slop' with the correct value. Hmm, maybe
I should have used the same variable name between OCaml and Go,
instead of 'slop'/'fudge'; oh well. (And Go is painfully verbose in
the amount of boilerplate formatting it requires, compared to the
other languages; although I will be the first to admit that I'm
probably not an idiomatic Go coder)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
On 09/03/22 00:14, Eric Blake wrote: > This proves that the stat counters increment as desired, as well as > proving that our RUInt32 generator type works. How is this related to RUint32? > > This commit is also a showcase of whether I can do 64-bit math in > various languages (C's terseness in 'a == b + (c > d)' is annoying to > replicate in languages that don't like playing fast and loose with > types). :) I don't understand; please elaborate. Thanks Laszlo > --- > python/t/620-stats.py | 77 +++ > ocaml/tests/Makefile.am | 1 + > ocaml/tests/test_620_stats.ml | 77 +++ > golang/Makefile.am| 3 +- > golang/libnbd_620_stats.go| 181 ++ > 5 files changed, 338 insertions(+), 1 deletion(-) > create mode 100644 python/t/620-stats.py > create mode 100644 ocaml/tests/test_620_stats.ml > create mode 100644 golang/libnbd_620_stats.go > > diff --git a/python/t/620-stats.py b/python/t/620-stats.py > new file mode 100644 > index ..62f8443f > --- /dev/null > +++ b/python/t/620-stats.py > @@ -0,0 +1,77 @@ > +# libnbd Python bindings > +# Copyright (C) 2010-2022 Red Hat Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + > +import nbd > + > +h = nbd.NBD() > + > +# Pre-connection, stats start out at 0 > +bs0 = h.stats_bytes_sent() > +cs0 = h.stats_chunks_sent() > +br0 = h.stats_bytes_received() > +cr0 = h.stats_chunks_received() > + > +assert bs0 == 0 > +assert cs0 == 0 > +assert br0 == 0 > +assert cr0 == 0 > + > +# Connection performs handshaking, which increments stats. > +# The number of bytes/chunks here may grow over time as more features get > +# automatically negotiated, so merely check that they are non-zero. > +h.connect_command(["nbdkit", "-s", "--exit-with-parent", "null"]) > + > +bs1 = h.stats_bytes_sent() > +cs1 = h.stats_chunks_sent() > +br1 = h.stats_bytes_received() > +cr1 = h.stats_chunks_received() > + > +assert cs1 > 0 > +assert bs1 > cs1 > +assert cr1 > 0 > +assert br1 > cr1 > + > +# A flush command should be one chunk out, one chunk back (even if > +# structured replies are in use) > +h.flush() > + > +bs2 = h.stats_bytes_sent() > +cs2 = h.stats_chunks_sent() > +br2 = h.stats_bytes_received() > +cr2 = h.stats_chunks_received() > + > +assert bs2 == bs1 + 28 > +assert cs2 == cs1 + 1 > +assert br2 == br1 + 16 # assumes nbdkit uses simple reply > +assert cr2 == cr1 + 1 > + > +# Stats are still readable after the connection closes; we don't know if > +# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it. > +h.shutdown() > + > +bs3 = h.stats_bytes_sent() > +cs3 = h.stats_chunks_sent() > +br3 = h.stats_bytes_received() > +cr3 = h.stats_chunks_received() > + > +assert bs3 > bs2 > +assert cs3 == cs2 + 1 > +assert br3 >= br2 > +assert cr3 == cr2 + (br3 > br2) > + > +# Try to trigger garbage collection of h > +h = None > diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am > index 22cefb4d..c4751ad3 100644 > --- a/ocaml/tests/Makefile.am > +++ b/ocaml/tests/Makefile.am > @@ -42,6 +42,7 @@ ML_TESTS = \ > test_590_aio_copy.ml \ > test_600_debug_callback.ml \ > test_610_exception.ml \ > + test_620_stats.ml \ > $(NULL) > > EXTRA_DIST = $(ML_TESTS) > diff --git a/ocaml/tests/test_620_stats.ml b/ocaml/tests/test_620_stats.ml > new file mode 100644 > index ..648096fd > --- /dev/null > +++ b/ocaml/tests/test_620_stats.ml > @@ -0,0 +1,77 @@ > +(* hey emacs, this is OCaml code: -*- tuareg -*- *) > +(* libnbd OCaml test case > + * Copyright (C) 2013-2022 Red Hat Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free
Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
On Sat, Sep 03, 2022 at 05:32:23PM +0100, Richard W.M. Jones wrote: > > So the patch series is OK, modulo my comments. I've made tweaks according to your requests. > > But I think it'd be better not to bind our future selves to > complicated guarantees about what are basically internal details of > the library and protocol. This, plus the 2 patches to see nbd_stats_ in LIBNBD_DEBUG=1 output, are now in as: 043d430c..0c0082c6 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
So the patch series is OK, modulo my comments. But I think it'd be better not to bind our future selves to complicated guarantees about what are basically internal details of the library and protocol. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libnbd PATCH 4/4] tests: Add language binding tests for stats reception
This proves that the stat counters increment as desired, as well as proving that our RUInt32 generator type works. This commit is also a showcase of whether I can do 64-bit math in various languages (C's terseness in 'a == b + (c > d)' is annoying to replicate in languages that don't like playing fast and loose with types). :) --- python/t/620-stats.py | 77 +++ ocaml/tests/Makefile.am | 1 + ocaml/tests/test_620_stats.ml | 77 +++ golang/Makefile.am| 3 +- golang/libnbd_620_stats.go| 181 ++ 5 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 python/t/620-stats.py create mode 100644 ocaml/tests/test_620_stats.ml create mode 100644 golang/libnbd_620_stats.go diff --git a/python/t/620-stats.py b/python/t/620-stats.py new file mode 100644 index ..62f8443f --- /dev/null +++ b/python/t/620-stats.py @@ -0,0 +1,77 @@ +# libnbd Python bindings +# Copyright (C) 2010-2022 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import nbd + +h = nbd.NBD() + +# Pre-connection, stats start out at 0 +bs0 = h.stats_bytes_sent() +cs0 = h.stats_chunks_sent() +br0 = h.stats_bytes_received() +cr0 = h.stats_chunks_received() + +assert bs0 == 0 +assert cs0 == 0 +assert br0 == 0 +assert cr0 == 0 + +# Connection performs handshaking, which increments stats. +# The number of bytes/chunks here may grow over time as more features get +# automatically negotiated, so merely check that they are non-zero. +h.connect_command(["nbdkit", "-s", "--exit-with-parent", "null"]) + +bs1 = h.stats_bytes_sent() +cs1 = h.stats_chunks_sent() +br1 = h.stats_bytes_received() +cr1 = h.stats_chunks_received() + +assert cs1 > 0 +assert bs1 > cs1 +assert cr1 > 0 +assert br1 > cr1 + +# A flush command should be one chunk out, one chunk back (even if +# structured replies are in use) +h.flush() + +bs2 = h.stats_bytes_sent() +cs2 = h.stats_chunks_sent() +br2 = h.stats_bytes_received() +cr2 = h.stats_chunks_received() + +assert bs2 == bs1 + 28 +assert cs2 == cs1 + 1 +assert br2 == br1 + 16 # assumes nbdkit uses simple reply +assert cr2 == cr1 + 1 + +# Stats are still readable after the connection closes; we don't know if +# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it. +h.shutdown() + +bs3 = h.stats_bytes_sent() +cs3 = h.stats_chunks_sent() +br3 = h.stats_bytes_received() +cr3 = h.stats_chunks_received() + +assert bs3 > bs2 +assert cs3 == cs2 + 1 +assert br3 >= br2 +assert cr3 == cr2 + (br3 > br2) + +# Try to trigger garbage collection of h +h = None diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am index 22cefb4d..c4751ad3 100644 --- a/ocaml/tests/Makefile.am +++ b/ocaml/tests/Makefile.am @@ -42,6 +42,7 @@ ML_TESTS = \ test_590_aio_copy.ml \ test_600_debug_callback.ml \ test_610_exception.ml \ + test_620_stats.ml \ $(NULL) EXTRA_DIST = $(ML_TESTS) diff --git a/ocaml/tests/test_620_stats.ml b/ocaml/tests/test_620_stats.ml new file mode 100644 index ..648096fd --- /dev/null +++ b/ocaml/tests/test_620_stats.ml @@ -0,0 +1,77 @@ +(* hey emacs, this is OCaml code: -*- tuareg -*- *) +(* libnbd OCaml test case + * Copyright (C) 2013-2022 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + *) + +let () = + let nbd = NBD.create () in + + (* Pre-connection, stats start out at 0 *) + let bs0 = NBD.stats_bytes_sent nbd in + let cs0 = NBD.stats_chunks_sent nbd in + let br0 = NBD.stats_bytes_received nbd in + let cr0 = NBD.stats_chunks_received nbd in + assert (bs0 = 0L); + assert
