On Tue, May 21, 2019 at 10:15:48PM -0500, Eric Blake wrote: > The next patch wants to make queuing commands smarter; rather than > duplicating the logic at each command's call site, it's better to > centralize things in command_common, and to also make this helper > routine visible for use in NBD_CMD_DISC.
There's a few problems with this patch. To avoid leaking symbols -- even though we have a linker script, not all future platforms are expected to support it -- global symbols which are not exported should be called nbd_internal_*. So command_common should be nbd_internal_command_common. The bigger problem is: > @@ -505,8 +495,5 @@ nbd_unlocked_aio_block_status (struct nbd_connection > *conn, > cmd->extent_fn = extent; > cmd->extent_id = id; > > - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) > - return -1; > - > return cmd->handle; This changes the semantics of the function because the cmd->extent_* fields are set too late. (Yes, command_common is rather ad hoc :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
