qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 574b83: block/vdi: When writing new bmap entr


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 574b83: block/vdi: When writing new bmap entry fails, don'...
Date: Thu, 01 Apr 2021 05:35:51 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 574b8304cfcc314adb615bb1fd4b159a59ab0441
      
https://github.com/qemu/qemu/commit/574b8304cfcc314adb615bb1fd4b159a59ab0441
  Author: David Edmondson <david.edmondson@oracle.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M block/vdi.c

  Log Message:
  -----------
  block/vdi: When writing new bmap entry fails, don't leak the buffer

If a new bitmap entry is allocated, requiring the entire block to be
written, avoiding leaking the buffer allocated for the block should
the write fail.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Max Reitz <mreitz@redhat.com>
Message-id: 20210325112941.365238-2-pbonzini@redhat.com
Message-Id: <20210309144015.557477-2-david.edmondson@oracle.com>
Acked-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: 07ee2ab4fd0147edb64ba88e55407dd9d6656175
      
https://github.com/qemu/qemu/commit/07ee2ab4fd0147edb64ba88e55407dd9d6656175
  Author: David Edmondson <david.edmondson@oracle.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M block/vdi.c

  Log Message:
  -----------
  block/vdi: Don't assume that blocks are larger than VdiHeader

Given that the block size is read from the header of the VDI file, a
wide variety of sizes might be seen. Rather than re-using a block
sized memory region when writing the VDI header, allocate an
appropriately sized buffer.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Max Reitz <mreitz@redhat.com>
Message-id: 20210325112941.365238-3-pbonzini@redhat.com
Message-Id: <20210309144015.557477-3-david.edmondson@oracle.com>
Acked-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: 2f6ef0393b54383c204d4d6aa5b8eec2bcad566f
      
https://github.com/qemu/qemu/commit/2f6ef0393b54383c204d4d6aa5b8eec2bcad566f
  Author: David Edmondson <david.edmondson@oracle.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M util/qemu-coroutine-lock.c

  Log Message:
  -----------
  coroutine-lock: Store the coroutine in the CoWaitRecord only once

When taking the slow path for mutex acquisition, set the coroutine
value in the CoWaitRecord in push_waiter(), rather than both there and
in the caller.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210325112941.365238-4-pbonzini@redhat.com
Message-Id: <20210309144015.557477-4-david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: 050de36b13f7a841b7805391bca44f36370e86e4
      
https://github.com/qemu/qemu/commit/050de36b13f7a841b7805391bca44f36370e86e4
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M include/qemu/coroutine.h
    M util/qemu-coroutine-lock.c

  Log Message:
  -----------
  coroutine-lock: Reimplement CoRwlock to fix downgrade bug

An invariant of the current rwlock is that if multiple coroutines hold a
reader lock, all must be runnable. The unlock implementation relies on
this, choosing to wake a single coroutine when the final read lock
holder exits the critical section, assuming that it will wake a
coroutine attempting to acquire a write lock.

The downgrade implementation violates this assumption by creating a
read lock owning coroutine that is exclusively runnable - any other
coroutines that are waiting to acquire a read lock are *not* made
runnable when the write lock holder converts its ownership to read
only.

More in general, the old implementation had lots of other fairness bugs.
The root cause of the bugs was that CoQueue would wake up readers even
if there were pending writers, and would wake up writers even if there
were readers.  In that case, the coroutine would go back to sleep *at
the end* of the CoQueue, losing its place at the head of the line.

To fix this, keep the queue of waiters explicitly in the CoRwlock
instead of using CoQueue, and store for each whether it is a
potential reader or a writer.  This way, downgrade can look at the
first queued coroutines and wake it only if it is a reader, causing
all other readers in line to be released in turn.

Reported-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210325112941.365238-5-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: 25bc2daed0482732a2dd258dde4386f505582fa9
      
https://github.com/qemu/qemu/commit/25bc2daed0482732a2dd258dde4386f505582fa9
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M tests/unit/test-coroutine.c

  Log Message:
  -----------
  test-coroutine: Add rwlock upgrade test

Test that rwlock upgrade is fair, and that readers go back to sleep if
a writer is in line.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210325112941.365238-6-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: b6489ac06695e257ea0a9841364577e247fdee30
      
https://github.com/qemu/qemu/commit/b6489ac06695e257ea0a9841364577e247fdee30
  Author: David Edmondson <david.edmondson@oracle.com>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M tests/unit/test-coroutine.c

  Log Message:
  -----------
  test-coroutine: Add rwlock downgrade test

Test that downgrading an rwlock does not result in a failure to
schedule coroutines queued on the rwlock.

The diagram associated with test_co_rwlock_downgrade() describes the
intended behaviour, but what was observed previously corresponds to:

| c1     | c2         | c3         | c4       |
|--------+------------+------------+----------|
| rdlock |            |            |          |
| yield  |            |            |          |
|        | wrlock     |            |          |
|        | <queued>   |            |          |
|        |            | rdlock     |          |
|        |            | <queued>   |          |
|        |            |            | wrlock   |
|        |            |            | <queued> |
| unlock |            |            |          |
| yield  |            |            |          |
|        | <dequeued> |            |          |
|        | downgrade  |            |          |
|        | ...        |            |          |
|        | unlock     |            |          |
|        |            | <dequeued> |          |
|        |            | <queued>   |          |

This results in a failure...

ERROR:../tests/test-coroutine.c:369:test_co_rwlock_downgrade: assertion failed: 
(c3_done)
Bail out! ERROR:../tests/test-coroutine.c:369:test_co_rwlock_downgrade: 
assertion failed: (c3_done)

...as a result of the c3 coroutine failing to run to completion.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210325112941.365238-7-pbonzini@redhat.com
Message-Id: <20210309144015.557477-5-david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


  Commit: 1bd16067b652cce41a9214d0c62c73d5b45ab4b1
      
https://github.com/qemu/qemu/commit/1bd16067b652cce41a9214d0c62c73d5b45ab4b1
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-03-31 (Wed, 31 Mar 2021)

  Changed paths:
    M block/vdi.c
    M include/qemu/coroutine.h
    M tests/unit/test-coroutine.c
    M util/qemu-coroutine-lock.c

  Log Message:
  -----------
  Merge remote-tracking branch 
'remotes/stefanha-gitlab/tags/block-pull-request' into staging

Pull request

A fix for VDI image files and more generally for CoRwlock.

# gpg: Signature made Wed 31 Mar 2021 10:50:39 BST
# gpg:                using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full]
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>" [full]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha-gitlab/tags/block-pull-request:
  test-coroutine: Add rwlock downgrade test
  test-coroutine: Add rwlock upgrade test
  coroutine-lock: Reimplement CoRwlock to fix downgrade bug
  coroutine-lock: Store the coroutine in the CoWaitRecord only once
  block/vdi: Don't assume that blocks are larger than VdiHeader
  block/vdi: When writing new bmap entry fails, don't leak the buffer

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/6ee55e1d10c2...1bd16067b652



reply via email to

[Prev in Thread] Current Thread [Next in Thread]