On Wed, Feb 21, 2024 at 12:19:47PM -0600, Eric Blake wrote:
I noticed during integration testing that nbd-server blindly reports a size of 0 for all NBD_OPT_INFO requests, unless I pass a size argument on the command line to nbd-server. At first, I thought it was a side effect of me trying to use nbd-server on a block device (an LVM partition), as it is a common bug to rely on stat().st_size which only works for regular files (a block device has to use lseek(SEEK_END)); but then I noticed it happening when using nbd-server to serve regular files as well.
I then turned to the source code, where I see that client->exportsize is set in just these places:
commit_client() - exportsize = OFFT_MAX, then try setupexport()
setupexport() - default to client->server->expected_size (if one was provided), further validating that actual size is large enough when actual size can be computed - if neither treefile or F_WAIT is set, compute actual size by opening one or more files and using size_autodetect() (which does the right thing for block devices, so my earlier thought about over-reliance on stat() was wrong)
but these functions are only reached for NBD_OPT_EXPORT_NAME and NBD_OPT_GO, not NBD_OPT_INFO.
Yeah, that's a bug. Obviously we don't want to commit_client when we do OPT_INFO (it's meant to do "everything needed to make sure the export is ready to go"), but I apparently missed that it also makes sure it has certain bits of information that OPT_INFO needs.
The upshot is that for NBD_OPT_GO, there are some scenarios (treefile, F_WAIT) where nbd-server advertises a size of 9223372036854775807 (0x7fffffff_ffffffff) meaning unknown, but a size of 0 there is only possible if the file was successfully opened and really is zero bytes in length. Conversely, NBD_OPT_INFO is always advertising a size of 0, which means most of the time, the size changes between NBD_OPT_INFO and NBD_OPT_GO.
That should not happen :) I think the standard is clear enough that OPT_INFO and OPT_GO should return the same information. The fact that I messed up doesn't mean I think we should change the standard :-) -- w@uter.{be,co.za} wouter@{grep.be,fosdem.org,debian.org} I will have a Tin-Actinium-Potassium mixture, thanks.