[PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

Paolo Bonzini posted 3 patches 3 years ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210413160850.240064-1-pbonzini@redhat.com
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
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(-)
[PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Paolo Bonzini 3 years ago
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(-)
-- 
2.30.1


Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Peter Maydell 3 years ago
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

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Peter Maydell 3 years ago
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

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Peter Maydell 3 years ago
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

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Markus Armbruster 3 years ago
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


Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by Peter Maydell 3 years ago
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

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
Posted by no-reply@patchew.org 3 years ago
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