The following changes since commit 7d48cf8102a10e4a54333811bafb5eb566509268:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-08-01 14:33:56 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 33f21e4f044ac1c37f60edc1f1aee628be8f463b:

  mc146818rtc: implement UIP latching as intended (2017-08-01 17:27:34 +0200)

----------------------------------------------------------------
* Xen fix (Anthony)
* chardev fixes (Anton, Marc-André)
* small dead code removal (Zhongyi)
* documentation (Dan)
* bugfixes (David)
* decrease migration downtime (Jay)
* improved error output (Laurent)
* RTC tests and bugfix (me)
* Bluetooth clang analyzer fix (me)
* KVM CPU hotplug race (Peng Hao)
* Two other patches from Philippe's clang analyzer series

----------------------------------------------------------------
Anthony PERARD (1):
      exec: Add lock parameter to qemu_ram_ptr_length

Anton Nefedov (1):
      char: don't exit on hmp 'chardev-add help'

Daniel P. Berrange (2):
      docs: document deprecation policy & deprecated features in appendix
      qemu-options: document existance of versioned machine types

Dr. David Alan Gilbert (2):
      vl.c/exit: pause cpus before closing block devices
      cpu_physical_memory_sync_dirty_bitmap: Fix alignment check

Jay Zhou (1):
      migration: optimize the downtime

Laurent Vivier (1):
      accel: cleanup error output

Mao Zhongyi (2):
      hw/scsi/vmw_pvscsi: Remove the dead error handling
      hw/scsi/vmw_pvscsi: Convert to realize

Marc-André Lureau (1):
      char-fd: remove useless chr pointer

Paolo Bonzini (5):
      bt: stop the sdp memory allocation craziness
      rtc-test: cleanup register_b_set_flag test
      rtc-test: introduce more update tests
      mc146818rtc: simplify check_update_timer
      mc146818rtc: implement UIP latching as intended

Peng Hao (1):
      target-i386: kvm_get/put_vcpu_events don't handle sipi_vector

 accel/accel.c             |  20 +++---
 chardev/char-fd.c         |   1 -
 chardev/char.c            |   2 +-
 exec.c                    |  12 ++--
 hw/bt/sdp.c               |  17 +++--
 hw/scsi/vmw_pvscsi.c      |  12 +---
 hw/timer/mc146818rtc.c    |  37 +++++-----
 include/chardev/char-fd.h |   2 +-
 include/chardev/char.h    |   4 +-
 include/exec/ram_addr.h   |   7 +-
 memory.c                  |  36 +++++++++-
 qemu-doc.texi             | 175 ++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx           |  15 +++-
 target/i386/kvm.c         |  35 ++++++----
 tests/rtc-test.c          | 156 +++++++++++++++++++++++++++++++++--------
 vl.c                      |  12 ++--
 16 files changed, 437 insertions(+), 106 deletions(-)
-- 
1.8.3.1

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PULL 00/17] Misc changes for QEMU 2.10-rc1 (?)
Message-id: 1501604245-33460-1-git-send-email-pbonzini@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
3489547b43 mc146818rtc: implement UIP latching as intended
ed88cc342f mc146818rtc: simplify check_update_timer
818434f924 rtc-test: introduce more update tests
d2208f1403 rtc-test: cleanup register_b_set_flag test
acd549e69e hw/scsi/vmw_pvscsi: Convert to realize
07345966fe hw/scsi/vmw_pvscsi: Remove the dead error handling
7651c26752 migration: optimize the downtime
c24276b69b qemu-options: document existance of versioned machine types
6240b43885 bt: stop the sdp memory allocation craziness
8e3ab7f33e exec: Add lock parameter to qemu_ram_ptr_length
c2d7f542e9 target-i386: kvm_get/put_vcpu_events don't handle sipi_vector
caaa83b53e docs: document deprecation policy & deprecated features in appendix
e085726add char: don't exit on hmp 'chardev-add help'
b9ba7ecb28 char-fd: remove useless chr pointer
ee56cb9f35 accel: cleanup error output
235bccb31d cpu_physical_memory_sync_dirty_bitmap: Fix alignment check
99135ab7e2 vl.c/exit: pause cpus before closing block devices

=== OUTPUT BEGIN ===
Checking PATCH 1/17: vl.c/exit: pause cpus before closing block devices...
Checking PATCH 2/17: cpu_physical_memory_sync_dirty_bitmap: Fix alignment check...
Checking PATCH 3/17: accel: cleanup error output...
Checking PATCH 4/17: char-fd: remove useless chr pointer...
Checking PATCH 5/17: char: don't exit on hmp 'chardev-add help'...
Checking PATCH 6/17: docs: document deprecation policy & deprecated features in appendix...
Checking PATCH 7/17: target-i386: kvm_get/put_vcpu_events don't handle sipi_vector...
Checking PATCH 8/17: exec: Add lock parameter to qemu_ram_ptr_length...
Checking PATCH 9/17: bt: stop the sdp memory allocation craziness...
ERROR: space prohibited before that '++' (ctx:WxB)
#78: FILE: hw/bt/sdp.c:741:
+        data[len ++] = attribute_id >> 8;
                  ^

