| Message ID | 20210413160850.240064-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Patchew URL: https://patchew.org/QEMU/20210413160850.240064-1-pbonzini@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210413160850.240064-1-pbonzini@redhat.com Subject: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 === 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' 487722a qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code 588f61f osdep: protect qemu/osdep.h with extern "C" 5316327 osdep: include glib-compat.h before other QEMU headers === OUTPUT BEGIN === 1/3 Checking commit 5316327519a7 (osdep: include glib-compat.h before other QEMU headers) 2/3 Checking commit 588f61fea9ae (osdep: protect qemu/osdep.h with extern "C") WARNING: architecture specific defines should be avoided #51: FILE: include/qemu/compiler.h:14: +#ifdef __cplusplus ERROR: storage class should be at the beginning of the declaration #52: FILE: include/qemu/compiler.h:15: +#define QEMU_EXTERN_C extern "C" ERROR: storage class should be at the beginning of the declaration #54: FILE: include/qemu/compiler.h:17: +#define QEMU_EXTERN_C extern WARNING: architecture specific defines should be avoided #77: FILE: include/qemu/osdep.h:116: +#ifdef __cplusplus WARNING: architecture specific defines should be avoided #88: FILE: include/qemu/osdep.h:730: +#ifdef __cplusplus total: 2 errors, 3 warnings, 47 lines checked Patch 2/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/3 Checking commit 487722a3d4ef (qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210413160850.240064-1-pbonzini@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > > The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620: > > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100) > > are available in the Git repository at: > > https://gitlab.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241: > > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200) > > ---------------------------------------------------------------- > * Fix C++ compilation of qemu/osdep.h. > * Fix -object cryptodev-vhost-user > > ---------------------------------------------------------------- > Paolo Bonzini (2): > osdep: include glib-compat.h before other QEMU headers > osdep: protect qemu/osdep.h with extern "C" > > Thomas Huth (1): > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code This still seems to have the same version of patch 2 that I gave review comments on, and which you haven't replied to ? thanks -- PMM
On Tue, 13 Apr 2021 at 21:15, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620: > > > > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100) > > > > are available in the Git repository at: > > > > https://gitlab.com/bonzini/qemu.git tags/for-upstream > > > > for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241: > > > > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200) > > > > ---------------------------------------------------------------- > > * Fix C++ compilation of qemu/osdep.h. > > * Fix -object cryptodev-vhost-user > > > > ---------------------------------------------------------------- > > Paolo Bonzini (2): > > osdep: include glib-compat.h before other QEMU headers > > osdep: protect qemu/osdep.h with extern "C" > > > > Thomas Huth (1): > > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code > > This still seems to have the same version of patch 2 that I gave > review comments on, and which you haven't replied to ? Ping? I'd like to tag rc3 today... I guess we could just leave out the extern C related changes, as they're not a regression since 5.2. thanks -- PMM
On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > > The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620: > > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100) > > are available in the Git repository at: > > https://gitlab.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241: > > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200) > > ---------------------------------------------------------------- > * Fix C++ compilation of qemu/osdep.h. > * Fix -object cryptodev-vhost-user > > ---------------------------------------------------------------- > Paolo Bonzini (2): > osdep: include glib-compat.h before other QEMU headers > osdep: protect qemu/osdep.h with extern "C" > > Thomas Huth (1): > qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code Given Dan's review, I think that the osdep patches need another revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO patch here and tag rc3 with just that. If we need an rc4 (which on our current track record is not unlikely) we can put in some version of the osdep patches; if not, this isn't a regression since 5.2 so I'm happy releasing 6.0 with it still present. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620: >> >> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100) >> >> are available in the Git repository at: >> >> https://gitlab.com/bonzini/qemu.git tags/for-upstream >> >> for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241: >> >> qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200) >> >> ---------------------------------------------------------------- >> * Fix C++ compilation of qemu/osdep.h. >> * Fix -object cryptodev-vhost-user >> >> ---------------------------------------------------------------- >> Paolo Bonzini (2): >> osdep: include glib-compat.h before other QEMU headers >> osdep: protect qemu/osdep.h with extern "C" >> >> Thomas Huth (1): >> qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code > > Given Dan's review, I think that the osdep patches need another > revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO > patch here and tag rc3 with just that. If we need an rc4 (which Uh, I had a question on that one: Message-ID: <87tuo9j7hw.fsf@dusky.pond.sub.org> https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02341.html > on our current track record is not unlikely) we can put in some > version of the osdep patches; if not, this isn't a regression > since 5.2 so I'm happy releasing 6.0 with it still present. > > thanks > -- PMM
On Thu, 15 Apr 2021 at 06:57, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > >> Paolo Bonzini (2): > >> osdep: include glib-compat.h before other QEMU headers > >> osdep: protect qemu/osdep.h with extern "C" > >> > >> Thomas Huth (1): > >> qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code > > > > Given Dan's review, I think that the osdep patches need another > > revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO > > patch here and tag rc3 with just that. If we need an rc4 (which > > Uh, I had a question on that one: > > Message-ID: <87tuo9j7hw.fsf@dusky.pond.sub.org> > https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02341.html Sorry, I missed that. Let me know if the discussion ends up concluding that we should revert this change for 6.0. thanks -- PMM
The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241: qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200) ---------------------------------------------------------------- * Fix C++ compilation of qemu/osdep.h. * Fix -object cryptodev-vhost-user ---------------------------------------------------------------- Paolo Bonzini (2): osdep: include glib-compat.h before other QEMU headers osdep: protect qemu/osdep.h with extern "C" Thomas Huth (1): qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code disas/nanomips.cpp | 2 +- include/qemu/compiler.h | 6 ++++++ include/qemu/osdep.h | 13 +++++++++++-- qapi/qom.json | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-)