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