Libguestfs - Apr 2020 - Re: [PATCH nbdkit UNFINISHED] Add the ability to write plugins in golang.

On Fri, Apr 10, 2020 at 02:51:52PM +0100, Richard W.M. Jones
wrote:
> Similar to C, OCaml and Rust, this is not a plugin per se.  Instead
> it's more of a method and set of tests around writing plugins in
> golang.  They are standalone programs that compile into shared objects
> that nbdkit can then load (so there is no "go plugin" between
nbdkit
> and the user plugin, unlike in scripting languages like Perl).
Why did you choose this approach ?

Looking at the code below there's the general boilerplate that
will be approx the same for all plugins with cut+paste tedium
across projects, and then there's the "interesting" code in
test.go

The methods in test.go though look quite unappealing from a
Go programmer's POV, as the API contracts are full of CGo
types and unsafe pointers.

To make something that is attractive for Go programmers, I
think it needs to hide all the low level CGo stuff entirely.

Implementing a nbdkit plugin should require nothing more
that providing pure Go code that satisfies  a well defined
Go "interface" type definition. There should not be any
copy+paste of example boilerplate, nor any use of CGo.

As an illustration, consider an interface and basic infrastructure:


  package nbdkitplugin


  type NBDKitFeature int
  const (
      NBDKitFeatureGetSize NBDKitFeature = iota
      NBDKitFeaturePread
      ...other optional methods...
  )

  type NBDKitPlugin interface {
      NBDGetFeatures() []NBDKitFeature
      NBDOpen(readonly bool)
      NBDClose()
      NBDGetSize() int64
      NBDPRead(count uint32, offset uint64, flags uint32) ([]byte)
      ....many other methods...
  }

  var plugin *C.struct_nbdkit_plugin
  var pluginImpl NBDKitPlugin

  func PluginInit(name string, impl NBDKitPlugin) {
      pluginImpl = impl

      features = impl.GetFeatures()
      plugin.name = C.CString(name)
      plugin.open = (*[0]byte)(C.open_wrapper)
      plugin.close = (*[0]byte)(C.close_wrapper)

      for _, feature := range features {
          switch feature {
              case NBDKitFeatureGetSize:
                plugin.get_size = (*[0]byte)(C.get_size_wrapper)
              case NBDKitFeaturePread:
                plugin.pread = (*[0]byte)(C.pread_wrapper)
          }
      }
      
  }

  ....all the methods like C.pread_wrapper/C.get_size_wrapper
      need to invoke pluginImpl methods....


  // This type implements all methods in NBDKitPlugin, with
  // no-op impls. This means that people implementing plugins
  // don't need to implement every method in the interface,
  // only the few they care about
  type PluginBase struct {
  }

  // We don't implement anytrhing by default
  func (plugin *PluginBase) NBDGetFeatures() []NBDKitFeature {
        return []NBDKitFeature{}
  }

  func (plugin *PluginBase) NBDOpen(readonly bool) {}
 
  func (plugin *PluginBase) NBDClose() {}
 
  func (plugin *PluginBase) NBDGetSize() int64 { return 0 }
  }
 
  func (plugin *PluginBase) NBDPRead(count uint32, offset uint64, flags uint32)
[]byte {
 	return []byte{}
  }

  ...  no-op impls of all other methods for NBDKitPlugin interface...


This code above would all be a standalone go module that is just
imported.


Now an impl of a plugin becomes just one single file

  import (
     libguestfs.org/nbdkitplugin
  )


  type TestPlugin struct {
      nbdkitplugin.PluginBase   // This provides no-op impls of all methods
      ...blah...
  }

  func NewTestPlugin() NBDKitPlugin {
      return &TestPlugin{
         ....blah...
      }
  }

  // This declares which methods we're actually implementing
  func (plugin *TestPlugin) NBDGetFeatures() []NBDKitFeature {
        return []NBDKitFeature{
	    NBDKitFeatureGetSize,
	    NBDKitFeaturePread,
	}
  }

  func (plugin *TestPlugin) NBDOpen(readonly bool) {
 	nbdkit.Debug("golang code running in the .open callback")
  }
 
  func  (plugin *TestPlugin) NBDClose() {
 	nbdkit.Debug("golang code running in the .close callback")
  }
 
  func (plugin *TestPlugin) NBDGetSize() int64 {
 	nbdkit.Debug("golang code running in the .get_size callback")
 	return 1024 * 1024
  }
 
  func (plugin *TestPlugin) NBDPRead(count uint32, offset uint64, flags uint32)
