[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: what size should NBD_OPT_INFO report?



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.


Reply to: