1. Patchew
  2. Libvirt
  3. View series

[Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field

John Snow posted 10 patches 6 years, 11 months ago
Failed in applying to current master (apply log)
block/dirty-bitmap.c           | 111 ++++++++++++++++++---------------
blockdev.c                     |  19 +++---
include/block/dirty-bitmap.h   |   7 +--
migration/block-dirty-bitmap.c |   8 +--
nbd/server.c                   |  14 ++---
qapi/block-core.json           |  10 ++-
qemu-deprecated.texi           |   6 ++
tests/qemu-iotests/124         | 110 ++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out     |   4 +-
tests/qemu-iotests/236.out     |  28 +++++++++
10 files changed, 239 insertions(+), 78 deletions(-)
[Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field
Posted by John Snow 6 years, 11 months ago 
The current internal meanings of "locked", "user_locked",
"qmp_locked", "frozen", "enabled", and "disabled" are all
a little muddled.

Deprecate the @status field in favor of two new booleans
that carry very specific meanings. Then, rename and rework
some of the internal semantics to help make the API a bit
more clear and easier to read.

Well, in my opinion.

Based on my current bitmaps branch.

V3:

[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/10:[0002] [FC] 'block/dirty-bitmap: add recording and busy properties'
002/10:[0002] [FC] 'block/dirty-bitmaps: rename frozen predicate helper'
003/10:[down] 'block/dirty-bitmap: remove set/reset assertions against enabled bit'
004/10:[0006] [FC] 'block/dirty-bitmap: change semantics of enabled predicate'
005/10:[down] 'nbd: change error checking order for bitmaps'
006/10:[0002] [FC] 'block/dirty-bitmap: explicitly lock bitmaps with successors'
007/10:[0011] [FC] 'block/dirty-bitmaps: unify qmp_locked and user_locked calls'
008/10:[0002] [FC] 'block/dirty-bitmaps: move comment block'
009/10:[down] 'blockdev: remove unused paio parameter documentation'
010/10:[down] 'iotests: add busy/recording bit test to 124'

V3: (Following Vladimir's feedback)

001: Changed texi phrasing, parameter --> field
002: Commit message adjustment
     comment edit: "not locked" --> "not user_locked"
003: pulled forward out of patch 004
004: Commit message rewrite
     create_successor and abdicate doc adjustments
005: New.
006: Change successor doc comment
     Commit message adjustment
007: BdrvDirtyBitmap struct comment adjustments
     Comment change to create_successor (lock --> busy)
     Incidental changes from 004's changes
008: Grammar fix (Eric)
009: New, trivial, unrelated fix tagging along.
010: iotest 124 added.

John Snow (10):
  block/dirty-bitmap: add recording and busy properties
  block/dirty-bitmaps: rename frozen predicate helper
  block/dirty-bitmap: remove set/reset assertions against enabled bit
  block/dirty-bitmap: change semantics of enabled predicate
  nbd: change error checking order for bitmaps
  block/dirty-bitmap: explicitly lock bitmaps with successors
  block/dirty-bitmaps: unify qmp_locked and user_locked calls
  block/dirty-bitmaps: move comment block
  blockdev: remove unused paio parameter documentation
  iotests: add busy/recording bit test to 124

 block/dirty-bitmap.c           | 111 ++++++++++++++++++---------------
 blockdev.c                     |  19 +++---
 include/block/dirty-bitmap.h   |   7 +--
 migration/block-dirty-bitmap.c |   8 +--
 nbd/server.c                   |  14 ++---
 qapi/block-core.json           |  10 ++-
 qemu-deprecated.texi           |   6 ++
 tests/qemu-iotests/124         | 110 ++++++++++++++++++++++++++++++++
 tests/qemu-iotests/124.out     |   4 +-
 tests/qemu-iotests/236.out     |  28 +++++++++
 10 files changed, 239 insertions(+), 78 deletions(-)

-- 
2.17.2
Re: [libvirt] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field
Posted by John Snow 6 years, 11 months ago 

On 2/22/19 7:06 PM, John Snow wrote:
> The current internal meanings of "locked", "user_locked",
> "qmp_locked", "frozen", "enabled", and "disabled" are all
> a little muddled.
> 
> Deprecate the @status field in favor of two new booleans
> that carry very specific meanings. Then, rename and rework
> some of the internal semantics to help make the API a bit
> more clear and easier to read.
> 
> Well, in my opinion.
> 
> Based on my current bitmaps branch.
> 
> V3:
> 
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/10:[0002] [FC] 'block/dirty-bitmap: add recording and busy properties'
> 002/10:[0002] [FC] 'block/dirty-bitmaps: rename frozen predicate helper'
> 003/10:[down] 'block/dirty-bitmap: remove set/reset assertions against enabled bit'
> 004/10:[0006] [FC] 'block/dirty-bitmap: change semantics of enabled predicate'
> 005/10:[down] 'nbd: change error checking order for bitmaps'
> 006/10:[0002] [FC] 'block/dirty-bitmap: explicitly lock bitmaps with successors'
> 007/10:[0011] [FC] 'block/dirty-bitmaps: unify qmp_locked and user_locked calls'
> 008/10:[0002] [FC] 'block/dirty-bitmaps: move comment block'
> 009/10:[down] 'blockdev: remove unused paio parameter documentation'
> 010/10:[down] 'iotests: add busy/recording bit test to 124'
> 
> V3: (Following Vladimir's feedback)
> 
> 001: Changed texi phrasing, parameter --> field
> 002: Commit message adjustment
>      comment edit: "not locked" --> "not user_locked"
> 003: pulled forward out of patch 004
> 004: Commit message rewrite
>      create_successor and abdicate doc adjustments
> 005: New.
> 006: Change successor doc comment
>      Commit message adjustment
> 007: BdrvDirtyBitmap struct comment adjustments
>      Comment change to create_successor (lock --> busy)
>      Incidental changes from 004's changes
> 008: Grammar fix (Eric)
> 009: New, trivial, unrelated fix tagging along.
> 010: iotest 124 added.
> 
> John Snow (10):
>   block/dirty-bitmap: add recording and busy properties
>   block/dirty-bitmaps: rename frozen predicate helper
>   block/dirty-bitmap: remove set/reset assertions against enabled bit
>   block/dirty-bitmap: change semantics of enabled predicate
>   nbd: change error checking order for bitmaps
>   block/dirty-bitmap: explicitly lock bitmaps with successors
>   block/dirty-bitmaps: unify qmp_locked and user_locked calls
>   block/dirty-bitmaps: move comment block
>   blockdev: remove unused paio parameter documentation
>   iotests: add busy/recording bit test to 124
> 
>  block/dirty-bitmap.c           | 111 ++++++++++++++++++---------------
>  blockdev.c                     |  19 +++---
>  include/block/dirty-bitmap.h   |   7 +--
>  migration/block-dirty-bitmap.c |   8 +--
>  nbd/server.c                   |  14 ++---
>  qapi/block-core.json           |  10 ++-
>  qemu-deprecated.texi           |   6 ++
>  tests/qemu-iotests/124         | 110 ++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/124.out     |   4 +-
>  tests/qemu-iotests/236.out     |  28 +++++++++
>  10 files changed, 239 insertions(+), 78 deletions(-)
> 

Pushed to my bitmaps branch with the following minor changes:

001/10:[0002] [FC] 'block/dirty-bitmap: add recording and busy properties'
002/10:[0002] [FC] 'block/dirty-bitmaps: rename frozen predicate helper'
003/10:[----] [--] 'block/dirty-bitmap: remove set/reset assertions
against enabled bit'
004/10:[----] [--] 'block/dirty-bitmap: change semantics of enabled
predicate'
005/10:[----] [--] 'nbd: change error checking order for bitmaps'
006/10:[----] [--] 'block/dirty-bitmap: explicitly lock bitmaps with
successors'
007/10:[0002] [FC] 'block/dirty-bitmaps: unify qmp_locked and
user_locked calls'
008/10:[----] [--] 'block/dirty-bitmaps: move comment block'
009/10:[----] [--] 'blockdev: remove unused paio parameter documentation'
010/10:[0003] [FC] 'iotests: add busy/recording bit test to 124'


001: Texi doc spelling (Eric)
002: Commit message spelling (Eric)
     Additional asserts (Vladimir)
007: Comment phrasing (Vladimir)
010: Extra test comment

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field
Posted by no-reply@patchew.org 6 years, 11 months ago 
Patchew URL: https://patchew.org/QEMU/20190223000614.13894-1-jsnow@redhat.com/



Hi,

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

Message-id: 20190223000614.13894-1-jsnow@redhat.com
Subject: [Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field
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
>From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190220160628.6555-1-marcandre.lureau@redhat.com -> patchew/20190220160628.6555-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
5615f81566 iotests: add busy/recording bit test to 124
7c4f722b89 blockdev: remove unused paio parameter documentation
30e0427368 block/dirty-bitmaps: move comment block
a29ece1728 block/dirty-bitmaps: unify qmp_locked and user_locked calls
3d0665404c block/dirty-bitmap: explicitly lock bitmaps with successors
d932d4e9c4 nbd: change error checking order for bitmaps
2ee1f03bae block/dirty-bitmap: change semantics of enabled predicate
4fca6cb44e block/dirty-bitmap: remove set/reset assertions against enabled bit
7d032c41b8 block/dirty-bitmaps: rename frozen predicate helper
9d17322444 block/dirty-bitmap: add recording and busy properties

=== OUTPUT BEGIN ===
1/10 Checking commit 9d1732244435 (block/dirty-bitmap: add recording and busy properties)
2/10 Checking commit 7d032c41b8ab (block/dirty-bitmaps: rename frozen predicate helper)
WARNING: line over 80 characters
#85: FILE: block/dirty-bitmap.c:248:
+        error_setg(errp, "Cannot create a successor for a bitmap that is in-use "

total: 0 errors, 1 warnings, 122 lines checked

Patch 2/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/10 Checking commit 4fca6cb44e9b (block/dirty-bitmap: remove set/reset assertions against enabled bit)
4/10 Checking commit 2ee1f03baedb (block/dirty-bitmap: change semantics of enabled predicate)
5/10 Checking commit d932d4e9c476 (nbd: change error checking order for bitmaps)
6/10 Checking commit 3d0665404c60 (block/dirty-bitmap: explicitly lock bitmaps with successors)
7/10 Checking commit a29ece1728f0 (block/dirty-bitmaps: unify qmp_locked and user_locked calls)
ERROR: open brace '{' following function declarations go on the next line
#37: FILE: block/dirty-bitmap.c:190:
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {

total: 1 errors, 0 warnings, 263 lines checked

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

8/10 Checking commit 30e042736827 (block/dirty-bitmaps: move comment block)
9/10 Checking commit 7c4f722b8967 (blockdev: remove unused paio parameter documentation)
10/10 Checking commit 5615f815663d (iotests: add busy/recording bit test to 124)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190223000614.13894-1-jsnow@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.