qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL v2 00/10] Block layer patches
@ 2021-02-02 16:28 Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 01/10] MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs Kevin Wolf
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

The following changes since commit cf7ca7d5b9faca13f1f8e3ea92cfb2f741eb0c0e:

  Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2021-02-01 16:28:00 +0000)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 26513a01741f51650f5dd716681995359794ba6f:

  block: Fix VM size column width in bdrv_snapshot_dump() (2021-02-02 17:23:55 +0100)

----------------------------------------------------------------
Block layer patches:

- Fix double processing of nodes in bdrv_set_aio_context()
- Fix potential hang in block export shutdown
- block/nvme: Minor tracing improvements
- iotests: Some more fixups for the 'check' rewrite
- MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs

----------------------------------------------------------------
Kevin Wolf (3):
      iotests: Revert emulator selection to old behaviour
      iotests: Fix -makecheck output
      block: Fix VM size column width in bdrv_snapshot_dump()

Philippe Mathieu-Daudé (2):
      block/nvme: Properly display doorbell stride length in trace event
      block/nvme: Trace NVMe spec version supported by the controller

Sergio Lopez (2):
      block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()
      block: move blk_exp_close_all() to qemu_cleanup()

Vladimir Sementsov-Ogievskiy (3):
      MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs
      iotests/297: pylint: ignore too many statements
      iotests: check: return 1 on failure

 block.c                              | 35 +++++++++++++++++++++++++++--------
 block/nvme.c                         |  8 +++++++-
 block/qapi.c                         |  4 ++--
 qemu-nbd.c                           |  1 +
 softmmu/runstate.c                   |  9 +++++++++
 storage-daemon/qemu-storage-daemon.c |  1 +
 tests/qemu-iotests/testenv.py        |  2 +-
 tests/qemu-iotests/testrunner.py     | 10 +++++++---
 MAINTAINERS                          | 10 ++++++++++
 block/trace-events                   |  1 +
 tests/qemu-iotests/check             |  5 ++++-
 tests/qemu-iotests/pylintrc          |  2 ++
 12 files changed, 72 insertions(+), 16 deletions(-)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PULL v2 01/10] MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 02/10] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() Kevin Wolf
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

I'm developing Qemu backup for several years, and finally new backup
architecture, including block-copy generic engine and backup-top filter
landed upstream, great thanks to reviewers and especially to
Max Reitz!

I also have plans of moving other block-jobs onto block-copy, so that
we finally have one generic block copying path, fast and well-formed.

