On 9/15/19 9:55 AM, Richard W.M. Jones wrote: > The source for the easter egg example is not included, but it is: > > org 0x7c00 > ; clear screen > mov ah,0 > mov al,3 > int 0x10 > ; print string > mov ah,0x13 > mov bl,0xa > mov al,1 > mov cx,len > mov dh,0 > mov dl,0 > mov bp,hello > int 0x10 > hlt > hello: db "*** Hello from nbdkit! ***",0xd,0xa > len: equ $-hello > ; pad to end of sector > times 510-($-$$) db 0 > ; boot signature > db 0x55,0xaa > ---
> +++ b/plugins/reflection/nbdkit-reflection-plugin.pod > @@ -0,0 +1,104 @@ > +=head1 NAME > + > +nbdkit-reflection-plugin - reflect export name back to the client > + > +=head1 SYNOPSIS > + > + nbdkit reflection [mode=]exportname|base64exportname > + > +=head1 DESCRIPTION > + > +C<nbdkit-reflection-plugin> is a test plugin which reflects > +information sent by the client back to the client. > + > +In its default mode (C<mode=exportname>) it converts the export name > +passed from the client into a disk image. C<mode=base64exportname> is > +similar except the client must base64-encode the data in the export > +name, allowing arbitrary binary data to be sent (see L</EXAMPLES> > +below to make this clearer). Is it worth noting that the NBD protocol imposes a 4k limit on the export name, which would limit things to about a 3k disk image when using base64? (It looks like nbdkit does not currently enforce that limit -- maybe it should, but as a separate patch -- but even if we don't change that, you're still limited to finding a client willing to send an export name larger than the protocol permits) > + > +The plugin only supports read-only access. To make the disk writable, > +add L<nbdkit-cow-filter(1)> on top. > + > +=head1 EXAMPLES > + > +Create a reflection disk. By setting the export name to C<"hello"> > +when we open it, a virtual disk of only 5 bytes containing these > +characters is created. We then display the contents: > + > + $ nbdkit reflection mode=exportname > + $ nbdsh -u 'nbd://localhost/hello' -c - <<'EOF' > + size = h.get_size() > + print("size = %d" % size) > + buf = h.pread(size, 0) > + print("buf = %r" % buf) > + EOF This leaves nbdkit running as a background daemon after the fact. Is it worth implementing a new 'nbdkit --one-shot' flag that causes nbdkit to exit after the first client disconnects? I also wonder if we want to add a way to differentiate between SIGINT (terminate nbdkit as soon as possible) vs. SIGHUP (no new clients permitted, but keep nbdkit up and running for as long as current clients stay connected) [it would have to be a new command line option, as existing users are expecting SIGHUP and SIGINT to behave identically, and the idea is somewhat at odds with our other idea in TODO of having the v3 protocol have strongly-typed plugin parameters with a way to send SIGHUP as a way to do a live reload of parameters backed by a file] > +=head1 SEE ALSO > + > +L<nbdkit(1)>, > +L<nbdkit-plugin(3)>, > +L<nbdkit-cow-filter(1)>, > +L<nbdkit-data-plugin(1)>, > +L<nbdkit-memory-plugin(1)/Preloading small amounts of data>. Except that nbdkit-memory-plugin points only to nbdkit-data-plugin, so is this link helping? > +++ b/plugins/reflection/reflection.c > +/* The mode. */ > +enum mode { > + MODE_EXPORTNAME, > + MODE_BASE64EXPORTNAME, > +}; > +static enum mode mode = MODE_EXPORTNAME; Explicit assignment to a 0 value, instead of relying on .bss; but I'm okay with it (it's not as obvious as =0 or =false). > + > +static int > +reflection_config (const char *key, const char *value) > +{ > + if (strcmp (key, "mode") == 0) { > + if (strcasecmp (value, "exportname") == 0) { Do we want to be nice and allow 'export-name' as an [undocumented] alternative spelling? > + mode = MODE_EXPORTNAME; > + } > + else if (strcasecmp (value, "base64exportname") == 0) { similarly for 'base64-export-name' > +#define reflection_config_help \ > + "mode=MODE Plugin mode." > + Worth listing the valid values of MODE, or the fact that this parameter is optional because it defaults to exportname? > +/* Create the per-connection handle. > + * > + * This is a rather unusual plugin because it has to parse data sent > + * by the client. For security reasons, be careful about: > + * > + * - Returning more data than is sent by the client. > + * > + * - Inputs that result in unbounded output. > + * > + * - Inputs that could hang, crash or exploit the server. > + */ Nice reminder. I agree that we're okay for now, but as we add new modes in the future, it will remind us to think about it. > +/* Read-only plugin so multi-conn is safe. */ > +static int > +reflection_can_multi_conn (void *handle) > +{ > + return 1; Correct for now. I also see your followup to patch 4 that changes this once IP addresses are exposed. > +++ b/tests/test-reflection-base64.sh > + > +requires nbdsh --version > +# XXX This needs to test $PYTHON somehow. > +#requires python -c 'import base64' Not just any python, but the version of python hard-coded as what will run nbdsh (which may be different than the $PYTHON that nbdkit was compiled with). I'm not sure what to suggest, other than just: requires nbdsh -c 'import base64' which could, in fact, be a single test (no need to test if nbdsh --version works if nbdsh -c ... works). > + > +export e sock > +for e in "" "test" "ใในใ" \ Worth also testing '-n' or '\n' (two strings that caused problems with the sh plugin implementation) (here, and in the plaintext version)? > + > +# Test that it fails if the caller passes in non-base64 data. The > +# server drops the connection in this case so it's not very graceful > +# but we should at least get an nbd.Error and not something else. > +nbdsh -c ' > +import os > +import sys > + > +h.set_export_name ("xyz") > +try: > + h.connect_unix (os.environ["sock"]) > + # This should not happen. > + sys.exit (1) > +except nbd.Error as ex: > + sys.exit (0) > +# This should not happen. > +sys.exit (1) The test looks good. (Yeah, figuring out how to make it more graceful in the future might be nice, but that's not this patch's problem. I'm thinking that if a plugin's .open() fails, nbdkit could reply with NBD_REP_ERR_UNKNOWN or NBD_REP_ERR_INVALID, and then wait for the client to disconnect, rather than the current hard hangup initiated by the server) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs