| Message ID | 20191015103900.313928-1-its@irrelevant.dk (mailing list archive) |
|---|---|
| Headers | show |
| Series | nvme: support NVMe v1.3d, SGLs and multiple namespaces | expand |
Patchew URL: https://patchew.org/QEMU/20191015103900.313928-1-its@irrelevant.dk/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/misc/imx7_gpr.o
CC hw/misc/mst_fpga.o
/tmp/qemu-test/src/hw/block/nvme.c: In function 'nvme_map_prp':
/tmp/qemu-test/src/hw/block/nvme.c:232:42: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) prp2);
^
/tmp/qemu-test/src/hw/block/nvme.c:258:50: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) prp_ent);
^
/tmp/qemu-test/src/hw/block/nvme.c: In function 'nvme_map_sgl':
/tmp/qemu-test/src/hw/block/nvme.c:414:42: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) addr);
^
/tmp/qemu-test/src/hw/block/nvme.c:429:38: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) addr);
^
/tmp/qemu-test/src/hw/block/nvme.c:478:38: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) addr);
^
/tmp/qemu-test/src/hw/block/nvme.c:493:34: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) addr);
^
/tmp/qemu-test/src/hw/block/nvme.c: In function 'nvme_post_cqes':
/tmp/qemu-test/src/hw/block/nvme.c:847:39: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_write((void *) addr);
^
/tmp/qemu-test/src/hw/block/nvme.c: In function 'nvme_process_sq':
/tmp/qemu-test/src/hw/block/nvme.c:1971:38: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
trace_nvme_err_addr_read((void *) addr);
^
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/block/nvme.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 662, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8aa0a85fff1f457c9dc7c826d7b3189d', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2g1bl41s/src/docker-src.2019-10-15-13.13.48.993:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=8aa0a85fff1f457c9dc7c826d7b3189d
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2g1bl41s/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 5m56.522s
user 0m7.913s
The full log is available at
http://patchew.org/logs/20191015103900.313928-1-its@irrelevant.dk/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20191015103900.313928-1-its@irrelevant.dk/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v2 00/20] nvme: support NVMe v1.3d, SGLs and multiple namespaces
Type: series
Message-id: 20191015103900.313928-1-its@irrelevant.dk
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
c68f7e0 nvme: handle dma errors
855f2b8 nvme: make lba data size configurable
68fc575 nvme: remove redundant NvmeCmd pointer parameter
eb585d1 nvme: bump controller pci device id
227280c nvme: support multiple namespaces
ccc877b nvme: add support for scatter gather lists
76d6fe6 nvme: allow multiple aios per command
73227cb nvme: refactor prp mapping
df5fd9f nvme: bump supported specification version to 1.3
c85c0ff nvme: add missing mandatory features
1188552 nvme: add logging to error information log page
714808c nvme: add support for the asynchronous event request command
88bdfce nvme: add support for the get log page command
7716649 nvme: refactor device realization
7d2d51e nvme: add support for the abort command
4ec0e81 nvme: allow completion queues in the cmb
68f00db nvme: populate the mandatory subnqn and ver fields
f08d66a nvme: add missing fields in the identify controller data structure
315a6eb nvme: move device parameters to separate struct
b94cf4a nvme: remove superfluous breaks
=== OUTPUT BEGIN ===
1/20 Checking commit b94cf4aea07b (nvme: remove superfluous breaks)
2/20 Checking commit 315a6eb1f09f (nvme: move device parameters to separate struct)
ERROR: Macros with complex values should be enclosed in parenthesis
#177: FILE: hw/block/nvme.h:6:
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+ DEFINE_PROP_STRING("serial", _state, _props.serial), \
+ DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+ DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)
total: 1 errors, 0 warnings, 181 lines checked
Patch 2/20 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/20 Checking commit f08d66aa761b (nvme: add missing fields in the identify controller data structure)
4/20 Checking commit 68f00db57e87 (nvme: populate the mandatory subnqn and ver fields)
5/20 Checking commit 4ec0e81a8ca5 (nvme: allow completion queues in the cmb)
6/20 Checking commit 7d2d51e5da89 (nvme: add support for the abort command)
7/20 Checking commit 7716649c3d6d (nvme: refactor device realization)
8/20 Checking commit 88bdfce1a599 (nvme: add support for the get log page command)
9/20 Checking commit 714808cd3ef8 (nvme: add support for the asynchronous event request command)
10/20 Checking commit 11885522fa87 (nvme: add logging to error information log page)
11/20 Checking commit c85c0ff5ea35 (nvme: add missing mandatory features)
12/20 Checking commit df5fd9f283a4 (nvme: bump supported specification version to 1.3)
13/20 Checking commit 73227cb3c83c (nvme: refactor prp mapping)
14/20 Checking commit 76d6fe6ea1cf (nvme: allow multiple aios per command)
15/20 Checking commit ccc877b6f72b (nvme: add support for scatter gather lists)
16/20 Checking commit 227280c8d08c (nvme: support multiple namespaces)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42:
new file mode 100644
total: 0 errors, 1 warnings, 801 lines checked
Patch 16/20 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/20 Checking commit eb585d1231e3 (nvme: bump controller pci device id)
18/20 Checking commit 68fc575b3fc7 (nvme: remove redundant NvmeCmd pointer parameter)
19/20 Checking commit 855f2b86dd6c (nvme: make lba data size configurable)
20/20 Checking commit c68f7e0d0c55 (nvme: handle dma errors)
WARNING: line over 80 characters
#77: FILE: hw/block/nvme.c:257:
+ if (nvme_addr_read(n, prp_ent, (void *) prp_list, prp_trans)) {
WARNING: line over 80 characters
#103: FILE: hw/block/nvme.c:428:
+ if (nvme_addr_read(n, addr, segment, nsgld * sizeof(NvmeSglDescriptor))) {
total: 0 errors, 2 warnings, 148 lines checked
Patch 20/20 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20191015103900.313928-1-its@irrelevant.dk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Tue, 10/15 12:38, Klaus Jensen wrote: > Hi, > > (Quick note to Fam): most of this series is irrelevant to you as the > maintainer of the nvme block driver, but patch "nvme: add support for > scatter gather lists" touches block/nvme.c due to changes in the shared > NvmeCmd struct. Yeah, that part looks sane to me. For the block/nvme.c bit: Acked-by: Fam Zheng <fam@euphon.net>
On Tue, Oct 15, 2019 at 12:38:40PM +0200, Klaus Jensen wrote: > Hi, > > (Quick note to Fam): most of this series is irrelevant to you as the > maintainer of the nvme block driver, but patch "nvme: add support for > scatter gather lists" touches block/nvme.c due to changes in the shared > NvmeCmd struct. > > Anyway, v2 comes with a good bunch of changes. Compared to v1[1], I have > squashed some commits in the beginning of the series and heavily > refactored "nvme: support multiple block requests per request" into the > new commit "nvme: allow multiple aios per command". > > I have also removed the original implementation of the Abort command > (commit "nvme: add support for the abort command") as it is currently > too tricky to test reliably. It has been replaced by a stub that, > besides a trivial sanity check, just fails to abort the given command. > *Some* implementation of the Abort command is mandatory, but given the > "best effort" nature of the command this is acceptable for now. When the > device gains support for arbitration it should be less tricky to test. > > The support for multiple namespaces is now backwards compatible. The > nvme device still accepts a 'drive' parameter, but for multiple > namespaces the use of 'nvme-ns' devices are required. I also integrated > some feedback from Paul so the device supports non-consecutive namespace > ids. > > I have also added some new commits at the end: > > - "nvme: bump controller pci device id" makes sure the Linux kernel > doesn't apply any quirks to the controller that it no longer has. > - "nvme: handle dma errors" won't actually do anything before this[2] > fix to include/hw/pci/pci.h is merged. With these two patches added, > the device reliably passes some additional nasty tests from blktests > (block/011 "disable PCI device while doing I/O" and block/019 "break > PCI link device while doing I/O"). Before this patch, block/011 > would pass from time to time if you were lucky, but would at least > mess up the controller pretty badly, causing a reset in the best > case. > > > [1]: https://patchwork.kernel.org/project/qemu-devel/list/?series=142383 > [2]: https://patchwork.kernel.org/patch/11184911/ > > > Klaus Jensen (20): > nvme: remove superfluous breaks > nvme: move device parameters to separate struct > nvme: add missing fields in the identify controller data structure > nvme: populate the mandatory subnqn and ver fields > nvme: allow completion queues in the cmb > nvme: add support for the abort command > nvme: refactor device realization > nvme: add support for the get log page command > nvme: add support for the asynchronous event request command > nvme: add logging to error information log page > nvme: add missing mandatory features > nvme: bump supported specification version to 1.3 > nvme: refactor prp mapping > nvme: allow multiple aios per command > nvme: add support for scatter gather lists > nvme: support multiple namespaces > nvme: bump controller pci device id > nvme: remove redundant NvmeCmd pointer parameter > nvme: make lba data size configurable > nvme: handle dma errors > > block/nvme.c | 18 +- > hw/block/Makefile.objs | 2 +- > hw/block/nvme-ns.c | 139 +++ > hw/block/nvme-ns.h | 60 ++ > hw/block/nvme.c | 1863 +++++++++++++++++++++++++++++++++------- > hw/block/nvme.h | 219 ++++- > hw/block/trace-events | 37 +- > include/block/nvme.h | 132 ++- > 8 files changed, 2094 insertions(+), 376 deletions(-) > create mode 100644 hw/block/nvme-ns.c > create mode 100644 hw/block/nvme-ns.h > > -- > 2.23.0 > Gentle ping on this. I'm aware that this is a lot to go through, but I would like to know if anyone has had a chance to look at it? https://patchwork.kernel.org/project/qemu-devel/list/?series=187637
Hi, (Quick note to Fam): most of this series is irrelevant to you as the maintainer of the nvme block driver, but patch "nvme: add support for scatter gather lists" touches block/nvme.c due to changes in the shared NvmeCmd struct. Anyway, v2 comes with a good bunch of changes. Compared to v1[1], I have squashed some commits in the beginning of the series and heavily refactored "nvme: support multiple block requests per request" into the new commit "nvme: allow multiple aios per command". I have also removed the original implementation of the Abort command (commit "nvme: add support for the abort command") as it is currently too tricky to test reliably. It has been replaced by a stub that, besides a trivial sanity check, just fails to abort the given command. *Some* implementation of the Abort command is mandatory, but given the "best effort" nature of the command this is acceptable for now. When the device gains support for arbitration it should be less tricky to test. The support for multiple namespaces is now backwards compatible. The nvme device still accepts a 'drive' parameter, but for multiple namespaces the use of 'nvme-ns' devices are required. I also integrated some feedback from Paul so the device supports non-consecutive namespace ids. I have also added some new commits at the end: - "nvme: bump controller pci device id" makes sure the Linux kernel doesn't apply any quirks to the controller that it no longer has. - "nvme: handle dma errors" won't actually do anything before this[2] fix to include/hw/pci/pci.h is merged. With these two patches added, the device reliably passes some additional nasty tests from blktests (block/011 "disable PCI device while doing I/O" and block/019 "break PCI link device while doing I/O"). Before this patch, block/011 would pass from time to time if you were lucky, but would at least mess up the controller pretty badly, causing a reset in the best case. [1]: https://patchwork.kernel.org/project/qemu-devel/list/?series=142383 [2]: https://patchwork.kernel.org/patch/11184911/ Klaus Jensen (20): nvme: remove superfluous breaks nvme: move device parameters to separate struct nvme: add missing fields in the identify controller data structure nvme: populate the mandatory subnqn and ver fields nvme: allow completion queues in the cmb nvme: add support for the abort command nvme: refactor device realization nvme: add support for the get log page command nvme: add support for the asynchronous event request command nvme: add logging to error information log page nvme: add missing mandatory features nvme: bump supported specification version to 1.3 nvme: refactor prp mapping nvme: allow multiple aios per command nvme: add support for scatter gather lists nvme: support multiple namespaces nvme: bump controller pci device id nvme: remove redundant NvmeCmd pointer parameter nvme: make lba data size configurable nvme: handle dma errors block/nvme.c | 18 +- hw/block/Makefile.objs | 2 +- hw/block/nvme-ns.c | 139 +++ hw/block/nvme-ns.h | 60 ++ hw/block/nvme.c | 1863 +++++++++++++++++++++++++++++++++------- hw/block/nvme.h | 219 ++++- hw/block/trace-events | 37 +- include/block/nvme.h | 132 ++- 8 files changed, 2094 insertions(+), 376 deletions(-) create mode 100644 hw/block/nvme-ns.c create mode 100644 hw/block/nvme-ns.h