ERROR: space prohibited before that '++' (ctx:WxB)
#79: FILE: hw/bt/sdp.c:742:
+        data[len ++] = attribute_id & 0xff;
                  ^

total: 2 errors, 0 warnings, 48 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/17: qemu-options: document existance of versioned machine types...
Checking PATCH 11/17: migration: optimize the downtime...
Checking PATCH 12/17: hw/scsi/vmw_pvscsi: Remove the dead error handling...
Checking PATCH 13/17: hw/scsi/vmw_pvscsi: Convert to realize...
Checking PATCH 14/17: rtc-test: cleanup register_b_set_flag test...
ERROR: space required before the open parenthesis '('
#73: FILE: tests/rtc-test.c:344:
+    } while(0)

total: 1 errors, 0 warnings, 115 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 15/17: rtc-test: introduce more update tests...
Checking PATCH 16/17: mc146818rtc: simplify check_update_timer...
Checking PATCH 17/17: mc146818rtc: implement UIP latching as intended...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

On 01/08/2017 18:48, no-reply@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/17: vl.c/exit: pause cpus before closing block devices...
> Checking PATCH 2/17: cpu_physical_memory_sync_dirty_bitmap: Fix alignment check...
> Checking PATCH 3/17: accel: cleanup error output...
> Checking PATCH 4/17: char-fd: remove useless chr pointer...
> Checking PATCH 5/17: char: don't exit on hmp 'chardev-add help'...
> Checking PATCH 6/17: docs: document deprecation policy & deprecated features in appendix...
> Checking PATCH 7/17: target-i386: kvm_get/put_vcpu_events don't handle sipi_vector...
> Checking PATCH 8/17: exec: Add lock parameter to qemu_ram_ptr_length...
> Checking PATCH 9/17: bt: stop the sdp memory allocation craziness...
> ERROR: space prohibited before that '++' (ctx:WxB)
> #78: FILE: hw/bt/sdp.c:741:
> +        data[len ++] = attribute_id >> 8;
>                   ^
> 
> ERROR: space prohibited before that '++' (ctx:WxB)
> #79: FILE: hw/bt/sdp.c:742:
> +        data[len ++] = attribute_id & 0xff;

This is the preexisting Bluetooth code... I didn't change the space,
should I have done that?

> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 10/17: qemu-options: document existance of versioned machine types...
> Checking PATCH 11/17: migration: optimize the downtime...
> Checking PATCH 12/17: hw/scsi/vmw_pvscsi: Remove the dead error handling...
> Checking PATCH 13/17: hw/scsi/vmw_pvscsi: Convert to realize...
> Checking PATCH 14/17: rtc-test: cleanup register_b_set_flag test...
> ERROR: space required before the open parenthesis '('
> #73: FILE: tests/rtc-test.c:344:
> +    } while(0)

This seems to be more common than "while (0)" inside macros, should we
allow it in checkpatch.pl?

Paolo

> total: 1 errors, 0 warnings, 115 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 15/17: rtc-test: introduce more update tests...
> Checking PATCH 16/17: mc146818rtc: simplify check_update_timer...
> Checking PATCH 17/17: mc146818rtc: implement UIP latching as intended...
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org
> 

On 1 August 2017 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/08/2017 18:48, no-reply@patchew.org wrote:
>> ERROR: space prohibited before that '++' (ctx:WxB)
>> #78: FILE: hw/bt/sdp.c:741:
>> +        data[len ++] = attribute_id >> 8;
>>                   ^
>>
>> ERROR: space prohibited before that '++' (ctx:WxB)
>> #79: FILE: hw/bt/sdp.c:742:
>> +        data[len ++] = attribute_id & 0xff;
>
> This is the preexisting Bluetooth code... I didn't change the space,
> should I have done that?

Judgement call -- I usually fix up existing errors if I'm touching
a bit of code anyway, unless it's a whitespace-only change or a
pure code-motion patch.

>> ERROR: space required before the open parenthesis '('
>> #73: FILE: tests/rtc-test.c:344:
>> +    } while(0)
>
> This seems to be more common than "while (0)" inside macros, should we
> allow it in checkpatch.pl?

