Re: [Qemu-devel] [PATCH 4/5] pr-manager: add query-pr-managers QMP command

[Michal Privoznik]

On 06/26/2018 05:40 PM, Paolo Bonzini wrote:
> This command lets you query the connection status of each pr-manager-helper
> object.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  include/scsi/pr-manager.h |  2 ++
>  qapi/block.json           | 27 +++++++++++++++++++++++
>  scsi/pr-manager-helper.c  | 13 +++++++++++
>  scsi/pr-manager.c         | 45 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 87 insertions(+)
> 
> diff --git a/include/scsi/pr-manager.h b/include/scsi/pr-manager.h
> index 5d2f13a5e4..adadca7954 100644
> --- a/include/scsi/pr-manager.h
> +++ b/include/scsi/pr-manager.h
> @@ -33,8 +33,10 @@ typedef struct PRManagerClass {
>  
>      /* <public> */
>      int (*run)(PRManager *pr_mgr, int fd, struct sg_io_hdr *hdr);
> +    bool (*is_connected)(PRManager *pr_mgr);
>  } PRManagerClass;
>  
> +bool pr_manager_is_connected(PRManager *pr_mgr);
>  BlockAIOCB *pr_manager_execute(PRManager *pr_mgr,
>                                 AioContext *ctx, int fd,
>                                 struct sg_io_hdr *hdr,
> diff --git a/qapi/block.json b/qapi/block.json
> index c694524002..dc3323c954 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -77,6 +77,33 @@
>  { 'struct': 'BlockdevSnapshotInternal',
>    'data': { 'device': 'str', 'name': 'str' } }
>  
> +##
> +# @PRManagerInfo:
> +#
> +# Information about a persistent reservation manager
> +#
> +# @id: the identifier of the persistent reservation manager
> +#
> +# @is-connected: whether the persistent reservation manager is connected to
> +#                the underlying storage or helper
> +#
> +# Since: 3.0
> +##
> +{ 'struct': 'PRManagerInfo',
> +  'data': {'id': 'str', 'is-connected': 'bool'} }
> +
> +##
> +# @query-pr-managers:
> +#
> +# Returns a list of information about each persistent reservation manager.
> +#
> +# Returns: a list of @PRManagerInfo for each persistent reservation manager
> +#
> +# Since: 3.0
> +##
> +{ 'command': 'query-pr-managers', 'returns': ['PRManagerInfo'] }
> +
> +
>  ##
>  # @blockdev-snapshot-internal-sync:
>  #
> diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
> index 0c0fe389b7..b11481be9e 100644
> --- a/scsi/pr-manager-helper.c
> +++ b/scsi/pr-manager-helper.c
> @@ -235,6 +235,18 @@ out:
>      return ret;
>  }
>  
> +static bool pr_manager_helper_is_connected(PRManager *p)
> +{
> +    PRManagerHelper *pr_mgr = PR_MANAGER_HELPER(p);
> +    bool result;
> +
> +    qemu_mutex_lock(&pr_mgr->lock);
> +    result = (pr_mgr->ioc != NULL);

I worry it is not that easy. pr_mgr->ioc is unset only when there's
PR_IN/PR_OUT command coming from the guest (in pr_manager_helper_run ->
pr_manager_helper_write). In fact, after 5/5 that is also the time when
the event is delivered. But that might be too late for mgmt app to
restart the helper process (although pr_manager_helper_run() tries to
reconnect for 5 seconds before giving up).

The patch is good code-wise though.

Michal