On 7/21/20 7:41 PM, Eric Blake wrote:

The handling of NBD_OPT_SET_META_CONTEXT interacted with the export
name (see commit 20b8509a9ccdab118ce7b7043be63bbad74f1e79).  I have
attempted to keep previous behaviour in this change, but note that
there is no regression test for this.  I added a debug message so we
can tell when this unusual case actually happens which should help
with diagnosis of problems.

Yeah, that commit specifically mentioned that I used gdb breakpoints in qemu-io to test things, and was not interested in hacking libnbd at the time.  Although now that we are debating about exposing nbd_opt_* commands in libnbd to someone that opts in, maybe that _could_ be a case to write such a regression test.  Meanwhile, I'll just try to fire up another gdb session to prove to myself that things still work.


Here's snippets of my gdb session proving it does indeed still work (now that you've actually pushed your patches):

$ ./nbdkit -U - -v memory 1 --run \
  'gdb --args qemu-img map --output=json "$uri"'
...
Reading symbols from qemu-img...
(gdb) b nbd_send_meta_query
Breakpoint 1 at 0xf3773: file /home/eblake/qemu/nbd/client.c, line 653.
(gdb) r
Starting program: /home/eblake/qemu/qemu-img map --output=json nbd+unix://\?socket=/tmp/nbdkityhz8Ab/socket
...
nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_STRUCTURED_REPLY: client requested structured replies

Thread 1 "qemu-img" hit Breakpoint 1, nbd_send_meta_query (ioc=0x55555583f720, opt=10, export=0x5555557e7e80 "", query=0x55555571994c "base:allocation",
    errp=0x7fffffffcc20) at /home/eblake/qemu/nbd/client.c:653
653         uint32_t export_len = strlen(export);
(gdb) p export="a"
$1 = 0x7fffecd80fa0 "a"
(gdb) c
...
nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT: client requested export 'a' nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT: set count: 1 nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT: set base:allocation nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT: replying with base:allocation id 1 nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT: reply complete nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_GO: client requested export '' nbdkit: memory[1]: debug: newstyle negotiation: NBD_OPT_SET_META_CONTEXT export name "a" ≠ final client exportname "", so discarding the previous context
...

nbdkit: memory.1: error: invalid request: NBD_CMD_BLOCK_STATUS: base:allocation was not negotiated
nbdkit: debug: starting worker thread memory.13
nbdkit: debug: starting worker thread memory.14
nbdkit: debug: starting worker thread memory.15
nbdkit: memory.1: debug: sending error reply: Invalid argument
qemu-img: Could not read file metadata: Invalid argument
...

So using gdb to hack in a different name shows the nbdkit code works; it also shows that qemu does NOT expect NBD_CMD_BLOCK_STATUS to fail if NBD_OPT_SET_META_CONTEXT succeeded (but that's understandable - qemu always sends the same export name; the fault lies in me changing state with gdb behind qemu's back).

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

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to