[Libguestfs] [nbdkit PATCH 2/4] truncate: Factor out reading real_size under mutex

[Richard W.M. Jones]

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/