[libvirt] [PATCH v2 8/8] storage_driver: Release pool object lock for some long running jobs

[John Ferlan]

On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> As advertised in previous commit, there are three APIs that might
> run for quite some time (because they read/write data from/to a
> volume) and these three are: downloadVol, uploadVol, wipeVol.
> Release pool object lock and reacquire it later to allow more
> concurrency.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/storage/storage_backend_iscsi_direct.c | 6 +++++-
>  src/storage/storage_backend_rbd.c          | 8 ++++++--
>  src/storage/storage_driver.c               | 6 ++++++
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 

I think we could/should note in src/storage/storage_backend.h that for
the three functions (Download, Upload, and Wipe) that upon entry the
@obj for the pool is unlocked, but the pool object's "asyncjobs" will be
adjusted to allow concurrency and that the the volume's "in_use"
adjusted to ensure singular usage.

I'd also add a similar comment to virStorageBackendVolWipeLocal,
virStorageBackendDiskVolWipe, virStorageBackendVolUploadLocal, and
virStorageBackendVoldownloadLocal that the @pool is unlocked prior to
the call.

If virStorageBackendRBDVolWipe or virStorageBackenISCSIDirectWipeVol get
a similar information is up to you, I'd probably put it there too just
for completeness for the "next" poor soul that copies from one of the
existing backends.

Whether or not anyone actually reads it is their problem, at least we
noted it!


Reviewed-by: John Ferlan <jferlan at redhat.com>

John

FWIW: I almost had an oh sh*t moment with virObject{Unlock|Lock} before
I came back in off the ledge since we're not talking the pools' rwlock,
but the pool's obj/def lock that isn't an rwlock.