Re: [PATCH 19/29] block/export: Move strong user reference to block_exports

[Max Reitz]

On 10.09.20 15:33, Max Reitz wrote:
> On 07.09.20 20:20, Kevin Wolf wrote:
>> The reference owned by the user/monitor that is created when adding the
>> export and dropped when removing it was tied to the 'exports' list in
>> nbd/server.c. Every block export will have a user reference, so move it
>> to the block export level and tie it to the 'block_exports' list in
>> block/export/export.c instead. This is necessary for introducing a QMP
>> command for removing exports.
>>
>> Note that exports are present in block_exports even after the user has
>> requested shutdown. This is different from NBD's exports where exports
>> are immediately removed on a shutdown request, even if they are still in
>> the process of shutting down. In order to avoid that the user still
>> interacts with an export that is shutting down (and possibly removes it
>> a second time), we need to remember if the user actually still owns it.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  include/block/export.h | 8 ++++++++
>>  block/export/export.c  | 6 ++++++
>>  blockdev-nbd.c         | 5 -----
>>  nbd/server.c           | 2 --
>>  4 files changed, 14 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
>> diff --git a/include/block/export.h b/include/block/export.h
>> index cdc6e161ea..4833947e89 100644
>> --- a/include/block/export.h
>> +++ b/include/block/export.h
>> @@ -60,6 +60,14 @@ struct BlockExport {
>>       */
>>      int refcount;
>>  
>> +    /*
>> +     * True if one of the references in refcount belongs to the user. After 
>> the
>> +     * user has dropped their reference, they may not e.g. remove the same
>> +     * export a second time (which would decrease the refcount without 
>> having
>> +     * it incremented first).
> 
> Not sure though whether this (or the request_shutdown documentation)
> should mention that request_shutdown drops the user reference.

Er, no, it shouldn’t, because it doesn’t.  Only
blk_exp_request_shutdown() does that.  Though since that’s a public
function (used by nbd/server.c), maybe it should be documented there.

Max

signature.asc
Description: OpenPGP digital signature