So, now I suggest to bring all parts of backup architecture into
"Block Jobs" subsystem (actually, aio_task is shared with qcow2 and
qemu-co-shared-resource can be reused somewhere else, but I'd keep an
eye on them in context of block-jobs) and add myself as co-maintainer.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210128144144.27617-1-vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bcd88668bc..00626941f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2210,6 +2210,7 @@ F: scsi/*
 
 Block Jobs
 M: John Snow <jsnow@redhat.com>
+M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 L: qemu-block@nongnu.org
 S: Supported
 F: blockjob.c
@@ -2222,7 +2223,16 @@ F: block/commit.c
 F: block/stream.c
 F: block/mirror.c
 F: qapi/job.json
+F: block/block-copy.c
+F: include/block/block-copy.c
+F: block/backup-top.h
+F: block/backup-top.c
+F: include/block/aio_task.h
+F: block/aio_task.c
+F: util/qemu-co-shared-resource.c
+F: include/qemu/co-shared-resource.h
 T: git https://gitlab.com/jsnow/qemu.git jobs
+T: git https://src.openvz.org/scm/~vsementsov/qemu.git jobs
 
 Block QAPI, monitor, command line
 M: Markus Armbruster <armbru@redhat.com>
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 02/10] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 01/10] MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 03/10] block: move blk_exp_close_all() to qemu_cleanup() Kevin Wolf
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Sergio Lopez <slp@redhat.com>

Some graphs may contain an indirect reference to the first BDS in the
chain that can be reached while walking it bottom->up from one its
children.

Doubling-processing of a BDS is especially problematic for the
aio_notifiers, as they might attempt to work on both the old and the
new AIO contexts.

To avoid this problem, add every child and parent to the ignore list
before actually processing them.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Sergio Lopez <slp@redhat.com>
Message-Id: <20210201125032.44713-2-slp@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 91a66d4f3e..5c428e1595 100644
--- a/block.c
+++ b/block.c
@@ -6439,7 +6439,10 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
                                  AioContext *new_context, GSList **ignore)
 {
     AioContext *old_context = bdrv_get_aio_context(bs);
-    BdrvChild *child;
+    GSList *children_to_process = NULL;
+    GSList *parents_to_process = NULL;
+    GSList *entry;
+    BdrvChild *child, *parent;
 
     g_assert(qemu_get_current_aio_context() == qemu_get_aio_context());
 
@@ -6454,16 +6457,33 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
             continue;
         }
         *ignore = g_slist_prepend(*ignore, child);
-        bdrv_set_aio_context_ignore(child->bs, new_context, ignore);
+        children_to_process = g_slist_prepend(children_to_process, child);
     }
-    QLIST_FOREACH(child, &bs->parents, next_parent) {
-        if (g_slist_find(*ignore, child)) {
+
+    QLIST_FOREACH(parent, &bs->parents, next_parent) {
+        if (g_slist_find(*ignore, parent)) {
             continue;
         }
-        assert(child->klass->set_aio_ctx);
-        *ignore = g_slist_prepend(*ignore, child);
-        child->klass->set_aio_ctx(child, new_context, ignore);
+        *ignore = g_slist_prepend(*ignore, parent);
+        parents_to_process = g_slist_prepend(parents_to_process, parent);
+    }
+
+    for (entry = children_to_process;
+         entry != NULL;
+         entry = g_slist_next(entry)) {
+        child = entry->data;
+        bdrv_set_aio_context_ignore(child->bs, new_context, ignore);
+    }
+    g_slist_free(children_to_process);
+
+    for (entry = parents_to_process;
+         entry != NULL;
+         entry = g_slist_next(entry)) {
+        parent = entry->data;
+        assert(parent->klass->set_aio_ctx);
+        parent->klass->set_aio_ctx(parent, new_context, ignore);
     }
+    g_slist_free(parents_to_process);
 
     bdrv_detach_aio_context(bs);
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 03/10] block: move blk_exp_close_all() to qemu_cleanup()
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 01/10] MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 02/10] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 04/10] iotests/297: pylint: ignore too many statements Kevin Wolf
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Sergio Lopez <slp@redhat.com>

Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before
bdrv_drain_all_begin().

Export drivers may have coroutines yielding at some point in the block
layer, so we need to shut them down before draining the block layer,
as otherwise they may get stuck blk_wait_while_drained().

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
Signed-off-by: Sergio Lopez <slp@redhat.com>
Message-Id: <20210201125032.44713-3-slp@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                              | 1 -
 qemu-nbd.c                           | 1 +
 softmmu/runstate.c                   | 9 +++++++++
 storage-daemon/qemu-storage-daemon.c | 1 +
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 5c428e1595..4e52b1c588 100644
--- a/block.c
+++ b/block.c
@@ -4435,7 +4435,6 @@ static void bdrv_close(BlockDriverState *bs)
 void bdrv_close_all(void)
 {
     assert(job_next(NULL) == NULL);
-    blk_exp_close_all();
 
     /* Drop references from requests still in flight, such as canceled block
      * jobs whose AIO context has not been polled yet */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 0d513cb38c..608c63e82a 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -503,6 +503,7 @@ static const char *socket_activation_validate_opts(const char *device,
 static void qemu_nbd_shutdown(void)
 {
     job_cancel_sync_all();
+    blk_exp_close_all();
     bdrv_close_all();
 }
 
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index beee050815..a7fcb603f7 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "audio/audio.h"
 #include "block/block.h"
+#include "block/export.h"
 #include "chardev/char.h"
 #include "crypto/cipher.h"
 #include "crypto/init.h"
@@ -784,6 +785,14 @@ void qemu_cleanup(void)
      */
     migration_shutdown();
 
+    /*
+     * Close the exports before draining the block layer. The export
+     * drivers may have coroutines yielding on it, so we need to clean
+     * them up before the drain, as otherwise they may be get stuck in
+     * blk_wait_while_drained().
+     */
+    blk_exp_close_all();
+
     /*
      * We must cancel all block jobs while the block layer is drained,
      * or cancelling will be affected by throttling and thus may block
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index e0c87edbdd..d8d172cc60 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -314,6 +314,7 @@ int main(int argc, char *argv[])
         main_loop_wait(false);
     }
 
+    blk_exp_close_all();
     bdrv_drain_all_begin();
     bdrv_close_all();
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 04/10] iotests/297: pylint: ignore too many statements
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 03/10] block: move blk_exp_close_all() to qemu_cleanup() Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 05/10] iotests: Revert emulator selection to old behaviour Kevin Wolf
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Ignore two complains, which now lead to 297 failure on testenv.py and
testrunner.py.

Fixes: 2e5a2f57db481f18fcf70be2a36b1417370b8476
Fixes: d74c754c924ca34e90b7c96ce2f5609d82c0e628
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210129161323.615027-1-vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/pylintrc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index cd3702e23c..7a6c0a9474 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -21,6 +21,8 @@ disable=invalid-name,
         unsubscriptable-object,
         # These are temporary, and should be removed:
         missing-docstring,
+        too-many-return-statements,
+        too-many-statements
 
 [FORMAT]
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 05/10] iotests: Revert emulator selection to old behaviour
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 04/10] iotests/297: pylint: ignore too many statements Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 06/10] iotests: check: return 1 on failure Kevin Wolf
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

If the qemu-system-{arch} binary for the host architecture can't be
found, the old 'check' implementation selected the alphabetically first
system emulator binary that it could find. The new Python implementation
just uses the first result of glob.iglob(), which has an undefined
order.

This is a problem that breaks CI because the iotests aren't actually
prepared to run on any emulator. They should be, so this is really a bug
in the failing test cases that should be fixed there, but as a quick
fix, let's revert to the old behaviour to let CI runs succeed again.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210202142802.119999-1-kwolf@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/testenv.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index b31275f518..1fbec854c1 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -135,7 +135,7 @@ class TestEnv(ContextManager['TestEnv']):
         if not os.path.exists(self.qemu_prog):
             pattern = root('qemu-system-*')
             try:
-                progs = glob.iglob(pattern)
+                progs = sorted(glob.iglob(pattern))
                 self.qemu_prog = next(p for p in progs if isxfile(p))
             except StopIteration:
                 sys.exit("Not found any Qemu executable binary by pattern "
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 06/10] iotests: check: return 1 on failure
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 05/10] iotests: Revert emulator selection to old behaviour Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 07/10] iotests: Fix -makecheck output Kevin Wolf
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

We should indicate failure by exit code, not only output.

Reported-by: Peter Maydell
Fixes: f203080bbd9f9e5b31041b1f2afcd6040c5aaec5
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210201085041.3079-1-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/testrunner.py | 4 +++-
 tests/qemu-iotests/check         | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 24b3fba115..25754e9a09 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -318,7 +318,7 @@ class TestRunner(ContextManager['TestRunner']):
 
         return res
 
-    def run_tests(self, tests: List[str]) -> None:
+    def run_tests(self, tests: List[str]) -> bool:
         n_run = 0
         failed = []
         notrun = []
@@ -363,5 +363,7 @@ class TestRunner(ContextManager['TestRunner']):
         if failed:
             print('Failures:', ' '.join(failed))
             print(f'Failed {len(failed)} of {n_run} iotests')
+            return False
         else:
             print(f'Passed all {n_run} iotests')
+            return True
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 5190dee82e..d1c87ceaf1 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -140,4 +140,7 @@ if __name__ == '__main__':
     else:
         with TestRunner(env, makecheck=args.makecheck,
                         color=args.color) as tr:
-            tr.run_tests([os.path.join(env.source_iotests, t) for t in tests])
+            paths = [os.path.join(env.source_iotests, t) for t in tests]
+            ok = tr.run_tests(paths)
+            if not ok:
+                sys.exit(1)
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 07/10] iotests: Fix -makecheck output
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 06/10] iotests: check: return 1 on failure Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 08/10] block/nvme: Properly display doorbell stride length in trace event Kevin Wolf
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

For -makecheck, the old 'check' implementation skipped the output when
starting a test. It only had the condensed output at the end of a test.

testrunner.py prints the normal output when starting a test even for
-makecheck. This output contains '\r' at the end so that it can be
overwritten with the result at the end of the test. However, for
-makecheck this is shorter output in a different format, so effectively
we end up with garbled output that mixes both output forms.

Revert to the old behaviour of only printing a message after the test
had completed in -makecheck mode.

Fixes: d74c754c924ca34e90b7c96ce2f5609d82c0e628
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210201161024.127921-1-kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/testrunner.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 25754e9a09..1fc61fcaa3 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -301,8 +301,10 @@ class TestRunner(ContextManager['TestRunner']):
         last_el = self.last_elapsed.get(test)
         start = datetime.datetime.now().strftime('%H:%M:%S')
 
-        self.test_print_one_line(test=test, starttime=start, lasttime=last_el,
-                                 end='\r', test_field_width=test_field_width)
+        if not self.makecheck:
+            self.test_print_one_line(test=test, starttime=start,
+                                     lasttime=last_el, end='\r',
+                                     test_field_width=test_field_width)
 
         res = self.do_run_test(test)
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 08/10] block/nvme: Properly display doorbell stride length in trace event
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (6 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 07/10] iotests: Fix -makecheck output Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 09/10] block/nvme: Trace NVMe spec version supported by the controller Kevin Wolf
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Commit 15b2260bef3 ("block/nvme: Trace controller capabilities")
misunderstood the doorbell stride value from the datasheet, use
the correct one. The 'doorbell_scale' variable used few lines
later is correct.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210127212137.3482291-2-philmd@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index 5a6fbacf4a..80c4318d8f 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -745,7 +745,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     trace_nvme_controller_capability("Contiguous Queues Required",
                                      NVME_CAP_CQR(cap));
     trace_nvme_controller_capability("Doorbell Stride",
-                                     2 << (2 + NVME_CAP_DSTRD(cap)));
+                                     1 << (2 + NVME_CAP_DSTRD(cap)));
     trace_nvme_controller_capability("Subsystem Reset Supported",
                                      NVME_CAP_NSSRS(cap));
     trace_nvme_controller_capability("Memory Page Size Minimum",
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 09/10] block/nvme: Trace NVMe spec version supported by the controller
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (7 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 08/10] block/nvme: Properly display doorbell stride length in trace event Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 16:28 ` [PULL v2 10/10] block: Fix VM size column width in bdrv_snapshot_dump() Kevin Wolf
  2021-02-02 17:57 ` [PULL v2 00/10] Block layer patches Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Philippe Mathieu-Daudé <philmd@redhat.com>

NVMe controllers implement different versions of the spec,
and different features of it. It is useful to gather this
information when debugging.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210127212137.3482291-3-philmd@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/nvme.c       | 6 ++++++
 block/trace-events | 1 +
 2 files changed, 7 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index 80c4318d8f..2b5421e7aa 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -708,6 +708,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     AioContext *aio_context = bdrv_get_aio_context(bs);
     int ret;
     uint64_t cap;
+    uint32_t ver;
     uint64_t timeout_ms;
     uint64_t deadline, now;
     volatile NvmeBar *regs = NULL;
@@ -764,6 +765,11 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     bs->bl.request_alignment = s->page_size;
     timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000);
 
+    ver = le32_to_cpu(regs->vs);
+    trace_nvme_controller_spec_version(extract32(ver, 16, 16),
+                                       extract32(ver, 8, 8),
+                                       extract32(ver, 0, 8));
+
     /* Reset device to get a clean state. */
     regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE);
     /* Wait for CSTS.RDY = 0. */
diff --git a/block/trace-events b/block/trace-events
index 8368f4acb0..ecbc32a80a 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -136,6 +136,7 @@ qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s
 # nvme.c
 nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64
 nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64
+nvme_controller_spec_version(uint32_t mjr, uint32_t mnr, uint32_t ter) "Specification supported: %u.%u.%u"
 nvme_kick(void *s, unsigned q_index) "s %p q #%u"
 nvme_dma_flush_queue_wait(void *s) "s %p"
 nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL v2 10/10] block: Fix VM size column width in bdrv_snapshot_dump()
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (8 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 09/10] block/nvme: Trace NVMe spec version supported by the controller Kevin Wolf
@ 2021-02-02 16:28 ` Kevin Wolf
  2021-02-02 17:57 ` [PULL v2 00/10] Block layer patches Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2021-02-02 16:28 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

size_to_str() can return a size like "4.24 MiB", with a single digit
integer part and two fractional digits. This is eight characters, but
commit b39847a5 changed the format string to only reserve seven
characters for the column.

This can result in unaligned columns, which in turn changes the output of
iotests case 267 because exceeding the column size defeats the attempt
to filter the size out of the output (observed with the ppc64 emulator).
The resulting change is only a whitespace change, but since commit
f203080b this is enough for iotests to consider the test failed.

