From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXVXF-0005TB-10 for qemu-devel@nongnu.org; Mon, 16 Mar 2015 09:58:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXVXE-0006Zi-1M for qemu-devel@nongnu.org; Mon, 16 Mar 2015 09:58:24 -0400 Message-ID: <5506E173.1000808@redhat.com> Date: Mon, 16 Mar 2015 09:58:11 -0400 From: Max Reitz MIME-Version: 1.0 References: <1424887718-10800-1-git-send-email-mreitz@redhat.com> <1424887718-10800-21-git-send-email-mreitz@redhat.com> <550027AB.4000107@redhat.com> In-Reply-To: <550027AB.4000107@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 20/25] block/nbd: Comment on discard/flush silently failing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 2015-03-11 at 07:31, Paolo Bonzini wrote: > > On 25/02/2015 19:08, Max Reitz wrote: >> If some operation cannot be performed by a block driver, it is normally >> supposed to return an error. In these cases, however, it is fine to >> pretend the operations were carried out successfully because if the NBD >> block driver would not implement discard or flush in the first place, >> this is exactly what the block layer would do. >> >> Because this may not be obvious, add a comment for it. >> >> Signed-off-by: Max Reitz >> --- >> block/nbd-client.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/block/nbd-client.c b/block/nbd-client.c >> index be6803d..ab13607 100644 >> --- a/block/nbd-client.c >> +++ b/block/nbd-client.c >> @@ -315,6 +315,7 @@ int nbd_client_co_flush(BlockDriverState *bs) >> ssize_t ret; >> >> if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) { >> + /* This mirrors the behavior of bdrv_co_flush() in block.c */ >> return 0; >> } >> >> @@ -350,6 +351,7 @@ int nbd_client_co_discard(BlockDriverState *bs, int64_t sector_num, >> ssize_t ret; >> >> if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) { >> + /* This mirrors the behavior of bdrv_co_discard() in block.c */ >> return 0; > Should this return -EOPNOTSUPP instead? That's what this patch is for. I asked myself the same thing, and it turns out, the functions deliberately return 0 because bdrv_co_flush() and bdrv_co_discard() do the same if the block driver does not support these functions at all, so that's why I'm adding these comments. Max > Paolo > >> } >> request.from = sector_num * 512; >>