Re: [Qemu-devel] [PATCH v2] sheepdog: implement SD_OP_FLUSH_VDI operation

[Stefan Hajnoczi]

On Tue, Apr 03, 2012 at 01:15:40PM +0800, Liu Yuan wrote:
> +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
> +{
> +    BDRVSheepdogState *s = bs->opaque;
> +    SheepdogObjReq hdr = { 0 };
> +    SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr;
> +    SheepdogInode *inode = &s->inode;
> +    int ret;
> +    unsigned int wlen = 0, rlen = 0;
> +
> +    if (!s->cache_enabled)
> +        return 0;
> +
> +    hdr.opcode = SD_OP_FLUSH_VDI;
> +    hdr.oid = vid_to_vdi_oid(inode->vdi_id);
> +
> +    ret = do_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen);
> +    if (ret) {
> +        error_report("failed to send a request to the sheep");
> +        return -1;
> +    }
> +
> +    if (rsp->result != SD_RES_SUCCESS) {
> +        error_report("%s", sd_strerror(rsp->result));
> +        return -1;
> +    }
> +
> +    return 0;
> +}

.bdrv_co_flush_to_disk() is a coroutine_fn so that it can yield while
waiting for socket I/O.  Using do_req() defeats its purpose and will
pause both the guest and QEMU's own monitor/VNC/etc while waiting for
the sheepdog server to communicate with us.

You should use coroutine send/recv functions - they yield if the socket
returns EAGAIN/EWOULDBLOCK.

Also please return negative errno values instead of -1 (which is
-EPERM).

Stefan