[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:49PM -0600, Eric Blake wrote:
> 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.  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.

Amending myself: reading size_autodetect(), it tries ioctl(fd,
BLKGETSIZE64) first (even when fd is not a block device!), then falls
back to fstat(fd) coupled with lseek(SEEK_END) if fstat() reported
st_size of 0 and not a regular file; but if all of those fail, it
reports UINT64_MAX, which is different than OFFT_MAX.

> 
> For comparison, both nbdkit and qemu as an NBD server always advertise
> the same size for both NBD_OPT_INFO and NBD_OPT_GO (but it should also
> be noted that neither of these has the complexity of multifile like
> nbd-server).
> 
> Should we tweak the NBD standard to recommend that a server advertise
> a size of OFFT_MAX for NBD_OPT_INFO if it is prohibitive to determine
> an actual size, rather than 0?  Furthermore, is it worth adding code
> to make NBD_OPT_INFO try harder to provide a sensible value when
> possible (if expected_size was set, or if we are not multifile or
> F_WAIT, then a stat() is enough to get the size for serving a regular
> file; and if stat() says we have a block device, we can still try the
> open/lseek/close)?
> 
> Also, is it worth trying to specify that since no known NBD servers
> allow exports with sizes larger than OFFT_MAX, a server MUST NOT
> report a size equal or larger than 9223372036854775808
> (0x80000000_00000000)?  There are definitely a few places that I could
> simplify in libnbd if we have a protocol guarantee that a valid export
> size will never have the most significant bit set, and therefore we
> don't have to worry about whether size is represented as a signed or
> unsigned value (which also implies that libnbd's function
> nbd_get_size() returning ssize_t instead of size_t is acceptable).
> But if we do that, then maybe 0x80000000_00000000 or even
> 0xffffffff_ffffffff would serve as a better recommended sentinel than
> 0x7fffffff_ffffffff for representing an indeterminate size.

And the fact that nbd-server already has two different sentinel values
(depending on whether it even attempted an open, vs whether the
attempt to read size from an open fd failed) is another factor into
what makes the most sense to do going forward.  If we do tweak the NBD
standard, at best it should only be an advisory on best server
practice (with the caveat that clients have to be prepared for servers
that return other odd values, like 0, even when a size is not known).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org


Reply to: