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

[Kevin Wolf]

Am 03.04.2012 10:35, schrieb Liu Yuan:
> From: Liu Yuan <address@hidden>
> 
> Flush operation is supposed to flush the write-back cache of
> sheepdog cluster.
> 
> By issuing flush operation, we can assure the Guest of data
> reaching the sheepdog cluster storage.
> 
> Cc: Kevin Wolf <address@hidden>
> Cc: Michael Tokarev <address@hidden>
> Cc: MORITA Kazutaka <address@hidden>
> Cc: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Liu Yuan <address@hidden>
> ---
>  v2 -> v3:
>  address Stefan Hajnoczi comments. Thanks !
>   - use qemu_co_send/recv to send/revc msg
> 
>  v1 -> v2:
>  address Michael Tokarev comments. Thanks !
>   - use per-device flag
>   - use bs->fd instead of 'connect_to_sdog()'

This version looks okay to me now. Waiting for Kazutaka's Acked-by.

> +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
> +                       unsigned int *wlen)
> +{
> +    int ret;
> +
> +    ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
> +    if (ret < sizeof(*hdr)) {
> +        error_report("failed to send a req, %s", strerror(errno));
> +    }
> +
> +    ret = qemu_co_send(sockfd, data, *wlen);
> +    if (ret < *wlen) {
> +        error_report("failed to send a req, %s", strerror(errno));
> +    }
> +
> +    return ret;
> +}

If sending the header fails, why do we still send the data? In fact, if
sending the data succeeds then (which is unlikely, but who knows), we'll
even return success.

This only copies the logic of the existing send_req(), so if it needs to
be fixed, another patch on top that fixes both instances would be okay.

Kevin