Libguestfs - Mar 2020 - Re: nbdkit / mingw support

On 3/24/20 3:12 PM, Eric Blake wrote:
>> (For non-mingw platforms) this breaks the source API promises rather
>> seriously, so if I understand your proposal correctly I don't think
>> this is a good idea.  It's possibly something we can consider for
>> internal plugins, or for the V3 API.
> 
> How does it break API to request that someone link against a particular 
> library if they want to avoid undefined symbols (and to continue to 
> allow the to link with undefined symbols if they choose not to compile 
> against that library)?
I guess what I haven't said yet is that the existing proposed patches 
from Frank for making mingw compile are NOT the right way to resolve the 
need for -no-undefined on that platform.  Instead of creating lots of 
wrapper functions buried inside #ifdef WINDOWS_COMPAT, we really should 
be focusing on creating a clean libnbdkit.so/dll library that exposes 
all of the needed symbols without the need for preprocessor magic, and 
therefore without API breaks.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

I think I understand now that libnbdkit.so won't break the ABI for
existing plugins.  Does it require that plugins for newer nbdkit use
-lnbdkit (which would be a source API break) or would it still be
possible to compile without this?  I guess as long as plugins do not
start using -no-undefined then it would still work, so it wouldn't be
a source API break.

I had a look into how we might implement libnbdkit.so.  Some functions
are obviously self-contained (eg. nbdkit_parse_*, nbdkit_realpath,
nbdkit_debug, nbdkit_error, nbdkit_*extents).

Unfortunately some functions depend themselves on internals
of the server:

 * nbdkit_nanosleep, nbdkit_export_name, nbdkit_peer_name call
   threadlocal_get_conn
 * nbdkit_set_error calls threadlocal_set_error
 * nbdkit_shutdown must set the quit global (or call a server function)

I guess we can deal with the first ones by moving threadlocal.c into
the same library, although it's a bit awkward.  The quit flag is still
more awkward because you have to move a lot of quit pipe handling code
into the library which has knock-on effects all over.

Rich.

----------------------------------------------------------------------

server/debug.c:nbdkit_vdebug (const char *fs, va_list args)
server/debug.c:nbdkit_debug (const char *fs, ...)
server/extents.c:nbdkit_extents_new (uint64_t start, uint64_t end)
server/extents.c:nbdkit_extents_free (struct nbdkit_extents *exts)
server/extents.c:nbdkit_extents_count (const struct nbdkit_extents *exts)
server/extents.c:nbdkit_get_extent (const struct nbdkit_extents *exts, size_t i)
server/extents.c:nbdkit_add_extent (struct nbdkit_extents *exts,
server/log.c:nbdkit_verror (const char *fs, va_list args)
server/log.c:nbdkit_error (const char *fs, ...)
server/log.c:nbdkit_vfprintf(FILE *f, const char *fmt, va_list args)
server/plugins.c:nbdkit_set_error (int err)
server/public.c:nbdkit_absolute_path (const char *path)
server/public.c:nbdkit_realpath (const char *path)
server/public.c:nbdkit_parse_int (const char *what, const char *str, int *rp)
server/public.c:nbdkit_parse_int8_t (const char *what, const char *str, int8_t
*rp)
server/public.c:nbdkit_parse_int16_t (const char *what, const char *str, int16_t
*rp)
server/public.c:nbdkit_parse_int32_t (const char *what, const char *str, int32_t
*rp)
server/public.c:nbdkit_parse_int64_t (const char *what, const char *str, int64_t
*rp)
server/public.c:nbdkit_parse_unsigned (const char *what, const char *str,
unsigned *rp)
server/public.c:nbdkit_parse_uint8_t (const char *what, const char *str, uint8_t
*rp)
server/public.c:nbdkit_parse_uint16_t (const char *what, const char *str,
uint16_t *rp)
server/public.c:nbdkit_parse_uint32_t (const char *what, const char *str,
uint32_t *rp)
server/public.c:nbdkit_parse_uint64_t (const char *what, const char *str,
uint64_t *rp)
server/public.c:nbdkit_parse_size (const char *str)
server/public.c:nbdkit_parse_bool (const char *str)
server/public.c:nbdkit_read_password (const char *value, char **password)
server/public.c:nbdkit_nanosleep (unsigned sec, unsigned nsec)
server/public.c:nbdkit_export_name (void)
server/public.c:nbdkit_peer_name (struct sockaddr *addr, socklen_t *addrlen)
server/quit.c:nbdkit_shutdown (void)



-- 
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

On 3/25/20 7:16 AM, Richard W.M. Jones wrote:
> I think I understand now that libnbdkit.so won't break the ABI for
> existing plugins.  Does it require that plugins for newer nbdkit use
> -lnbdkit (which would be a source API break) or would it still be
> possible to compile without this?  I guess as long as plugins do not
> start using -no-undefined then it would still work, so it wouldn't be
> a source API break.
Yes, that's what I'm thinking - it's an API break if you have to add
in
-lnbdkit; but you only have to add that if you want to use 
-no-undefined. Plugins compiled without -lnbdkit (either because they 
predate the library, or because they specifically did not care about 
-no-undefined) should still be viable.
> 
> I had a look into how we might implement libnbdkit.so.  Some functions
> are obviously self-contained (eg. nbdkit_parse_*, nbdkit_realpath,
> nbdkit_debug, nbdkit_error, nbdkit_*extents).
> 
> Unfortunately some functions depend themselves on internals
> of the server:
> 
>   * nbdkit_nanosleep, nbdkit_export_name, nbdkit_peer_name call
>     threadlocal_get_conn
>   * nbdkit_set_error calls threadlocal_set_error
>   * nbdkit_shutdown must set the quit global (or call a server function)
Yeah, there's some awkward dependencies to figure out. It's obvious the 
library has to export public nbdkit_* interfaces for the sake of 
plugins, but can it also export one additional symbol _nbdkit_init() for 
internal use?  Then we can have the nbdkit binary pass whatever 
additional hooks are needed for proper isolation of the public interface 
(a callback pointer to get at threadlocal_get_conn, 
threadlocal_set_error, the address of the quit global, ...), and leave 
the symbol undocumented (plus the fact that the leading _ will 
discourage plugins from trying to abuse it).
> 
> I guess we can deal with the first ones by moving threadlocal.c into
> the same library, although it's a bit awkward.  The quit flag is still
> more awkward because you have to move a lot of quit pipe handling code
> into the library which has knock-on effects all over.
The other extreme is to have the entire nbdkit engine in libnbdkit.so, 
plus the addition of a single internal-only callback _nbdkit_main(), 
then the nbdkit binary becomes a bare-bones:
int main (int argc, char *argv[]) {
   return _nbdkit_main (argc, argv);
}

at which point you don't have to decide which functionality lives where. 
  After all, you already have a linker script that limits what the main 
binary exports; which really becomes what libnbdkit.so has to export. 
If we are worried about plugins trying to abuse that additional entry 
point, you can have _nbdkit_main set a flag on first execution so that 
subsequent attempts to call it fail immediately; but I'm not too worried 
about it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org