On Thu, Sep 23, 2021 at 03:33:45PM -0500, Eric Blake wrote: > > +++ b/block/nbd.c > > @@ -1407,15 +1407,17 @@ static int nbd_client_co_pwritev(BlockDriverState > > *bs, int64_t offset, > > } > > > > static int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t > > offset, > > - int bytes, BdrvRequestFlags flags) > > + int64_t bytes, BdrvRequestFlags > > flags) > > { > > BDRVNBDState *s = (BDRVNBDState *)bs->opaque; > > NBDRequest request = { > > .type = NBD_CMD_WRITE_ZEROES, > > .from = offset, > > - .len = bytes, > > + .len = bytes, /* .len is uint32_t actually */ > > }; > > > > + assert(bytes < UINT32_MAX); /* relay on max_pwrite_zeroes */ > > And again. Here, you happen to get by with < because we clamped > bl.max_pwrite_zeroes at BDRV_REQUEST_MAX_BYTES, which is INT_MAX > rounded down. But I had to check; whereas using <= would be less > worrisome, even if we never get a request that large.
Whoops, I was reading a local patch of mine. Upstream has merely: uint32_t max = MIN_NON_ZERO(NBD_MAX_BUFFER_SIZE, s->info.max_block); bs->bl.max_pdiscard = QEMU_ALIGN_DOWN(INT_MAX, min); bs->bl.max_pwrite_zeroes = max; which is an even smaller limit than BDRV_REQUEST_MAX_BYTES (and obviously one we're trying to raise). But the point remains that using <= rather than < will make it easier to review the code where we raise the limits (either up to the 4G-1 limit of the current protocol, or with protocol extensions to finally get to 64-bit requests). > > If you agree with my analysis, I can make that change while preparing > my pull request. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org