qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kwolf@redhat.com, rusty@au1.ibm.com, qemu-devel@nongnu.org,
	stefanha@redhat.com, qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current
Date: Thu, 19 Sep 2013 11:42:48 -0600	[thread overview]
Message-ID: <523B3798.7040105@redhat.com> (raw)
In-Reply-To: <1379609334-20811-3-git-send-email-pbonzini@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

On 09/19/2013 10:48 AM, Paolo Bonzini wrote:
> The following sequence happens:

> Hence, whenever the guest is reset, the cache mode of the disk should
> be reset to whatever was specified in the "-drive" option.  With this
> change, the Linux virtio-blk driver finds that writeback caching is
> enabled, and tells the block layer to send cache flush commands
> appropriately.
> 
> Reported-by: Rusty Russell <rusty@au1.ibm.com
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/block/virtio-blk.c          | 8 ++++++--
>  include/hw/virtio/virtio-blk.h | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

>  
>      features = vdev->guest_features;
> -    bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
> +    if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) {
> +        bdrv_set_enable_write_cache(s->bs,
> +                                    !!(features & (1 << VIRTIO_BLK_F_WCE)));
> +    }
>  }
>  
>  static void virtio_blk_save(QEMUFile *f, void *opaque)
> @@ -674,6 +678,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
>      }
>  
>      blkconf_serial(&blk->conf, &blk->serial);
> +    s->original_wce = bdrv_enable_write_cache(blk->conf.bs);

At first, I was worried that this does 'bool = int', and whether that
was correct in all cases.  But looking further, bdrv_enable_write_cache
merely returns bs->enable_write_cache (also an int), but that all
assignments to bs->enable_write_cache are careful to only assign 0 or 1.
 A followup patch that changes the types to bool might be in order, but
doesn't invalidate this patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-09-19 17:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-19 16:48 [Qemu-devel] [PATCH 0/2] Serious bugs in the handling of writeback caching Paolo Bonzini
2013-09-19 16:48 ` [Qemu-devel] [PATCH 1/2] blockdev: do not default cache.no-flush to true Paolo Bonzini
2013-09-19 17:33   ` Eric Blake
2013-09-19 16:48 ` [Qemu-devel] [PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current Paolo Bonzini
2013-09-19 17:42   ` Eric Blake [this message]
2013-09-20  9:54   ` Kevin Wolf
2013-09-20 10:28     ` Paolo Bonzini
2013-09-20 15:31   ` [Qemu-devel] [PATCH v2 " Paolo Bonzini
2013-09-20 17:28 ` [Qemu-devel] [PATCH 0/2] Serious bugs in the handling of writeback caching Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=523B3798.7040105@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=rusty@au1.ibm.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).