From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXWGC-0002Aa-6B for qemu-devel@nongnu.org; Mon, 16 Mar 2015 10:44:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXWGB-00069t-4Z for qemu-devel@nongnu.org; Mon, 16 Mar 2015 10:44:52 -0400 Message-ID: <5506EC57.1090305@redhat.com> Date: Mon, 16 Mar 2015 15:44:39 +0100 From: Paolo Bonzini 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> <5506E173.1000808@redhat.com> In-Reply-To: <5506E173.1000808@redhat.com> Content-Type: text/plain; charset=windows-1252 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: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 16/03/2015 14:58, Max Reitz wrote: >>> >>> 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. Right, but a better model than block.c should be for example block/raw-posix.c, which returns ENOTSUP (I checked now...). Paolo