Overall the space is much more common: 551 examples with the
space vs 90 without; so I don't think a relaxation of checkpatch
is particularly justified. I don't think macros need to be any
different from the rest of our code on things like spacing.

thanks
-- PMM

On 01/08/2017 19:10, Peter Maydell wrote:
> On 1 August 2017 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 01/08/2017 18:48, no-reply@patchew.org wrote:
>>> ERROR: space prohibited before that '++' (ctx:WxB)
>>> #78: FILE: hw/bt/sdp.c:741:
>>> +        data[len ++] = attribute_id >> 8;
>>>                   ^
>>>
>>> ERROR: space prohibited before that '++' (ctx:WxB)
>>> #79: FILE: hw/bt/sdp.c:742:
>>> +        data[len ++] = attribute_id & 0xff;
>>
>> This is the preexisting Bluetooth code... I didn't change the space,
>> should I have done that?
> 
> Judgement call -- I usually fix up existing errors if I'm touching
> a bit of code anyway, unless it's a whitespace-only change or a
> pure code-motion patch.

Me too.  In this case however the code is pretty much untouched so it's
unlikely that it would become consistent one day (and I suspect no one
wants to get on git blame for bluetooth emulation :)).

>>> ERROR: space required before the open parenthesis '('
>>> #73: FILE: tests/rtc-test.c:344:
>>> +    } while(0)
>>
>> This seems to be more common than "while (0)" inside macros, should we
>> allow it in checkpatch.pl?
> 
> Overall the space is much more common: 551 examples with the
> space vs 90 without; so I don't think a relaxation of checkpatch
> is particularly justified. I don't think macros need to be any
> different from the rest of our code on things like spacing.

Ok, for this patch I kept it consistent within the file, but for 2.11 we
can change everything to "while (0)".

Paolo

On 1 August 2017 at 18:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Ok, for this patch I kept it consistent within the file, but for 2.11 we
> can change everything to "while (0)".

Is this a proposal to bite the bullet and fix all our
code style issues in existing code? :-)

It would be an interesting question to find out what % of
QEMU's lines of code have not been touched in the six
years since we added checkpatch, and how they're
distributed in the codebase (are there any neat
visualisations of this sort of thing?) Fix-when-touched
works OK where we're actively working but can leave
some big dead spots I suspect.

thanks
-- PMM

On 01/08/2017 19:22, Peter Maydell wrote:
> On 1 August 2017 at 18:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Ok, for this patch I kept it consistent within the file, but for 2.11 we
>> can change everything to "while (0)".
> 
> Is this a proposal to bite the bullet and fix all our
> code style issues in existing code? :-)

Only "while (0)" :)

> It would be an interesting question to find out what % of
> QEMU's lines of code have not been touched in the six
> years since we added checkpatch, and how they're
> distributed in the codebase (are there any neat
> visualisations of this sort of thing?) Fix-when-touched
> works OK where we're actively working but can leave
> some big dead spots I suspect.

That's an interesting idea.  Since I've been volunteered to present
about QEMU at KVM Forum, I might give it a shot.

Paolo

On 1 August 2017 at 17:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 7d48cf8102a10e4a54333811bafb5eb566509268:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-08-01 14:33:56 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 33f21e4f044ac1c37f60edc1f1aee628be8f463b:
>
>   mc146818rtc: implement UIP latching as intended (2017-08-01 17:27:34 +0200)
>
> ----------------------------------------------------------------
> * Xen fix (Anthony)
> * chardev fixes (Anton, Marc-André)
> * small dead code removal (Zhongyi)
> * documentation (Dan)
> * bugfixes (David)
> * decrease migration downtime (Jay)
> * improved error output (Laurent)
> * RTC tests and bugfix (me)
> * Bluetooth clang analyzer fix (me)
> * KVM CPU hotplug race (Peng Hao)
> * Two other patches from Philippe's clang analyzer series
>

Applied, thanks, but can you check whatever it is in your
workflow that's mangling non-ascii characters?

thanks
-- PMM

> > ----------------------------------------------------------------
> > * Xen fix (Anthony)
> > * chardev fixes (Anton, Marc-André)
> > * small dead code removal (Zhongyi)
> > * documentation (Dan)
> > * bugfixes (David)
> > * decrease migration downtime (Jay)
> > * improved error output (Laurent)
> > * RTC tests and bugfix (me)
> > * Bluetooth clang analyzer fix (me)
> > * KVM CPU hotplug race (Peng Hao)
> > * Two other patches from Philippe's clang analyzer series
> >
> 
> Applied, thanks, but can you check whatever it is in your
> workflow that's mangling non-ascii characters?

It's https://bugzilla.mozilla.org/show_bug.cgi?id=1017768.  Usually
I apply patches with patchew, which bypasses that bug.

Paolo