Re: [Qemu-devel] [PATCH v2 01/11] block: replace in_use with refcnt_soft and refcnt_hard

[Paolo Bonzini]

Il 17/07/2013 11:42, Fam Zheng ha scritto:
> Introduce refcnt_soft (soft reference) and refcnt_hard (hard reference)
> to BlockDriverState, since in_use mechanism cannot provide proper
> management of lifecycle when a BDS is referenced in multiple places
> (e.g. pointed to by another bs's backing_hd while also used as a block
> job device, in the use case of image fleecing).
> 
> The original in_use case is considered a "hard reference" in this patch,
> where the bs is busy and should not be used in other tasks that require
> a hard reference. (However the interface doesn't force this, caller
> still need to call bdrv_in_use() to check by itself.).
> 
> A soft reference is implemented but not used yet. It will be used in
> following patches to manage the lifecycle together with hard reference.
> 
> If bdrv_ref() is called on a BDS, it must be released by exactly the
> same numbers of bdrv_unref() with the same "soft/hard" type, and never
> call bdrv_delete() directly. If the BDS is only used locally (unnamed),
> bdrv_ref/bdrv_unref can be skipped and just use bdrv_delete().

Pardon the stupid question: why do we need a "soft" reference?  We have
these behaviors:

- a sync:'none' backup job doesn't stop until cancelled

- cancelling or completing the job closes the target device, which in
turn stops the NBD server and removes the need to access the source
device via backing_hd

- ejecting the source device cancels the job, which in turn also removes
the need to access the source device via backing_hd

blockdev-backup can sipmly add a reference to the DriveInfo of the
target that it receives.  backup_start has to choose between using
drive_put_ref and bdrv_delete on the target, and can do so using
drive_get_by_blockdev.

backup_start can also mark the target in use, so that drive_del is
prevented while backup_start is running.  After the target device is
closed (and not in_use anymore), all you need to do is invoke drive_del
to delete the BDS.

I don't dislike the series, but I wonder if all this machinery is
actually needed in 1.6.

Paolo