Message ID | 20200224143008.13362-1-kwolf@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | Add qemu-storage-daemon | expand |
Patchew URL: https://patchew.org/QEMU/20200224143008.13362-1-kwolf@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v2 00/20] Add qemu-storage-daemon Message-id: 20200224143008.13362-1-kwolf@redhat.com Type: series === 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 === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 15c7c50 qemu-storage-daemon: Add --monitor option c659b9c monitor: Add allow_hmp parameter to monitor_init() 101c369 hmp: Fail gracefully if chardev is already in use b8da228 qmp: Fail gracefully if chardev is already in use 9a25c79 monitor: Create QAPIfied monitor_init() 5bcc6b7 qapi: Create 'pragma' module e63b4a1 stubs: Update monitor stubs for qemu-storage-daemon 5df46cf qemu-storage-daemon: Add --chardev option ec67d87 qemu-storage-daemon: Add main loop 2176838 qemu-storage-daemon: Add --export option d5531ad blockdev-nbd: Boxed argument type for nbd-server-add 43fa74c qemu-storage-daemon: Add --nbd-server option 0225824 qemu-storage-daemon: Add --object option cfd2dad qapi: Flatten object-add aa6c1fa qemu-storage-daemon: Add --blockdev option db1e4ec block: Move sysemu QMP commands to QAPI block module 745dfca block: Move common QMP commands to block-core QAPI module ee5879e block: Move system emulator QMP commands to block/qapi-sysemu.c 21f9779 stubs: Add arch_type bec56f4 qemu-storage-daemon: Add barebone tool === OUTPUT BEGIN === 1/20 Checking commit bec56f4964e3 (qemu-storage-daemon: Add barebone tool) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #46: new file mode 100644 total: 0 errors, 1 warnings, 142 lines checked Patch 1/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/20 Checking commit 21f97790fa55 (stubs: Add arch_type) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #37: new file mode 100644 total: 0 errors, 1 warnings, 16 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 ee5879e3fd09 (block: Move system emulator QMP commands to block/qapi-sysemu.c) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #29: new file mode 100644 WARNING: Block comments use a leading /* on a separate line #255: FILE: block/qapi-sysemu.c:222: + /* For tray-less devices, blockdev-open-tray is a no-op (or may not be WARNING: Block comments use a trailing */ on a separate line #258: FILE: block/qapi-sysemu.c:225: + * value passed here (i.e. false). */ WARNING: Block comments use a leading /* on a separate line #302: FILE: block/qapi-sysemu.c:269: + /* For tray-less devices, blockdev-close-tray is a no-op (or may not be WARNING: Block comments use a trailing */ on a separate line #306: FILE: block/qapi-sysemu.c:273: + * value passed here (i.e. true). */ WARNING: Block comments use a leading /* on a separate line #439: FILE: block/qapi-sysemu.c:406: + /* If the medium has been inserted, the device has its own reference, so WARNING: Block comments use a trailing */ on a separate line #441: FILE: block/qapi-sysemu.c:408: + * the reference must be relinquished anyway */ WARNING: Block comments use a leading /* on a separate line #548: FILE: block/qapi-sysemu.c:515: + /* Enable I/O limits if they're not enabled yet, otherwise WARNING: Block comments use a trailing */ on a separate line #549: FILE: block/qapi-sysemu.c:516: + * just update the throttling group. */ total: 0 errors, 9 warnings, 1187 lines checked Patch 3/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/20 Checking commit 745dfca103d7 (block: Move common QMP commands to block-core QAPI module) 5/20 Checking commit db1e4ec67dea (block: Move sysemu QMP commands to QAPI block module) 6/20 Checking commit aa6c1fa307e6 (qemu-storage-daemon: Add --blockdev option) 7/20 Checking commit cfd2dadc400f (qapi: Flatten object-add) 8/20 Checking commit 0225824d841a (qemu-storage-daemon: Add --object option) WARNING: Block comments use a leading /* on a separate line #100: FILE: qemu-storage-daemon.c:155: + /* FIXME The keyval parser rejects 'help' arguments, so we must WARNING: Block comments use a trailing */ on a separate line #101: FILE: qemu-storage-daemon.c:156: + * unconditionall try QemuOpts first. */ total: 0 errors, 2 warnings, 98 lines checked Patch 8/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 9/20 Checking commit 43fa74cb2589 (qemu-storage-daemon: Add --nbd-server option) 10/20 Checking commit d5531ad747e1 (blockdev-nbd: Boxed argument type for nbd-server-add) 11/20 Checking commit 21768386959d (qemu-storage-daemon: Add --export option) 12/20 Checking commit ec67d87d63e9 (qemu-storage-daemon: Add main loop) ERROR: do not initialise statics to 0 or NULL #43: FILE: qemu-storage-daemon.c:56: +static volatile bool exit_requested = false; ERROR: Use of volatile is usually wrong, please add a comment #43: FILE: qemu-storage-daemon.c:56: +static volatile bool exit_requested = false; total: 2 errors, 0 warnings, 40 lines checked Patch 12/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 13/20 Checking commit 5df46cf77e9f (qemu-storage-daemon: Add --chardev option) ERROR: externs should be avoided in .c files #63: FILE: qemu-storage-daemon.c:117: +extern QemuOptsList qemu_chardev_opts; total: 1 errors, 0 warnings, 67 lines checked Patch 13/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 14/20 Checking commit e63b4a1f955a (stubs: Update monitor stubs for qemu-storage-daemon) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #31: new file mode 100644 total: 0 errors, 1 warnings, 58 lines checked Patch 14/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 15/20 Checking commit 5bcc6b7c65a7 (qapi: Create 'pragma' module) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #28: new file mode 100644 total: 0 errors, 1 warnings, 63 lines checked Patch 15/20 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 16/20 Checking commit 9a25c79eb12c (monitor: Create QAPIfied monitor_init()) ERROR: space required before the open parenthesis '(' #102: FILE: monitor/monitor.c:624: + switch(opts->mode) { total: 1 errors, 0 warnings, 153 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 b8da22839b6d (qmp: Fail gracefully if chardev is already in use) 18/20 Checking commit 101c36993299 (hmp: Fail gracefully if chardev is already in use) 19/20 Checking commit c659b9c72c1f (monitor: Add allow_hmp parameter to monitor_init()) 20/20 Checking commit 15c7c5049df6 (qemu-storage-daemon: Add --monitor option) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #266: new file mode 100644 total: 0 errors, 1 warnings, 228 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/20200224143008.13362-1-kwolf@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Mon, Feb 24, 2020 at 03:29:48PM +0100, Kevin Wolf wrote: > This series adds a new tool 'qemu-storage-daemon', which can be used to > export and perform operations on block devices. There is some overlap > between qemu-img/qemu-nbd and the new qemu-storage-daemon, but there are > a few important differences: > > * The qemu-storage-daemon has QMP support. The command set is obviously > restricted compared to the system emulator because there is no guest, > but all of the block operations that are not tied to gues devices are > present. > > This means that it can access advanced options or operations that the > qemu-img command line doesn't expose. For example, blockdev-create is > a lot more powerful than 'qemu-img create', and qemu-storage-daemon > allows to execute it without starting a guest. > > Compared to qemu-nbd it means that, for example, block jobs can now be > executed on the server side, and backing chains shared by multiple VMs > can be modified this way. > > * The existing tools all have a separately invented one-off syntax for > the job at hand, which usually comes with restrictions compared to the > system emulator. qemu-storage-daemon shares the same syntax with the > system emulator for most options and prefers QAPI based interfaces > where possible (such as --blockdev), so it should be easy to make use > of in libvirt. > > The exception is --chardev, for which not clear design for a QAPIfied > command line exists yet. We'll consider this interface unstable until > we've figured out how to solve it. For now it just uses the same > QemuOpts-based code as the system emulator. > > * While this series implements only NBD exports, the storage daemon is > intended to serve multiple protocols and its syntax reflects this. In > the past, we had proposals to add new one-off tools for exporting over > new protocols like FUSE or TCMU. > > With a generic storage daemon, additional export methods have a home > without adding a new tool for each of them. > > The plan is to merge qemu-storage-daemon as an experimental feature with > a reduced API stability promise in 5.0. > > Kevin Wolf (20): > qemu-storage-daemon: Add barebone tool > stubs: Add arch_type > block: Move system emulator QMP commands to block/qapi-sysemu.c > block: Move common QMP commands to block-core QAPI module > block: Move sysemu QMP commands to QAPI block module > qemu-storage-daemon: Add --blockdev option > qapi: Flatten object-add > qemu-storage-daemon: Add --object option > qemu-storage-daemon: Add --nbd-server option > blockdev-nbd: Boxed argument type for nbd-server-add > qemu-storage-daemon: Add --export option > qemu-storage-daemon: Add main loop > qemu-storage-daemon: Add --chardev option > stubs: Update monitor stubs for qemu-storage-daemon > qapi: Create 'pragma' module > monitor: Create QAPIfied monitor_init() > qmp: Fail gracefully if chardev is already in use > hmp: Fail gracefully if chardev is already in use > monitor: Add allow_hmp parameter to monitor_init() > qemu-storage-daemon: Add --monitor option > > qapi/block-core.json | 730 +++++++++++++-------------- > qapi/block.json | 512 +++++++++++-------- > qapi/control.json | 37 ++ > qapi/pragma.json | 24 + > qapi/qapi-schema.json | 25 +- > qapi/qom.json | 12 +- > qapi/transaction.json | 2 +- > configure | 2 +- > include/block/nbd.h | 1 + > include/monitor/monitor.h | 6 +- > include/qom/object_interfaces.h | 7 + > include/sysemu/arch_init.h | 2 + > block/qapi-sysemu.c | 590 ++++++++++++++++++++++ > blockdev-nbd.c | 40 +- > blockdev.c | 559 -------------------- > chardev/char.c | 8 +- > gdbstub.c | 2 +- > hw/block/xen-block.c | 11 +- > monitor/hmp-cmds.c | 21 +- > monitor/hmp.c | 8 +- > monitor/misc.c | 2 + > monitor/monitor.c | 86 ++-- > monitor/qmp-cmds.c | 2 +- > monitor/qmp.c | 11 +- > qemu-storage-daemon.c | 340 +++++++++++++ > qom/qom-qmp-cmds.c | 42 +- > stubs/arch_type.c | 4 + > stubs/monitor-core.c | 21 + > stubs/monitor.c | 17 +- > tests/test-util-sockets.c | 4 +- > scripts/qapi/gen.py | 5 + > Makefile | 37 ++ > Makefile.objs | 9 + > block/Makefile.objs | 4 +- > monitor/Makefile.objs | 2 + > qapi/Makefile.objs | 7 +- > qemu-deprecated.texi | 4 + > qom/Makefile.objs | 1 + > storage-daemon/Makefile.objs | 1 + > storage-daemon/qapi/Makefile.objs | 1 + > storage-daemon/qapi/qapi-schema.json | 26 + > stubs/Makefile.objs | 2 + > 42 files changed, 1955 insertions(+), 1272 deletions(-) > create mode 100644 qapi/pragma.json > create mode 100644 block/qapi-sysemu.c > create mode 100644 qemu-storage-daemon.c > create mode 100644 stubs/arch_type.c > create mode 100644 stubs/monitor-core.c > create mode 100644 storage-daemon/Makefile.objs > create mode 100644 storage-daemon/qapi/Makefile.objs > create mode 100644 storage-daemon/qapi/qapi-schema.json I haven't reviewed the patches in detail since they are mostly concerned with command-line interfaces and monitor changes. Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Am 28.02.2020 um 12:16 hat Stefan Hajnoczi geschrieben: > On Mon, Feb 24, 2020 at 03:29:48PM +0100, Kevin Wolf wrote: > > This series adds a new tool 'qemu-storage-daemon', which can be used to > > export and perform operations on block devices. There is some overlap > > between qemu-img/qemu-nbd and the new qemu-storage-daemon, but there are > > a few important differences: > > > > * The qemu-storage-daemon has QMP support. The command set is obviously > > restricted compared to the system emulator because there is no guest, > > but all of the block operations that are not tied to gues devices are > > present. > > > > This means that it can access advanced options or operations that the > > qemu-img command line doesn't expose. For example, blockdev-create is > > a lot more powerful than 'qemu-img create', and qemu-storage-daemon > > allows to execute it without starting a guest. > > > > Compared to qemu-nbd it means that, for example, block jobs can now be > > executed on the server side, and backing chains shared by multiple VMs > > can be modified this way. > > > > * The existing tools all have a separately invented one-off syntax for > > the job at hand, which usually comes with restrictions compared to the > > system emulator. qemu-storage-daemon shares the same syntax with the > > system emulator for most options and prefers QAPI based interfaces > > where possible (such as --blockdev), so it should be easy to make use > > of in libvirt. > > > > The exception is --chardev, for which not clear design for a QAPIfied > > command line exists yet. We'll consider this interface unstable until > > we've figured out how to solve it. For now it just uses the same > > QemuOpts-based code as the system emulator. > > > > * While this series implements only NBD exports, the storage daemon is > > intended to serve multiple protocols and its syntax reflects this. In > > the past, we had proposals to add new one-off tools for exporting over > > new protocols like FUSE or TCMU. > > > > With a generic storage daemon, additional export methods have a home > > without adding a new tool for each of them. > > > > The plan is to merge qemu-storage-daemon as an experimental feature with > > a reduced API stability promise in 5.0. > > > > Kevin Wolf (20): > > qemu-storage-daemon: Add barebone tool > > stubs: Add arch_type > > block: Move system emulator QMP commands to block/qapi-sysemu.c > > block: Move common QMP commands to block-core QAPI module > > block: Move sysemu QMP commands to QAPI block module > > qemu-storage-daemon: Add --blockdev option > > qapi: Flatten object-add > > qemu-storage-daemon: Add --object option > > qemu-storage-daemon: Add --nbd-server option > > blockdev-nbd: Boxed argument type for nbd-server-add > > qemu-storage-daemon: Add --export option > > qemu-storage-daemon: Add main loop > > qemu-storage-daemon: Add --chardev option > > stubs: Update monitor stubs for qemu-storage-daemon > > qapi: Create 'pragma' module > > monitor: Create QAPIfied monitor_init() > > qmp: Fail gracefully if chardev is already in use > > hmp: Fail gracefully if chardev is already in use > > monitor: Add allow_hmp parameter to monitor_init() > > qemu-storage-daemon: Add --monitor option > > > > qapi/block-core.json | 730 +++++++++++++-------------- > > qapi/block.json | 512 +++++++++++-------- > > qapi/control.json | 37 ++ > > qapi/pragma.json | 24 + > > qapi/qapi-schema.json | 25 +- > > qapi/qom.json | 12 +- > > qapi/transaction.json | 2 +- > > configure | 2 +- > > include/block/nbd.h | 1 + > > include/monitor/monitor.h | 6 +- > > include/qom/object_interfaces.h | 7 + > > include/sysemu/arch_init.h | 2 + > > block/qapi-sysemu.c | 590 ++++++++++++++++++++++ > > blockdev-nbd.c | 40 +- > > blockdev.c | 559 -------------------- > > chardev/char.c | 8 +- > > gdbstub.c | 2 +- > > hw/block/xen-block.c | 11 +- > > monitor/hmp-cmds.c | 21 +- > > monitor/hmp.c | 8 +- > > monitor/misc.c | 2 + > > monitor/monitor.c | 86 ++-- > > monitor/qmp-cmds.c | 2 +- > > monitor/qmp.c | 11 +- > > qemu-storage-daemon.c | 340 +++++++++++++ > > qom/qom-qmp-cmds.c | 42 +- > > stubs/arch_type.c | 4 + > > stubs/monitor-core.c | 21 + > > stubs/monitor.c | 17 +- > > tests/test-util-sockets.c | 4 +- > > scripts/qapi/gen.py | 5 + > > Makefile | 37 ++ > > Makefile.objs | 9 + > > block/Makefile.objs | 4 +- > > monitor/Makefile.objs | 2 + > > qapi/Makefile.objs | 7 +- > > qemu-deprecated.texi | 4 + > > qom/Makefile.objs | 1 + > > storage-daemon/Makefile.objs | 1 + > > storage-daemon/qapi/Makefile.objs | 1 + > > storage-daemon/qapi/qapi-schema.json | 26 + > > stubs/Makefile.objs | 2 + > > 42 files changed, 1955 insertions(+), 1272 deletions(-) > > create mode 100644 qapi/pragma.json > > create mode 100644 block/qapi-sysemu.c > > create mode 100644 qemu-storage-daemon.c > > create mode 100644 stubs/arch_type.c > > create mode 100644 stubs/monitor-core.c > > create mode 100644 storage-daemon/Makefile.objs > > create mode 100644 storage-daemon/qapi/Makefile.objs > > create mode 100644 storage-daemon/qapi/qapi-schema.json > > I haven't reviewed the patches in detail since they are mostly concerned > with command-line interfaces and monitor changes. > > Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Thanks, applied to the block branch. Kevin