[Qemu-devel] [PATCH 0/4] IDE: Do not flush empty drives

John Snow posted 4 patches 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170808175711.12203-1-jsnow@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
block.c                    |  2 +-
block/block-backend.c      | 40 +++++++++++++++++++++++++-----
hw/ide/core.c              | 11 +++++---
tests/Makefile.include     |  2 ++
tests/ide-test.c           | 19 ++++++++++++++
tests/test-block-backend.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 125 insertions(+), 11 deletions(-)
create mode 100644 tests/test-block-backend.c
[Qemu-devel] [PATCH 0/4] IDE: Do not flush empty drives
Posted by John Snow 6 years, 8 months ago
Patches one and two here are a 2.10 bandaid that avoids a crash.
Patches three and four are a more comprehensive fix as written by
Kevin in another discussion and are being posted here for the sake
of a discussion.

Patch three as written causes hangs in iotests 20, 39, 97, 98, 129,
153, 176, and 185. 124 actually segfaults.

For the purposes of 2.10, we'll likely just want patches 1 and 2
for now.

The problem in a nutshell: incrementing the in-flight counter of the
BDS from the BB layer assumes that every BB always has a BDS. That's
not true; and some devices like IDE have not in the past checked to
see if a given blk_ operation WOULD fail.

This culminates in a new regression where issuing a cache flush to a
CDROM (which is, for some reason, specification valid) will crash QEMU
due to a null dereference when attempting to atomically increment that
backend's in-flight counter.

John Snow (1):
  IDE: Do not flush empty CDROM drives

Kevin Wolf (3):
  IDE: test flush on empty CDROM
  block-backend: shift in-flight counter to BB from BDS
  block-backend: test flush op on empty backend

 block.c                    |  2 +-
 block/block-backend.c      | 40 +++++++++++++++++++++++++-----
 hw/ide/core.c              | 11 +++++---
 tests/Makefile.include     |  2 ++
 tests/ide-test.c           | 19 ++++++++++++++
 tests/test-block-backend.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 125 insertions(+), 11 deletions(-)
 create mode 100644 tests/test-block-backend.c

-- 
2.9.4


Re: [Qemu-devel] [Qemu-block] [PATCH 0/4] IDE: Do not flush empty drives
Posted by Stefan Hajnoczi 6 years, 8 months ago
On Tue, Aug 08, 2017 at 01:57:07PM -0400, John Snow wrote:
> Patches one and two here are a 2.10 bandaid that avoids a crash.
> Patches three and four are a more comprehensive fix as written by
> Kevin in another discussion and are being posted here for the sake
> of a discussion.
> 
> Patch three as written causes hangs in iotests 20, 39, 97, 98, 129,
> 153, 176, and 185. 124 actually segfaults.
> 
> For the purposes of 2.10, we'll likely just want patches 1 and 2
> for now.
> 
> The problem in a nutshell: incrementing the in-flight counter of the
> BDS from the BB layer assumes that every BB always has a BDS. That's
> not true; and some devices like IDE have not in the past checked to
> see if a given blk_ operation WOULD fail.
> 
> This culminates in a new regression where issuing a cache flush to a
> CDROM (which is, for some reason, specification valid) will crash QEMU
> due to a null dereference when attempting to atomically increment that
> backend's in-flight counter.
> 
> John Snow (1):
>   IDE: Do not flush empty CDROM drives
> 
> Kevin Wolf (3):
>   IDE: test flush on empty CDROM
>   block-backend: shift in-flight counter to BB from BDS
>   block-backend: test flush op on empty backend
> 
>  block.c                    |  2 +-
>  block/block-backend.c      | 40 +++++++++++++++++++++++++-----
>  hw/ide/core.c              | 11 +++++---
>  tests/Makefile.include     |  2 ++
>  tests/ide-test.c           | 19 ++++++++++++++
>  tests/test-block-backend.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+), 11 deletions(-)
>  create mode 100644 tests/test-block-backend.c

John will be offline until Monday.  I'm sending a new patch series for
2.10 with updated versions of Patch 1 & 2.

Stefan