From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:51712)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SoFKj-00061X-5c
	for qemu-devel@nongnu.org; Mon, 09 Jul 2012 10:53:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SoFKd-0004nz-Qm
	for qemu-devel@nongnu.org; Mon, 09 Jul 2012 10:53:04 -0400
Received: from mx1.redhat.com ([209.132.183.28]:14755)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1SoFKd-0004nn-Id
	for qemu-devel@nongnu.org; Mon, 09 Jul 2012 10:52:59 -0400
Message-ID: <4FFAF040.3020001@redhat.com>
Date: Mon, 09 Jul 2012 16:52:48 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <20120615204648.9853.1225.sendpatchset@skannery.in.ibm.com>
	<20120615204730.9853.61858.sendpatchset@skannery.in.ibm.com>
In-Reply-To: <20120615204730.9853.61858.sendpatchset@skannery.in.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for
 dynamic cache change
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: Supriya Kannery <supriyak@linux.vnet.ibm.com>
Cc: Shrinidhi Joshi <spjoshi31@gmail.com>, Stefan Hajnoczi <stefanha@gmail.com>, Jeff Cody <jcody@redhat.com>, qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>, Christoph Hellwig <hch@lst.de>

Am 15.06.2012 22:47, schrieb Supriya Kannery:
> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    2 ++
>  blockdev.c      |   26 ++++++++++++++++++++++++++
>  blockdev.h      |    2 ++
>  hmp-commands.hx |   14 ++++++++++++++
>  qmp-commands.hx |   27 +++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+)
> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +    BlockDriver *drv = bs->drv;
> +    int ret = 0, open_flags;
> +
> +    /* Quiesce IO for the given block device */
> +    qemu_aio_flush();

bdrv_drain_all() should be used instead.

> +    ret = bdrv_flush(bs);
> +    if (ret != 0) {
> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +        return ret;
> +    }
> +    open_flags = bs->open_flags;
> +    bdrv_close(bs);
> +
> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +    if (ret < 0) {
> +        /* Reopen failed. Try to open with original flags */
> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +        if (ret < 0) {
> +            /* Reopen failed with orig and modified flags */
> +            abort();

Like Eric said, committing a broken version and then fixing it later in
the series isn't really nice. At least bs->drv = NULL; instead of
abort() is required. Starting with the real thing is probably even better.

Kevin