On Tue, Apr 21, 2020 at 11:44:59AM +0100, Richard W.M. Jones wrote:
> Thanks: Dan Berrangé
>
> XXX UNFINISHED:
>
> - Is using uintptr for the handle a good idea? Plugins must return
> something != 0. In other languages we would allow plugins to
> return an arbitrary object here, but this is not possible in golang
> because of lack of GC roots.
Yeah, this is the bit that looks wierd to me when thinking about this
from a Go dev POV. You asked me on IRC whether we should have a separate
interface for a connection object, and this makes me think that we should
indeed do that.
Of course we still have the problem that C code is forbidden to keep
a pointer to a Go object across method calls.
IIUC, the C layuer in nbdkit treats the "void *" as a completely
opaque value, so we don't actually have to store any valid pointer
as the handle at all. It can be literally any value that fits in
a 32-bit pointer. Thus we can keep a global variable mapping the
GO objects to fake C "pointers"
So it would look like this:
// The plugin interface.
type PluginInterface interface {
// Open, GetSize and PRead are required for all plugins.
// Other methods are optional.
Config(key string, value string) error
ConfigComplete() error
Open(readonly bool) (PluginConnectionInterface, error)
}
type PluginConnectionInterface interface {
// Open, GetSize and PRead are required for all plugins.
// Other methods are optional.
Close()
GetSize() (uint64, error)
PRead(buf []byte, offset uint64, flags uint32) error
}
Now, we need todo some mapping
var connectionID uint
var connectionMap map[uint]PluginConnectionInterface
//export implOpen
func implOpen(c_readonly C.int) unsafe.Pointer {
readonly := false
if c_readonly != 0 {
readonly = true
}
h, err := pluginImpl.Open(readonly)
if err != nil {
set_error(err)
return nil
}
if h == 0 {
panic("Open method: handle must be != 0")
}
id := connectionID++
connectionMap[id] = h
return unsafe.Pointer(uintptr(id))
}
func getConn(handle unsafe.Pointer) PluginConnectionInterface {
id := uint(uintptr(handle))
conn, ok = connectionMap[id]
if !ok {
panic("Connection %d was not open", id)
}
}
//export implClose
func implClose(handle unsafe.Pointer) {
conn := getConn(handle)
conn.Close()
delete(connectionMap, id)
}
//export implGetSize
func implGetSize(handle unsafe.Pointer) C.int64_t {
conn := getConn(handle)
size, err := conn.GetSize()
if err != nil {
set_error(err)
return -1
}
return C.int64_t(size)
}
> diff --git a/plugins/golang/test/test.go b/plugins/golang/test/test.go
> new file mode 100644
> index 00000000..f5b1a33b
> --- /dev/null
> +++ b/plugins/golang/test/test.go
type TestPlugin struct {
nbdkit.Plugin
}
type TestPluginConnection struct {
nbdkit.PluginConnection
...other per-connection fields...
}
var pluginName = "test"
var size uint64
var size_set = false
func (p TestPlugin) Config(key string, value string) error {
if key == "size" {
var err error
size, err = strconv.ParseUint(value, 0, 64)
if err != nil {
return err
}
size_set = true
return nil
} else {
return nbdkit.PluginError{Errmsg: "unknown parameter"}
}
}
func (p TestPlugin) ConfigComplete() error {
if !size_set {
>+ return nbdkit.PluginError{Errmsg: "size parameter is required"}
}
return nil
}
func (p TestPlugin) Open(readonly bool) (nbdkit.PluginConnectionInterface,
error) {
nbdkit.Debug("golang code running in the .open callback")
return &TestPluginConnection{}, nil
}
func (p TestPluginConnection) GetSize() (uint64, error) {
nbdkit.Debug("golang code running in the .get_size callback")
return size, nil
}
func (p TestPluginConnection) PRead(buf []byte, offset uint64, flags uint32)
error {
nbdkit.Debug("golang code running in the .pread callback")
for i := 0; i < len(buf); i++ {
buf[i] = 0
}
return nil
}
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 :|
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
