Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-15 Thread Richard W.M. Jones 
On Mon, Aug 14, 2023 at 07:55:15AM +, Tage Johansson wrote:
> 
> On 8/11/2023 2:00 PM, Eric Blake wrote:
> 
> On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> 
> This commit creates basic Rust bindings in the rust directory.
> The bindings are generated by generator/Rust.ml and
> generator/RustSys.ml.
> ---
> 
> --- /dev/null
> +++ b/generator/RustSys.ml
> 
> +(** Print the struct for a closure. *)
> +let print_closure_struct { cbname; cbargs } =
> +  pr "#[repr(C)]\n";
> +  pr "#[derive(Debug, Clone, Copy)]\n";
> +  pr "pub struct nbd_%s_callback {\n" cbname;
> +  pr "pub callback: \n";
> +  pr "  Option 
> c_int>,\n"
> +(cbargs |> List.map cbarg_types |> List.flatten |> String.concat 
> ", ");
> +  pr "pub user_data: *mut c_void,\n";
> +  pr "pub free: Option,\n";
> +  pr "}\n"
> 
> Why is 'callback' an Option<> rather than a mandatory argument?  I get
> that 'free' must be an Option<> (because it corresponds to an
> OClosure, which is an optional callback), but 'callback' is a
> mandatory function pointer in the C API; why would it ever be
> acceptable to pass None instead of Some?
> 
> It uses the "nullable pointer optimization" which makes an Option
> where T is a non nullable type be represented with no extra space,
> and the None variant is represented by NULL.

We use this in nbdkit too:

https://gitlab.com/nbdkit/nbdkit/-/blob/0d25cd82b43c52fc3adc9319019f528a3233a61d/plugins/rust/src/lib.rs#L1294

(Notice how that struct mirrors the C struct nbdkit_plugin)

Rich.

> 
> Keep in mind that this is only the internal FFI bindings and the Option will
> never be part of the public interface.
> 
> 
> Best regards,
> 
> Tage
> 
> 
> 

> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-14 Thread Eric Blake 
On Mon, Aug 14, 2023 at 07:55:15AM +, Tage Johansson wrote:
> 
> On 8/11/2023 2:00 PM, Eric Blake wrote:
> > On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> > > This commit creates basic Rust bindings in the rust directory.
> > > The bindings are generated by generator/Rust.ml and
> > > generator/RustSys.ml.
> > > ---
> > > --- /dev/null
> > > +++ b/generator/RustSys.ml
> > > +(** Print the struct for a closure. *)
> > > +let print_closure_struct { cbname; cbargs } =
> > > +  pr "#[repr(C)]\n";
> > > +  pr "#[derive(Debug, Clone, Copy)]\n";
> > > +  pr "pub struct nbd_%s_callback {\n" cbname;
> > > +  pr "pub callback: \n";
> > > +  pr "  Option c_int>,\n"
> > > +(cbargs |> List.map cbarg_types |> List.flatten |> String.concat ", 
> > > ");
> > > +  pr "pub user_data: *mut c_void,\n";
> > > +  pr "pub free: Option,\n";
> > > +  pr "}\n"
> > Why is 'callback' an Option<> rather than a mandatory argument?  I get
> > that 'free' must be an Option<> (because it corresponds to an
> > OClosure, which is an optional callback), but 'callback' is a
> > mandatory function pointer in the C API; why would it ever be
> > acceptable to pass None instead of Some?
> 
> 
> It uses the "nullable pointer optimization" 
> 
> which makes an Option where T is a non nullable type be represented with
> no extra space, and the None variant is represented by NULL.
> 
> 
> Keep in mind that this is only the internal FFI bindings and the Option will
> never be part of the public interface.

Ah, so if I'm understanding correctly, even though the libnbd API
requires a non-NULL function pointer, the problem is that the FFI code
can't in general tell which C functions with a function pointer
parameter allow NULL and which do not, so the conservative approach is
to document that _all_ FFI interactions with C functions use an
Option nullable pointer when passing a function pointer across the
Rust->C boundary.  It looks odd in relation to the libnbd
documentation of requiring non-NULL function, but is well-hidden
inside our internal code, and does not leak out to the public Rust
interface.  That fills in some gaps for me; thanks.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-14 Thread Richard W.M. Jones 
On Mon, Aug 14, 2023 at 08:15:53AM +, Tage Johansson wrote:
> 
> On 8/11/2023 3:41 PM, Richard W.M. Jones wrote:
> 
> On Fri, Aug 11, 2023 at 08:22:34AM -0500, Eric Blake wrote:
> 
> On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> 
> This commit creates basic Rust bindings in the rust directory.
> The bindings are generated by generator/Rust.ml and
> generator/RustSys.ml.
> ---
> 
> +++ b/rust/Cargo.toml
> @@ -0,0 +1,49 @@
> +# nbd client library in userspace
> +# Copyright Tage Johansson
> +#
> +# 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 says LGPLv2+...
> 
> 
> +#
> +# 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
> +
> +[workspace]
> +
> +[workspace.package]
> +authors = ["Tage Johansson"]
> +version = "0.1.0"
> +edition = "2021"
> +description = "Rust bindings for libnbd, a client library for 
> controlling block devices over a network."
> +license = "LGPL-2.1-only"
> 
> ...but this does not.  Why the discrepancy?  It is a disservice to
> clients to have a more restrictive license for the Rust bindings than
> for the rest of libnbd.
> 
> I agree with Eric that we should fix this (it seems like a simple
> oversight rather than a deliberate thing).  If Tage you give me the OK
> I will change it in the upstream repo.
> 
> 
> You have the OK to change it in the upstream repo. It should be
> "GPL-2.1-or-later" according to this site.

I think "LGPL-2.1-or-later", since it's a library ..

Rich.

> 
> Best regards,
> 
> Tage
> 
> 
> Rich.
> 
> 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-14 Thread Tage Johansson 


On 8/11/2023 3:41 PM, Richard W.M. Jones wrote:

On Fri, Aug 11, 2023 at 08:22:34AM -0500, Eric Blake wrote:

On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:

This commit creates basic Rust bindings in the rust directory.
The bindings are generated by generator/Rust.ml and
generator/RustSys.ml.
---
+++ b/rust/Cargo.toml
@@ -0,0 +1,49 @@
+# nbd client library in userspace
+# Copyright Tage Johansson
+#
+# 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 says LGPLv2+...


+#
+# 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
+
+[workspace]
+
+[workspace.package]
+authors = ["Tage Johansson"]
+version = "0.1.0"
+edition = "2021"
+description = "Rust bindings for libnbd, a client library for controlling block 
devices over a network."
+license = "LGPL-2.1-only"

...but this does not.  Why the discrepancy?  It is a disservice to
clients to have a more restrictive license for the Rust bindings than
for the rest of libnbd.

I agree with Eric that we should fix this (it seems like a simple
oversight rather than a deliberate thing).  If Tage you give me the OK
I will change it in the upstream repo.



You have the OK to change it in the upstream repo. It should be 
"GPL-2.1-or-later" according to this site 
.



Best regards,

Tage



Rich.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-14 Thread Tage Johansson 


On 8/11/2023 2:00 PM, Eric Blake wrote:

On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:

This commit creates basic Rust bindings in the rust directory.
The bindings are generated by generator/Rust.ml and
generator/RustSys.ml.
---
--- /dev/null
+++ b/generator/RustSys.ml
+(** Print the struct for a closure. *)
+let print_closure_struct { cbname; cbargs } =
+  pr "#[repr(C)]\n";
+  pr "#[derive(Debug, Clone, Copy)]\n";
+  pr "pub struct nbd_%s_callback {\n" cbname;
+  pr "pub callback: \n";
+  pr "  Option c_int>,\n"
+(cbargs |> List.map cbarg_types |> List.flatten |> String.concat ", ");
+  pr "pub user_data: *mut c_void,\n";
+  pr "pub free: Option,\n";
+  pr "}\n"

Why is 'callback' an Option<> rather than a mandatory argument?  I get
that 'free' must be an Option<> (because it corresponds to an
OClosure, which is an optional callback), but 'callback' is a
mandatory function pointer in the C API; why would it ever be
acceptable to pass None instead of Some?



It uses the "nullable pointer optimization" 
 
which makes an Option where T is a non nullable type be represented 
with no extra space, and the None variant is represented by NULL.



Keep in mind that this is only the internal FFI bindings and the Option 
will never be part of the public interface.



Best regards,

Tage

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-11 Thread Richard W.M. Jones 
On Fri, Aug 11, 2023 at 08:22:34AM -0500, Eric Blake wrote:
> On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> > This commit creates basic Rust bindings in the rust directory.
> > The bindings are generated by generator/Rust.ml and
> > generator/RustSys.ml.
> > ---
> 
> > +++ b/rust/Cargo.toml
> > @@ -0,0 +1,49 @@
> > +# nbd client library in userspace
> > +# Copyright Tage Johansson
> > +#
> > +# 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 says LGPLv2+...
> 
> > +#
> > +# 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
> > +
> > +[workspace]
> > +
> > +[workspace.package]
> > +authors = ["Tage Johansson"]
> > +version = "0.1.0"
> > +edition = "2021"
> > +description = "Rust bindings for libnbd, a client library for controlling 
> > block devices over a network."
> > +license = "LGPL-2.1-only"
> 
> ...but this does not.  Why the discrepancy?  It is a disservice to
> clients to have a more restrictive license for the Rust bindings than
> for the rest of libnbd.

I agree with Eric that we should fix this (it seems like a simple
oversight rather than a deliberate thing).  If Tage you give me the OK
I will change it in the upstream repo.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-11 Thread Eric Blake 
On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> This commit creates basic Rust bindings in the rust directory.
> The bindings are generated by generator/Rust.ml and
> generator/RustSys.ml.
> ---

> +++ b/rust/Cargo.toml
> @@ -0,0 +1,49 @@
> +# nbd client library in userspace
> +# Copyright Tage Johansson
> +#
> +# 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 says LGPLv2+...

> +#
> +# 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
> +
> +[workspace]
> +
> +[workspace.package]
> +authors = ["Tage Johansson"]
> +version = "0.1.0"
> +edition = "2021"
> +description = "Rust bindings for libnbd, a client library for controlling 
> block devices over a network."
> +license = "LGPL-2.1-only"

...but this does not.  Why the discrepancy?  It is a disservice to
clients to have a more restrictive license for the Rust bindings than
for the rest of libnbd.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-11 Thread Eric Blake 
On Thu, Aug 03, 2023 at 03:36:05PM +, Tage Johansson wrote:
> This commit creates basic Rust bindings in the rust directory.
> The bindings are generated by generator/Rust.ml and
> generator/RustSys.ml.
> ---

> --- /dev/null
> +++ b/generator/RustSys.ml

> +(** Print the struct for a closure. *)
> +let print_closure_struct { cbname; cbargs } =
> +  pr "#[repr(C)]\n";
> +  pr "#[derive(Debug, Clone, Copy)]\n";
> +  pr "pub struct nbd_%s_callback {\n" cbname;
> +  pr "pub callback: \n";
> +  pr "  Option c_int>,\n"
> +(cbargs |> List.map cbarg_types |> List.flatten |> String.concat ", ");
> +  pr "pub user_data: *mut c_void,\n";
> +  pr "pub free: Option,\n";
> +  pr "}\n"

Why is 'callback' an Option<> rather than a mandatory argument?  I get
that 'free' must be an Option<> (because it corresponds to an
OClosure, which is an optional callback), but 'callback' is a
mandatory function pointer in the C API; why would it ever be
acceptable to pass None instead of Some?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-04 Thread Eric Blake 
On Fri, Aug 04, 2023 at 10:50:44AM +, Tage Johansson wrote:
> 
> On 8/4/2023 10:54 AM, Richard W.M. Jones wrote:
> > I applied the first 4 patches to my local repo.
> > 'make clean' now fails at:
> > 
> >error: failed to load manifest for dependency `libnbd-sys`
> >Caused by:
> >  failed to parse manifest at 
> > `/home/rjones/d/libnbd/rust/libnbd-sys/Cargo.toml`
> >Caused by:
> >  no targets specified in the manifest
> >  either src/lib.rs, src/main.rs, a [lib] section, or [[bin]] section 
> > must be present
> >make[1]: *** [Makefile:1057: clean-local] Error 101
> >make[1]: Leaving directory '/home/rjones/d/libnbd/rust'
> > 
> > I'm not sure what that means.  rust/libnbd-sys/Cargo.toml exists.
> 
> 
> It is because rust/libnbd-sys/src/lib.rs is generated and hence does not
> exist before running make the first time. I have solved that in v6.
> 
> 
> > ./configure works which is good:
> > 
> >checking for cargo... cargo
> >checking for rustfmt... rustfmt
> >checking if cargo is usable... yes
> >...
> >  Rust ... yes
> > 
> > However the generator fails to compile with:
> > 
> >make[3]: Entering directory '/home/rjones/d/libnbd/generator'
> >ocamlc.opt -g -annot -safe-string -warn-error 
> > +C+D+E+F+L+M+P+S+U+V+Y+Z+X+52-3 -I . -I . \
> >  -I +str str.cma -I +unix unix.cma utils.mli utils.ml state_machine.mli 
> > state_machine.ml API.mli API.ml state_machine_generator.mli 
> > state_machine_generator.ml C.mli C.ml Python.mli Python.ml OCaml.mli 
> > OCaml.ml GoLang.mli GoLang.ml RustSys.mli RustSys.ml Rust.mli Rust.ml 
> > generator.ml  -o generator
> > File "generator.ml", line 65, characters 2-11:
> >65 |   output_to ~formatter:(Some Rustfmt) "rust/libnbd-sys/src/lib.rs" 
> > RustSys.generate_rust_sys_bindings;
> >   ^
> >Error: This function has type string -> (unit -> 'weak1) -> unit
> >   It is applied to too many arguments; maybe you forgot a `;'.
> > 
> > This confused me for a while, but I think it's because this series
> > depends on another series or needs to be rebased?
> > 
> > I removed the ~formatter parameter to get it to compile.
> 
> 
> Hmmm, I think there is a patch I thought was upstream, which doesn't seem to
> be so. I'll include that patch in v6.
> 
> 
> > The next problem is:
> > 
> >/home/rjones/d/libnbd/run cargo build
> >   Compiling libnbd v0.1.0 (/home/rjones/d/libnbd/rust)
> >error: expected one of `:`, `;`, or `=`, found `-`
> >   --> src/bindings.rs:116:29
> >|
> >116 | pub const CONTEXT_QEMU_DIRTY-BITMAP:: &[u8] = 
> > b"qemu:dirty-bitmap:";
> >| ^ expected one of `:`, `;`, or `=`
> 
> 
> I've fixed that as well. I was not testing on the latest changes upstream so
> I didn't encounter that error before.

Yeah, I've been committing things that will definitely affect Rust
bindings (as I had to touch all the other languages), with more to
land soon.  Resolving merge conflicts during rebasing to latest
changes is, for good or bad, a necessary part of open source
programming (the more active a project, the more likely you have
people submitting work in parallel that needs to be resolved
together).  If you have questions about my recent additions, I'm happy
to answer.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-04 Thread Tage Johansson 



On 8/4/2023 10:54 AM, Richard W.M. Jones wrote:

I applied the first 4 patches to my local repo.
'make clean' now fails at:

   error: failed to load manifest for dependency `libnbd-sys`
   Caused by:
 failed to parse manifest at 
`/home/rjones/d/libnbd/rust/libnbd-sys/Cargo.toml`
   Caused by:
 no targets specified in the manifest
 either src/lib.rs, src/main.rs, a [lib] section, or [[bin]] section must 
be present
   make[1]: *** [Makefile:1057: clean-local] Error 101
   make[1]: Leaving directory '/home/rjones/d/libnbd/rust'

I'm not sure what that means.  rust/libnbd-sys/Cargo.toml exists.



It is because rust/libnbd-sys/src/lib.rs is generated and hence does not 
exist before running make the first time. I have solved that in v6.




./configure works which is good:

   checking for cargo... cargo
   checking for rustfmt... rustfmt
   checking if cargo is usable... yes
   ...
 Rust ... yes

However the generator fails to compile with:

   make[3]: Entering directory '/home/rjones/d/libnbd/generator'
   ocamlc.opt -g -annot -safe-string -warn-error 
+C+D+E+F+L+M+P+S+U+V+Y+Z+X+52-3 -I . -I . \
 -I +str str.cma -I +unix unix.cma utils.mli utils.ml state_machine.mli 
state_machine.ml API.mli API.ml state_machine_generator.mli 
state_machine_generator.ml C.mli C.ml Python.mli Python.ml OCaml.mli OCaml.ml 
GoLang.mli GoLang.ml RustSys.mli RustSys.ml Rust.mli Rust.ml generator.ml  -o 
generator
File "generator.ml", line 65, characters 2-11:
   65 |   output_to ~formatter:(Some Rustfmt) "rust/libnbd-sys/src/lib.rs" 
RustSys.generate_rust_sys_bindings;
  ^
   Error: This function has type string -> (unit -> 'weak1) -> unit
  It is applied to too many arguments; maybe you forgot a `;'.

This confused me for a while, but I think it's because this series
depends on another series or needs to be rebased?

I removed the ~formatter parameter to get it to compile.



Hmmm, I think there is a patch I thought was upstream, which doesn't 
seem to be so. I'll include that patch in v6.




The next problem is:

   /home/rjones/d/libnbd/run cargo build
  Compiling libnbd v0.1.0 (/home/rjones/d/libnbd/rust)
   error: expected one of `:`, `;`, or `=`, found `-`
  --> src/bindings.rs:116:29
   |
   116 | pub const CONTEXT_QEMU_DIRTY-BITMAP:: &[u8] = b"qemu:dirty-bitmap:";
   | ^ expected one of `:`, `;`, or `=`



I've fixed that as well. I was not testing on the latest changes 
upstream so I didn't encounter that error before.




   error[E0599]: no method named `set_debug_callback` found for struct `Handle` 
in the current scope
 --> src/handle.rs:38:21
  |
   23 | pub struct Handle {
  | - method `set_debug_callback` not found for this struct
   ...
   38 | nbd.set_debug_callback(|func_name, msg| {
  | ^^ method not found in `Handle`
   [and some more]

It might be an idea to run this command to check that the series is
bisectable:

   git rebase -i HEAD~12 -x 'make clean && make && make check'

It will generate a series of rebase commands which look like:

   pick 7475e8560a rust: create basic Rust bindings
   exec make clean && make && make check
   pick c48c7eee0f rust: Add a couple of integration tests
   exec make clean && make && make check
   pick 16debe7848 rust: Make it possible to run tests with Valgrind
   exec make clean && make && make check
   [etc]

which will do a full build cycle after every patch to make sure they
all work incrementally.



I will try that.


Best regards,

Tage



Rich.



___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-04 Thread Richard W.M. Jones 
I applied the first 4 patches to my local repo.
'make clean' now fails at:

  error: failed to load manifest for dependency `libnbd-sys`
  Caused by:
failed to parse manifest at 
`/home/rjones/d/libnbd/rust/libnbd-sys/Cargo.toml`
  Caused by:
no targets specified in the manifest
either src/lib.rs, src/main.rs, a [lib] section, or [[bin]] section must be 
present
  make[1]: *** [Makefile:1057: clean-local] Error 101
  make[1]: Leaving directory '/home/rjones/d/libnbd/rust'

I'm not sure what that means.  rust/libnbd-sys/Cargo.toml exists.

./configure works which is good:

  checking for cargo... cargo
  checking for rustfmt... rustfmt
  checking if cargo is usable... yes
  ...
Rust ... yes

However the generator fails to compile with:

  make[3]: Entering directory '/home/rjones/d/libnbd/generator'
  ocamlc.opt -g -annot -safe-string -warn-error +C+D+E+F+L+M+P+S+U+V+Y+Z+X+52-3 
-I . -I . \
-I +str str.cma -I +unix unix.cma utils.mli utils.ml state_machine.mli 
state_machine.ml API.mli API.ml state_machine_generator.mli 
state_machine_generator.ml C.mli C.ml Python.mli Python.ml OCaml.mli OCaml.ml 
GoLang.mli GoLang.ml RustSys.mli RustSys.ml Rust.mli Rust.ml generator.ml  -o 
generator
File "generator.ml", line 65, characters 2-11:
  65 |   output_to ~formatter:(Some Rustfmt) "rust/libnbd-sys/src/lib.rs" 
RustSys.generate_rust_sys_bindings;
 ^
  Error: This function has type string -> (unit -> 'weak1) -> unit
 It is applied to too many arguments; maybe you forgot a `;'.

This confused me for a while, but I think it's because this series
depends on another series or needs to be rebased?

I removed the ~formatter parameter to get it to compile.

The next problem is:

  /home/rjones/d/libnbd/run cargo build
 Compiling libnbd v0.1.0 (/home/rjones/d/libnbd/rust)
  error: expected one of `:`, `;`, or `=`, found `-`
 --> src/bindings.rs:116:29
  |
  116 | pub const CONTEXT_QEMU_DIRTY-BITMAP:: &[u8] = b"qemu:dirty-bitmap:";
  | ^ expected one of `:`, `;`, or `=`

  error[E0599]: no method named `set_debug_callback` found for struct `Handle` 
in the current scope
--> src/handle.rs:38:21
 |
  23 | pub struct Handle {
 | - method `set_debug_callback` not found for this struct
  ...
  38 | nbd.set_debug_callback(|func_name, msg| {
 | ^^ method not found in `Handle`
  [and some more]

It might be an idea to run this command to check that the series is
bisectable:

  git rebase -i HEAD~12 -x 'make clean && make && make check'

It will generate a series of rebase commands which look like:

  pick 7475e8560a rust: create basic Rust bindings
  exec make clean && make && make check
  pick c48c7eee0f rust: Add a couple of integration tests
  exec make clean && make && make check
  pick 16debe7848 rust: Make it possible to run tests with Valgrind
  exec make clean && make && make check
  [etc]

which will do a full build cycle after every patch to make sure they
all work incrementally.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [libnbd PATCH v5 01/12] rust: create basic Rust bindings

2023-08-03 Thread Tage Johansson 
This commit creates basic Rust bindings in the rust directory.
The bindings are generated by generator/Rust.ml and
generator/RustSys.ml.
---
 .gitignore |  10 +
 .ocamlformat   |   4 +
 Makefile.am|   2 +
 configure.ac   |  30 ++
 generator/Makefile.am  |   4 +
 generator/Rust.ml  | 548 +
 generator/Rust.mli |  20 ++
 generator/RustSys.ml   | 167 +++
 generator/RustSys.mli  |  19 ++
 generator/generator.ml |   3 +
 rust/Cargo.toml|  49 
 rust/Makefile.am   |  76 +
 rust/cargo_test/Cargo.toml |  23 ++
 rust/cargo_test/README.md  |   3 +
 rust/cargo_test/src/lib.rs |  31 +++
 rust/libnbd-sys/Cargo.toml |  32 +++
 rust/libnbd-sys/build.rs   |  26 ++
 rust/libnbd-sys/src/.keep  |   0
 rust/run-tests.sh.in   |  24 ++
 rust/src/error.rs  | 157 +++
 rust/src/handle.rs |  65 +
 rust/src/lib.rs|  28 ++
 rust/src/types.rs  |  18 ++
 rust/src/utils.rs  |  23 ++
 rustfmt.toml   |  19 ++
 scripts/git.orderfile  |  10 +
 26 files changed, 1391 insertions(+)
 create mode 100644 .ocamlformat
 create mode 100644 generator/Rust.ml
 create mode 100644 generator/Rust.mli
 create mode 100644 generator/RustSys.ml
 create mode 100644 generator/RustSys.mli
 create mode 100644 rust/Cargo.toml
 create mode 100644 rust/Makefile.am
 create mode 100644 rust/cargo_test/Cargo.toml
 create mode 100644 rust/cargo_test/README.md
 create mode 100644 rust/cargo_test/src/lib.rs
 create mode 100644 rust/libnbd-sys/Cargo.toml
 create mode 100644 rust/libnbd-sys/build.rs
 create mode 100644 rust/libnbd-sys/src/.keep
 create mode 100755 rust/run-tests.sh.in
 create mode 100644 rust/src/error.rs
 create mode 100644 rust/src/handle.rs
 create mode 100644 rust/src/lib.rs
 create mode 100644 rust/src/types.rs
 create mode 100644 rust/src/utils.rs
 create mode 100644 rustfmt.toml

diff --git a/.gitignore b/.gitignore
index efe3080..ac514b0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -174,6 +174,16 @@ Makefile.in
 /python/nbd.py
 /python/run-python-tests
 /run
+/rust/Cargo.lock
+/rust/libnbd-sys/Cargo.lock
+/rust/libnbd-sys/libnbd_version
+/rust/libnbd-sys/src/lib.rs
+/rust/src/async_bindings.rs
+/rust/src/bindings.rs
+/rust/target
+/rust/cargo_test/Cargo.lock
+/rust/cargo_test/target
+/rust/run-tests.sh
 /sh/nbdsh
 /sh/nbdsh.1
 /stamp-h1
diff --git a/.ocamlformat b/.ocamlformat
new file mode 100644
index 000..7bfe155
--- /dev/null
+++ b/.ocamlformat
@@ -0,0 +1,4 @@
+profile = default
+version = 0.25.1
+wrap-comments = true
+margin = 78
diff --git a/Makefile.am b/Makefile.am
index 243fabd..9f7707a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -28,6 +28,7 @@ EXTRA_DIST = \
README.md \
scripts/git.orderfile \
SECURITY \
+   rustfmt.toml \
$(NULL)
 
 CLEANFILES += m4/*~
@@ -55,6 +56,7 @@ SUBDIRS = \
ocaml/tests \
golang \
golang/examples \
+   rust \
interop \
fuzzing \
bash-completion \
diff --git a/configure.ac b/configure.ac
index 0b94f5e..816b59e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -613,6 +613,32 @@ AS_IF([test "x$enable_golang" != "xno"],[
 ],[GOLANG=no])
 AM_CONDITIONAL([HAVE_GOLANG],[test "x$GOLANG" != "xno"])
 
+dnl Rust.
+AC_ARG_ENABLE([rust],
+AS_HELP_STRING([--disable-rust], [disable Rust language bindings]),
+[],
+[enable_rust=yes])
+AS_IF([test "x$enable_rust" != "xno"],[
+AC_CHECK_PROG([CARGO],[cargo],[cargo],[no])
+AC_CHECK_PROG([RUSTFMT],[rustfmt],[rustfmt],[no])
+AS_IF([test "x$CARGO" != "xno"],[
+AC_MSG_CHECKING([if $CARGO is usable])
+AS_IF([ (
+  cd $srcdir/rust/cargo_test &&
+   $CARGO test 2>&AS_MESSAGE_LOG_FD 1>&2 &&
+   $CARGO doc 2>&AS_MESSAGE_LOG_FD 1>&2 &&
+   $CARGO fmt 2>&AS_MESSAGE_LOG_FD 1>&2
+   ) ],[
+AC_MSG_RESULT([yes])
+],[
+AC_MSG_RESULT([no])
+AC_MSG_WARN([Rust ($CARGO) is installed but not usable])
+CARGO=no
+])
+])
+],[CARGO=no])
+AM_CONDITIONAL([HAVE_RUST],[test "x$CARGO" != "xno" -a "x$RUSTFMT" != "xno"])
+
 AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime])
 NODELETE=
 `$LD --help 2>&1 | grep -- "-z nodelete" >/dev/null` && \
@@ -643,6 +669,8 @@ AC_CONFIG_FILES([run],
 [chmod +x,-w run])
 AC_CONFIG_FILES([sh/nbdsh],
 [chmod +x,-w sh/nbdsh])
+AC_CONFIG_FILES([rust/run-tests.sh],
+[chmod +x,-w rust/run-tests.sh])
 
 AC_CONFIG_FILES([Makefile
  bash-completion/Makefile
@@ -657,6 +685,7 @@ AC_CONFIG_FILES([Makefile
  generator/Makefile
  golang/Makefile
  golang/examples/Makefile
+ rust/Makefile
  include/Makefile
  info/Makefile