[Qemu-devel] [Bug 1353545] [NEW] QED does not deliver flush on synchronized write

[Qemu-devel] [Bug 1353545] [NEW] QED does not deliver flush on synchronized write

[Nybble]

Public bug reported:

At least one flush method should be provided by qed driver
(common/co/aio).

I was doing "sync write" benchmarking on varies qemu supported formats
(on ssd). I was surprised that QED shows significant high performance
over several other formats (qcow2/vmdk/vdi..). In some test cases QED
even runs a little bit faster than raw format!

After careful reading of the code I found QED skipped the necessary
flush by providing nothing on flush api. unlike read/write which require
at least one type of implementation, flush API is not mandatory because
your format could guarantee that synchronization is done in write path
or others. However that's not the case in QED because data is buffered
in the "bs->file" and QED needs to propagate the flush command to it to
make data safe.

However I'm not a skilled qemu developer. Above is what I got from my
thinking and study. Please correct me if in any way I misunderstood the
design. I really appreciate!

The patch is short:
https://github.com/wuxb45/qemu/commit/fe46e640f53a006907091e910fb4868a878ea0a8

qemu release version: 2.1.0. But this should have been there for a long time.
The patch is based on this recent commit:
69f87f713069f1f70f86cb65883f7d43e3aa21de

Thanks.

** Affects: qemu
     Importance: Undecided
         Status: New


** Tags: block disk qed

** Description changed:

  At least one flush method should be provided by qed driver
  (common/co/aio).
  
- 
- I was doing "sync write" benchmarking on varies qemu supported formats (on ssd). I was surprised that QED shows significant high performance over several other formats (qcow2/vmdk/vdi..). In some test cases QED even runs a little bit faster than raw format!
+ I was doing "sync write" benchmarking on varies qemu supported formats
+ (on ssd). I was surprised that QED shows significant high performance
+ over several other formats (qcow2/vmdk/vdi..). In some test cases QED
+ even runs a little bit faster than raw format!
  
  After careful reading of the code I found QED skipped the necessary
  flush by providing nothing on flush api. unlike read/write which require
  at least one type of implementation, flush API is not mandatory because
  your format could guarantee that synchronization is done in write path
  or others. However that's not the case in QED because data is buffered
- in the "bs->file" and it needs to populate the flush command to make
- data safe.
+ in the "bs->file" and QED needs to propagate the flush command to it to
+ make data safe.
  
  However I'm not a skilled qemu developer. Above is what I got from my
- thinking and study. Please correct me if I misunderstood the design. I
- really appreciate!
+ thinking and study. Please correct me if in any way I misunderstood the
+ design. I really appreciate!
  
- The patch is short: 
+ The patch is short:
  https://github.com/wuxb45/qemu/commit/fe46e640f53a006907091e910fb4868a878ea0a8
  
- qemu release version: 2.1.0. But this should have been their for a long time.
+ qemu release version: 2.1.0. But this should have been there for a long time.
  The patch is based on this recent commit:
  69f87f713069f1f70f86cb65883f7d43e3aa21de
  
- 
  Thanks.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1353545

Title:
  QED does not deliver flush on synchronized write

Status in QEMU:
  New

Bug description:
  At least one flush method should be provided by qed driver
  (common/co/aio).

  I was doing "sync write" benchmarking on varies qemu supported formats
  (on ssd). I was surprised that QED shows significant high performance
  over several other formats (qcow2/vmdk/vdi..). In some test cases QED
  even runs a little bit faster than raw format!

  After careful reading of the code I found QED skipped the necessary
  flush by providing nothing on flush api. unlike read/write which
  require at least one type of implementation, flush API is not
  mandatory because your format could guarantee that synchronization is
  done in write path or others. However that's not the case in QED
  because data is buffered in the "bs->file" and QED needs to propagate
  the flush command to it to make data safe.

  However I'm not a skilled qemu developer. Above is what I got from my
  thinking and study. Please correct me if in any way I misunderstood
  the design. I really appreciate!

  The patch is short:
  https://github.com/wuxb45/qemu/commit/fe46e640f53a006907091e910fb4868a878ea0a8

  qemu release version: 2.1.0. But this should have been there for a long time.
  The patch is based on this recent commit:
  69f87f713069f1f70f86cb65883f7d43e3aa21de

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1353545/+subscriptions

