qemu-commits
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 1466ef: qapi: rename BlockDirtyBitmapMergeSou

From: Richard Henderson
Subject: [Qemu-commits] [qemu/qemu] 1466ef: qapi: rename BlockDirtyBitmapMergeSource to BlockD...
Date: Tue, 26 Apr 2022 14:44:36 -0700 
  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: 1466ef6cbe26a55e9bd5d4c3d9f58c793e1eb2c4
      
https://github.com/qemu/qemu/commit/1466ef6cbe26a55e9bd5d4c3d9f58c793e1eb2c4
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/monitor/bitmap-qmp-cmds.c
    M include/block/block_int-global-state.h
    M qapi/block-core.json
    M qemu-img.c

  Log Message:
  -----------
  qapi: rename BlockDirtyBitmapMergeSource to BlockDirtyBitmapOrStr

Rename the type to be reused. Old name is "what is it for". To be
natively reused for other needs, let's name it exactly "what is it".

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Message-Id: <20220314213226.362217-2-v.sementsov-og@mail.ru>
[eblake: Adjust S-o-b to Vladimir's new email, with permission]
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e5fb29d5d001dd5f300ddb4ad48e11c3ab2d35ec
      
https://github.com/qemu/qemu/commit/e5fb29d5d001dd5f300ddb4ad48e11c3ab2d35ec
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M blockdev-nbd.c
    M nbd/server.c
    M qapi/block-export.json
    M qemu-nbd.c

  Log Message:
  -----------
  qapi: nbd-export: allow select bitmaps by node/name pair

Hi all! Current logic of relying on search through backing chain is not
safe neither convenient.

Sometimes it leads to necessity of extra bitmap copying. Also, we are
going to add "snapshot-access" driver, to access some snapshot state
through NBD. And this driver is not formally a filter, and of course
it's not a COW format driver. So, searching through backing chain will
not work. Instead of widening the workaround of bitmap searching, let's
extend the interface so that user can select bitmap precisely.

Note, that checking for bitmap active status is not copied to the new
API, I don't see a reason for it, user should understand the risks. And
anyway, bitmap from other node is unrelated to this export being
read-only or read-write.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Message-Id: <20220314213226.362217-3-v.sementsov-og@mail.ru>
[eblake: Adjust S-o-b to Vladimir's new email, with permission]
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: c08c220be70603941b680d5b39f2061222dd74e8
      
https://github.com/qemu/qemu/commit/c08c220be70603941b680d5b39f2061222dd74e8
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M tests/qemu-iotests/223
    M tests/qemu-iotests/223.out

  Log Message:
  -----------
  iotests/223: check new possibility of exporting bitmaps by node/name

Add simple test that new interface introduced in previous commit works.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Message-Id: <20220314213226.362217-4-v.sementsov-og@mail.ru>
[eblake: Adjust S-o-b to Vladimir's new email, with permission]
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 8846b7d1c137ba261b2300b20e94ae360d88a538
      
https://github.com/qemu/qemu/commit/8846b7d1c137ba261b2300b20e94ae360d88a538
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: safeguard against waking up invalid coroutine

The .reply_possible field of s->requests is never set to false.  This is
not a problem as it is only a safeguard to detect protocol errors,
but it's sloppy.  In fact, the field is actually not necessary at all,
because .coroutine is set to NULL in NBD_FOREACH_REPLY_CHUNK after
receiving the last chunk.  Thus, replace .reply_possible with .coroutine
and move the check before deciding the fate of this request.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-2-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 0c43c6fc896ff0894627b9464c3db94a33c8c7ac
      
https://github.com/qemu/qemu/commit/0c43c6fc896ff0894627b9464c3db94a33c8c7ac
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: mark more coroutine_fns

Several coroutine functions in block/nbd.c are not marked as such.  This
patch adds a few more markers; it is not exhaustive, but it focuses
especially on:

- places that wake other coroutines, because aio_co_wake() has very
different semantics inside a coroutine (queuing after yield vs. entering
immediately);

- functions with _co_ in their names, to avoid confusion

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-3-pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 172f5f1a4058c361bfcd19317b0e3151556b3edb
      
https://github.com/qemu/qemu/commit/172f5f1a4058c361bfcd19317b0e3151556b3edb
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: remove peppering of nbd_client_connected

It is unnecessary to check nbd_client_connected() because every time
s->state is moved out of NBD_CLIENT_CONNECTED the socket is shut down
and all coroutines are resumed.

The only case where it was actually needed is when the NBD
server disconnects and there is no reconnect-delay.  In that
case, nbd_receive_replies() does not set s->reply.handle and
nbd_co_do_receive_one_chunk() cannot continue.  For that one case,
check the return value of nbd_receive_replies().

As to the others:

* nbd_receive_replies() can put the current coroutine to sleep if another
reply is ongoing; then it will be woken by nbd_channel_error(), called
by the ongoing reply.  Or it can try itself to read a reply header and
fail, thus calling nbd_channel_error() itself.

* nbd_co_send_request() will write the body of the request and fail

* nbd_reply_chunk_iter_receive() will call nbd_co_receive_one_chunk()
and then nbd_co_do_receive_one_chunk(), which will handle the failure as
above; or it will just detect a previous call to nbd_iter_channel_error()
via iter->ret < 0.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-4-pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 8610b4491f02312f3bf0de10826fad7aa99ddfcf
      
https://github.com/qemu/qemu/commit/8610b4491f02312f3bf0de10826fad7aa99ddfcf
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/coroutines.h
    M block/nbd.c

  Log Message:
  -----------
  nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection

Elevate s->in_flight early so that other incoming requests will wait
on the CoQueue in nbd_co_send_request; restart them after getting back
from nbd_reconnect_attempt.  This could be after the reconnect timer or
nbd_cancel_in_flight have cancelled the attempt, so there is no
need anymore to cancel the requests there.

nbd_co_send_request now handles both stopping and restarting pending
requests after a successful connection, and there is no need to
hold send_mutex in nbd_co_do_establish_connection.  The current setup
is confusing because nbd_co_do_establish_connection is called both with
send_mutex taken and without it.  Before the patch it uses free_sema which
(at least in theory...) is protected by send_mutex, after the patch it
does not anymore.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-5-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: wrap long line]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: ee19d953ecff53908a645d38a1b913fdf15a72f4
      
https://github.com/qemu/qemu/commit/ee19d953ecff53908a645d38a1b913fdf15a72f4
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: use a QemuMutex to synchronize yanking, reconnection and coroutines

The condition for waiting on the s->free_sema queue depends on
both s->in_flight and s->state.  The latter is currently using
atomics, but this is quite dubious and probably wrong.

Because s->state is written in the main thread too, for example by
the yank callback, it cannot be protected by a CoMutex.  Introduce a
separate lock that can be used by nbd_co_send_request(); later on this
lock will also be used for s->state.  There will not be any contention
on the lock unless there is a yank or reconnect, so this is not
performance sensitive.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-6-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 8d45185cb76fa1dd7c3309940a967dc42d8619d4
      
https://github.com/qemu/qemu/commit/8d45185cb76fa1dd7c3309940a967dc42d8619d4
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: code motion and function renaming

Prepare for the next patch, so that the diff is less confusing.

nbd_client_connecting is moved closer to the definition point.

nbd_client_connecting_wait() is kept only for the reconnection
logic; when it is used to check if a request has to be reissued,
use the renamed function nbd_client_will_reconnect().  In the
next patch, the two cases will have different locking requirements.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-7-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: dba5156c0e9c0362b7c6121f9e1c89bb9be1f227
      
https://github.com/qemu/qemu/commit/dba5156c0e9c0362b7c6121f9e1c89bb9be1f227
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: move s->state under requests_lock

Remove the confusing, and most likely wrong, atomics.  The only function
that used to be somewhat in a hot path was nbd_client_connected(),
but it is not anymore after the previous patches.

The same logic is used both to check if a request had to be reissued
and also in nbd_reconnecting_attempt().  The former cases are outside
requests_lock, while nbd_reconnecting_attempt() does have the lock,
therefore the two have been separated in the previous commit.
nbd_client_will_reconnect() can simply take s->requests_lock, while
nbd_reconnecting_attempt() can inline the access now that no
complicated atomics are involved.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-8-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: a80a9a1c7397382fc4c4e6feaa8242b25cadb519
      
https://github.com/qemu/qemu/commit/a80a9a1c7397382fc4c4e6feaa8242b25cadb519
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: take receive_mutex when reading requests[].receiving

requests[].receiving is set by nbd_receive_replies() under the receive_mutex;
Read it under the same mutex as well.  Waking up receivers on errors happens
after each reply finishes processing, in nbd_co_receive_one_chunk().
If there is no currently-active reply, there are two cases:

* either there is no active request at all, in which case no
element of request[] can have .receiving = true

* or nbd_receive_replies() must be running and owns receive_mutex;
in that case it will get back to nbd_co_receive_one_chunk() because
the socket has been shutdown, and all waiting coroutines will wake up
in turn.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-9-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 620c5cb5da57dc97f655e6218e7ca9bc896394a2
      
https://github.com/qemu/qemu/commit/620c5cb5da57dc97f655e6218e7ca9bc896394a2
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: document what is protected by the CoMutexes

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220414175756.671165-10-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: a72d9008092e39c2c37e47a91bae4e170d0f1b33
      
https://github.com/qemu/qemu/commit/a72d9008092e39c2c37e47a91bae4e170d0f1b33
  Author: Richard Henderson <richard.henderson@linaro.org>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M block/coroutines.h
    M block/monitor/bitmap-qmp-cmds.c
    M block/nbd.c
    M blockdev-nbd.c
    M include/block/block_int-global-state.h
    M nbd/server.c
    M qapi/block-core.json
    M qapi/block-export.json
    M qemu-img.c
    M qemu-nbd.c
    M tests/qemu-iotests/223
    M tests/qemu-iotests/223.out

  Log Message:
  -----------
  Merge tag 'pull-nbd-2022-04-26' of https://repo.or.cz/qemu/ericb into staging

nbd patches for 2022-04-26

- Paolo Bonzini: thread-safety improvements to NBD client
- Vladimir Sementsov-Ogievsky: finer-grained selection of bitmaps during
  nbd-export

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmJoUhAACgkQp6FrSiUn
# Q2qnpgf/YCuONdwAndjEo7he5c1BfB/F2sujQJJ00CebUqnz5OFKQ85RwLC8DCGB
# rXnxqC/NF4yyYM+6uYWDpggDd0bJVKbfG7NE/AZsEZrK+n9xMkvGLRwGqMugUii+
# Px4Ba98++0giqGoAI8pU/wQZNh1I6uGabv/DPRTpwzBjbfAcATqV09OzaGiK3SRC
# Zm/55zmXm1zM4XSUtUzN1gILPG09P+51m6NVkANZbzps9e2PtfFy8EsWc5+YhuBM
# 5K7sN+5g8GpRhz6j8RkrhrbNpvg3bGvgRJRMcW7Bo8KVUdvT1Jng6xs8CIRv39AF
# jDJwGe+cq5p5PNuyqOrVSA/ynBZxBw==
# =I1yM
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 26 Apr 2022 01:12:00 PM PDT
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]

* tag 'pull-nbd-2022-04-26' of https://repo.or.cz/qemu/ericb:
  nbd: document what is protected by the CoMutexes
  nbd: take receive_mutex when reading requests[].receiving
  nbd: move s->state under requests_lock
  nbd: code motion and function renaming
  nbd: use a QemuMutex to synchronize yanking, reconnection and coroutines
  nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection
  nbd: remove peppering of nbd_client_connected
  nbd: mark more coroutine_fns
  nbd: safeguard against waking up invalid coroutine
  iotests/223: check new possibility of exporting bitmaps by node/name
  qapi: nbd-export: allow select bitmaps by node/name pair
  qapi: rename BlockDirtyBitmapMergeSource to BlockDirtyBitmapOrStr

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Compare: https://github.com/qemu/qemu/compare/88d5814e6b02...a72d9008092e
reply via email to
[Prev in Thread] Current Thread [Next in Thread]