[]byte {
 	nbdkit.Debug("golang code running in the .pread callback")
 	return []byte{}
  }

   ... don't need to implement any other methods, since PluginBase
       satisfies the interface contract....

  func init() {
       nbdkitplugin.PluginInitialize("test", NewTestPlugin())
  }


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, Apr 15, 2020 at 05:27:07PM +0100, Daniel P. Berrangé
wrote:
> On Fri, Apr 10, 2020 at 02:51:52PM +0100, Richard W.M. Jones wrote:
> > Similar to C, OCaml and Rust, this is not a plugin per se.  Instead
> > it's more of a method and set of tests around writing plugins in
> > golang.  They are standalone programs that compile into shared objects
> > that nbdkit can then load (so there is no "go plugin"
between nbdkit
> > and the user plugin, unlike in scripting languages like Perl).
> 
> Why did you choose this approach ?
Do you mean the design of constructing the .so file directly using
--buildmode=c-shared?  I wasn't aware there was any other way to do
it.  Should we use some kind of module loading instead?
> Looking at the code below there's the general boilerplate that
> will be approx the same for all plugins with cut+paste tedium
> across projects, and then there's the "interesting" code in
> test.go
> 
> The methods in test.go though look quite unappealing from a
> Go programmer's POV, as the API contracts are full of CGo
> types and unsafe pointers.
>
> To make something that is attractive for Go programmers, I
> think it needs to hide all the low level CGo stuff entirely.
> 
> Implementing a nbdkit plugin should require nothing more
> that providing pure Go code that satisfies  a well defined
> Go "interface" type definition. There should not be any
> copy+paste of example boilerplate, nor any use of CGo.
I don't disagree, I just have no idea how to achieve this.
> As an illustration, consider an interface and basic infrastructure:
[...]

I'll have a go at changing it like this, but I don't see how this
changes any of the fundamental problems, but maybe I will understand
more once I've done that.

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

On Wed, Apr 15, 2020 at 06:04:26PM +0100, Richard W.M. Jones
wrote:
> On Wed, Apr 15, 2020 at 05:27:07PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Apr 10, 2020 at 02:51:52PM +0100, Richard W.M. Jones wrote:
> > > Similar to C, OCaml and Rust, this is not a plugin per se. 
Instead
> > > it's more of a method and set of tests around writing plugins
in
> > > golang.  They are standalone programs that compile into shared
objects
> > > that nbdkit can then load (so there is no "go plugin"
between nbdkit
> > > and the user plugin, unlike in scripting languages like Perl).
> > 
> > Why did you choose this approach ?
> 
> Do you mean the design of constructing the .so file directly using
> --buildmode=c-shared?  I wasn't aware there was any other way to do
> it.  Should we use some kind of module loading instead?
No, I was mostly meaning the "copylib" approach to plugins, where the
plugin author owns the whole stack.

I was just thinking of what the "ideal" would be from a Go programmers
POV, and that I think involves the least possible exposure to CGo and
the native nbdkit APIs.
> > Looking at the code below there's the general boilerplate that
> > will be approx the same for all plugins with cut+paste tedium
> > across projects, and then there's the "interesting" code
in
> > test.go
> > 
> > The methods in test.go though look quite unappealing from a
> > Go programmer's POV, as the API contracts are full of CGo
> > types and unsafe pointers.
> >
> > To make something that is attractive for Go programmers, I
> > think it needs to hide all the low level CGo stuff entirely.
> > 
> > Implementing a nbdkit plugin should require nothing more
> > that providing pure Go code that satisfies  a well defined
> > Go "interface" type definition. There should not be any
> > copy+paste of example boilerplate, nor any use of CGo.
> 
> I don't disagree, I just have no idea how to achieve this.
> 
> > As an illustration, consider an interface and basic infrastructure:
> [...]
> 
> I'll have a go at changing it like this, but I don't see how this
> changes any of the fundamental problems, but maybe I will understand
> more once I've done that.
I think the hairy CGo/libnbdkit interaction problems still exist in what
I'm describing. Mostly I'm just thinking about how they can be isolated
so that they owned by nbdkit maintainers in one place, and not copied
around, nor even seen by plugin authors.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|