nutanix / libvfio-user

framework for emulating devices in userspace
BSD 3-Clause "New" or "Revised" License
162 stars 51 forks source link

lib: export dma_sg_size symbol in library #664

Closed berrange closed 2 years ago

berrange commented 2 years ago

The dma_sg_size() method is listed in libvfio-user.h but the symbol is marked private in the ELF library.

jlevon commented 2 years ago

Thanks. Unfortunately SPDK (and presumably qemu) don't pick this up because they statically link. Do you mind adding a test. Something like this:

diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py
index dda4038..b7129cb 100644
--- a/test/py/libvfio_user.py
+++ b/test/py/libvfio_user.py
@@ -610,6 +610,7 @@ lib.vfu_setup_device_dma.argtypes = (c.c_void_p, vfu_dma_register_cb_t,
                                      vfu_dma_unregister_cb_t)
 lib.vfu_setup_device_migration_callbacks.argtypes = (c.c_void_p,
     c.POINTER(vfu_migration_callbacks_t), c.c_uint64)
+lib.dma_sg_size.restype = (c.c_size_t)
 lib.vfu_addr_to_sg.argtypes = (c.c_void_p, c.c_void_p, c.c_size_t,
                                c.POINTER(dma_sg_t), c.c_int, c.c_int)
 lib.vfu_map_sg.argtypes = (c.c_void_p, c.POINTER(dma_sg_t), c.POINTER(iovec_t),
@@ -1142,6 +1143,9 @@ def vfu_setup_device_migration_callbacks(ctx, cbs=None, offset=0x4000):
     return lib.vfu_setup_device_migration_callbacks(ctx, cbs, offset)

+def dma_sg_size():
+    return lib.dma_sg_size()
+
 def vfu_addr_to_sg(ctx, dma_addr, length, max_sg=1,
                    prot=(mmap.PROT_READ | mmap.PROT_WRITE)):
     assert ctx is not None
diff --git a/test/py/test_map_unmap_sg.py b/test/py/test_map_unmap_sg.py
index fa98159..2e87eeb 100644
--- a/test/py/test_map_unmap_sg.py
+++ b/test/py/test_map_unmap_sg.py
@@ -34,6 +34,9 @@ import tempfile

 ctx = None

+def test_dma_sg_size():
+    size = dma_sg_size()
+    assert size == len(dma_sg_t())

 def test_map_sg_with_invalid_region():
     global ctx

I'll take a look at another way of testing too

berrange commented 2 years ago

Thanks. Unfortunately SPDK (and presumably qemu) don't pick this up because they statically link. Do you mind adding a test. Something like this:

FYI I found this when experimenting with replacing cmake with meson, because I made the existing unit tests use the shared library instead of static library. I figure that's probably a good enough test, but I wanted to just submit this patch on its own since its a standalone issue and other stuff will take a bit more time.

berrange commented 2 years ago

Added the suggested test suite changes now

jlevon commented 2 years ago

Thank you, merged.