On Wed, Apr 24, 2019 at 05:24:39PM -0500, Eric Blake wrote:
Add a helper function get_real_size() to make it easier to add
sanity
checking that mutex calls don't fail.
This patch is fine, ACK.
I'm a bit unsure why truncate.c needs a lock anyway. I guess
it's
because we are storing 'size' and 'real_size' as globals rather than
per-connection values in the handle, and we're worrying about parallel
connections where we don't want one thread reading inconsistent values
while another thread is in the middle of truncate_get_size()? At any
rate, as long as we don't have dynamic resize in the NBD protocol,
size can't change within the confines of a single connection. And even
if we DO assume that the underlying plugin reports different sizes for
different connections, our use of a global does not play well (if
client A sees size 1k, then client B sees size 2k, that changes client
A's use of 'real_size' to be something different than client A was
expecting).
So, I think we have to move 'size' and 'real_size' into the
per-connection handle, at which point, can't we just set them once
during truncate_prepare by moving the existing guts of
truncate_get_size there, and then letting truncate_get_size just
return h->size while all other truncate_* functions just access
h->real_size without a lock?
Even storing fields in the handle is not necessarily safe when using
the parallel thread model, because you can have multiple requests
outstanding on the same handle. (However I believe it is safe given
the current implementation of get_size / lack of resize support in
nbdkit.)
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/