Re: [Qemu-block] [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation manager using qemu-pr-helper

[Daniel P. Berrange]

On Tue, Sep 19, 2017 at 12:24:34PM +0200, Paolo Bonzini wrote:
> This adds a concrete subclass of pr-manager that talks to qemu-pr-helper.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> v1->v2: fixed string property double-free
>         fixed/cleaned up error handling
>         handle buffer underrun
> 
>  scsi/Makefile.objs       |   2 +-
>  scsi/pr-manager-helper.c | 302 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 303 insertions(+), 1 deletion(-)
>  create mode 100644 scsi/pr-manager-helper.c


> +static int pr_manager_helper_run(PRManager *p,
> +                                 int fd, struct sg_io_hdr *io_hdr)
> +{
> +    PRManagerHelper *pr_mgr = PR_MANAGER_HELPER(p);
> +
> +    uint32_t len;
> +    PRHelperResponse resp;
> +    int ret;
> +    int expected_dir;
> +    int attempts;
> +    uint8_t cdb[PR_HELPER_CDB_SIZE] = { 0 };
> +
> +    if (!io_hdr->cmd_len || io_hdr->cmd_len > PR_HELPER_CDB_SIZE) {
> +        return -EINVAL;
> +    }
> +
> +    memcpy(cdb, io_hdr->cmdp, io_hdr->cmd_len);
> +    assert(cdb[0] == PERSISTENT_RESERVE_OUT || cdb[0] == 
> PERSISTENT_RESERVE_IN);
> +    expected_dir =
> +        (cdb[0] == PERSISTENT_RESERVE_OUT ? SG_DXFER_TO_DEV : 
> SG_DXFER_FROM_DEV);
> +    if (io_hdr->dxfer_direction != expected_dir) {
> +        return -EINVAL;
> +    }
> +
> +    len = scsi_cdb_xfer(cdb);
> +    if (io_hdr->dxfer_len < len || len > PR_HELPER_DATA_SIZE) {
> +        return -EINVAL;
> +    }
> +
> +    qemu_mutex_lock(&pr_mgr->lock);
> +
> +    /* Try to reconnect while sending the CDB.  */
> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {

I'm curious why you need to loop here. The helper daemon should be running
already, as you're not spawning it on demand IIUC. So it shoudl either
succeed first time, or fail every time.

> +        if (!pr_mgr->ioc) {
> +            ret = pr_manager_helper_initialize(pr_mgr, NULL);
> +            if (ret < 0) {
> +                qemu_mutex_unlock(&pr_mgr->lock);
> +                g_usleep(G_USEC_PER_SEC);
> +                qemu_mutex_lock(&pr_mgr->lock);
> +                continue;
> +            }
> +        }
> +
> +        ret = pr_manager_helper_write(pr_mgr, fd, cdb, ARRAY_SIZE(cdb), 
> NULL);
> +        if (ret >= 0) {
> +            break;
> +        }
> +    }
> +    if (ret < 0) {
> +        goto out;
> +    }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|