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