[Qemu-devel] [Bug 1353545] Re: QED does not deliver flush on synchronized write

[Nybble]

** Changed in: qemu
       Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1353545

Title:
  QED does not deliver flush on synchronized write

Status in QEMU:
  Invalid

Bug description:
  At least one flush method should be provided by qed driver
  (common/co/aio).

  I was doing "sync write" benchmarking on varies qemu supported formats
  (on ssd). I was surprised that QED shows significant high performance
  over several other formats (qcow2/vmdk/vdi..). In some test cases QED
  even runs a little bit faster than raw format!

  After careful reading of the code I found QED skipped the necessary
  flush by providing nothing on flush api. unlike read/write which
  require at least one type of implementation, flush API is not
  mandatory because your format could guarantee that synchronization is
  done in write path or others. However that's not the case in QED
  because data is buffered in the "bs->file" and QED needs to propagate
  the flush command to it to make data safe.

  However I'm not a skilled qemu developer. Above is what I got from my
  thinking and study. Please correct me if in any way I misunderstood
  the design. I really appreciate!

  The patch is short:
  https://github.com/wuxb45/qemu/commit/fe46e640f53a006907091e910fb4868a878ea0a8

  qemu release version: 2.1.0. But this should have been there for a long time.
  The patch is based on this recent commit:
  69f87f713069f1f70f86cb65883f7d43e3aa21de

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1353545/+subscriptions

Re: [Qemu-devel] [Bug 1353545] Re: QED does not deliver flush on synchronized write

[Stefan Hajnoczi]

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

On Wed, Aug 06, 2014 at 03:32:29PM -0000, Nybble wrote:
> ** Changed in: qemu
>        Status: New => Invalid

I guess you saw that bdrv_co_flush() calls bdrv_co_flush(bs->file).
This means that even block drivers that do not provide
->bdrv_co_flush_to_disk() will have fdatasync() called on their image
file.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [Bug 1353545] Re: QED does not deliver flush on synchronized write

[Nybble]

I'm really sorry that I didn't see that call when I submit this...
I was writing a experimental block image driver and found it runs too slow. It turned out that it calls bdrv_flush itself and the flush on ->file is than called again from block.c

thanks for your confirmation!

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1353545

Title:
  QED does not deliver flush on synchronized write

Status in QEMU:
  Invalid

Bug description:
  At least one flush method should be provided by qed driver
  (common/co/aio).

  I was doing "sync write" benchmarking on varies qemu supported formats
  (on ssd). I was surprised that QED shows significant high performance
  over several other formats (qcow2/vmdk/vdi..). In some test cases QED
  even runs a little bit faster than raw format!

  After careful reading of the code I found QED skipped the necessary
  flush by providing nothing on flush api. unlike read/write which
  require at least one type of implementation, flush API is not
  mandatory because your format could guarantee that synchronization is
  done in write path or others. However that's not the case in QED
  because data is buffered in the "bs->file" and QED needs to propagate
  the flush command to it to make data safe.

  However I'm not a skilled qemu developer. Above is what I got from my
  thinking and study. Please correct me if in any way I misunderstood
  the design. I really appreciate!

  The patch is short:
  https://github.com/wuxb45/qemu/commit/fe46e640f53a006907091e910fb4868a878ea0a8

  qemu release version: 2.1.0. But this should have been there for a long time.
  The patch is based on this recent commit:
  69f87f713069f1f70f86cb65883f7d43e3aa21de

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1353545/+subscriptions