From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:39060)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SEzk2-0001xa-6o
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 05:09:34 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SEzjz-0002oD-7E
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 05:09:29 -0400
Received: from mx1.redhat.com ([209.132.183.28]:47161)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SEzjy-0002nw-Um
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 05:09:27 -0400
Message-ID: <4F7ABF13.4060305@redhat.com>
Date: Tue, 03 Apr 2012 11:12:51 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <20120403072349.GC27304@stefanha-thinkpad.localdomain>
	<1333442100-29240-1-git-send-email-namei.unix@gmail.com>
In-Reply-To: <1333442100-29240-1-git-send-email-namei.unix@gmail.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI
	operation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Liu Yuan <namei.unix@gmail.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>, Michael Tokarev <mjt@tls.msk.ru>, qemu-devel@nongnu.org, MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

Am 03.04.2012 10:35, schrieb Liu Yuan:
> From: Liu Yuan <tailai.ly@taobao.com>
> 
> 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 <kwolf@redhat.com>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> Cc: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Liu Yuan <tailai.ly@taobao.com>
> ---
>  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