Taking a character away from the tag name column and adding it to the VM
size column doesn't change anything in the common case (the tag name is
left justified, the VM size is right justified), but fixes this case.

Fixes: b39847a50553b7679d6d7fefbe6a108a17aacf8d
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210202155911.179865-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 0a96099e36..84a0aadc09 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -677,7 +677,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
     char *sizing = NULL;
 
     if (!sn) {
-        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
+        qemu_printf("%-10s%-17s%8s%20s%13s%11s",
                     "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
     } else {
         ti = sn->date_sec;
@@ -696,7 +696,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
             snprintf(icount_buf, sizeof(icount_buf),
                 "%"PRId64, sn->icount);
         }
-        qemu_printf("%-9s %-17s %7s%20s%13s%11s",
+        qemu_printf("%-9s %-16s %8s%20s%13s%11s",
                     sn->id_str, sn->name,
                     sizing,
                     date_buf,
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PULL v2 00/10] Block layer patches
  2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
                   ` (9 preceding siblings ...)
  2021-02-02 16:28 ` [PULL v2 10/10] block: Fix VM size column width in bdrv_snapshot_dump() Kevin Wolf
@ 2021-02-02 17:57 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2021-02-02 17:57 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block

On Tue, 2 Feb 2021 at 16:28, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit cf7ca7d5b9faca13f1f8e3ea92cfb2f741eb0c0e:
>
>   Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2021-02-01 16:28:00 +0000)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 26513a01741f51650f5dd716681995359794ba6f:
>
>   block: Fix VM size column width in bdrv_snapshot_dump() (2021-02-02 17:23:55 +0100)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - Fix double processing of nodes in bdrv_set_aio_context()
> - Fix potential hang in block export shutdown
> - block/nvme: Minor tracing improvements
> - iotests: Some more fixups for the 'check' rewrite
> - MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs

Yep, this has fixed the issues with the NetBSD VM setup.

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-02-02 18:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 16:28 [PULL v2 00/10] Block layer patches Kevin Wolf
2021-02-02 16:28 ` [PULL v2 01/10] MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs Kevin Wolf
2021-02-02 16:28 ` [PULL v2 02/10] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() Kevin Wolf
2021-02-02 16:28 ` [PULL v2 03/10] block: move blk_exp_close_all() to qemu_cleanup() Kevin Wolf
2021-02-02 16:28 ` [PULL v2 04/10] iotests/297: pylint: ignore too many statements Kevin Wolf
2021-02-02 16:28 ` [PULL v2 05/10] iotests: Revert emulator selection to old behaviour Kevin Wolf
2021-02-02 16:28 ` [PULL v2 06/10] iotests: check: return 1 on failure Kevin Wolf
2021-02-02 16:28 ` [PULL v2 07/10] iotests: Fix -makecheck output Kevin Wolf
2021-02-02 16:28 ` [PULL v2 08/10] block/nvme: Properly display doorbell stride length in trace event Kevin Wolf
2021-02-02 16:28 ` [PULL v2 09/10] block/nvme: Trace NVMe spec version supported by the controller Kevin Wolf
2021-02-02 16:28 ` [PULL v2 10/10] block: Fix VM size column width in bdrv_snapshot_dump() Kevin Wolf
2021-02-02 17:57 ` [PULL v2 00/10] Block layer patches Peter Maydell

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).