qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 13/21] qcow2: add .bdrv_store_persistent_dirty_b


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH 13/21] qcow2: add .bdrv_store_persistent_dirty_bitmaps()
Date: Mon, 12 Dec 2016 10:32:48 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

10.12.2016 17:53, Max Reitz wrote:
On 09.12.2016 18:55, Vladimir Sementsov-Ogievskiy wrote:
09.12.2016 20:05, Max Reitz wrote:
On 22.11.2016 18:26, Vladimir Sementsov-Ogievskiy wrote:
Realize block bitmap storing interface, to allow qcow2 images store
persistent bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
   block/qcow2-bitmap.c | 451
+++++++++++++++++++++++++++++++++++++++++++++++++++
   block/qcow2.c        |   1 +
   block/qcow2.h        |   1 +
   3 files changed, 453 insertions(+)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 81be1ca..a975388 100644
--- a/block/qcow2-bitmap.c
[...]

+            return;
+        }
+    }
+
+    /* check constraints and names */
+    for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap != NULL;
+            bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) {
Alignment to the opening parenthesis, please.
Hmm.. without an alignment it is not so simple to distinguish for-loop
header from its body.
I know, and it's even worse for "if". That is why I usually put the
opening { on a new line if I have to spread an if/while/for header over
multiple lines.

The usual convention for qemu code is to align at an opening parenthesis
if there is one.

Admittedly, the reasoning I gave for changing checkpatch.pl to accept
opening { on a new line in certain cases was that:

Good news, didn't know)


(1) We never codified exactly what to allow for multi-line if/while/for
     conditions.
(2) It was existing practice.

(1) applies in your case also; we don't have any explicitly written-out
convention for alignment of wrapped lines. (2) is more difficult, but
there are indeed a handful of cases where lines are wrapped and not
aligned to the opening parenthesis but just indented by an additional
four spaces...

So I guess since I'm insisting on putting the opening { on a new line
for multi-line conditions, you are allowed to indent the consecutive
lines by an additional level. ;-)

(It *is* against existing convention, but I'm not in a position to argue.)

[...]

[1] What about bitmaps that have BME_FLAG_IN_USE set but do not have a
corresponding BDS bitmap?

If such a bitmap does not have BME_FLAG_AUTO set, we didn't set the
flag, so we should keep it unchanged. That's what this function is
currently doing.

However, if such a bitmap does have BME_FLAG_AUTO set, it was definitely
us who set the IN_USE flag (because otherwise we would have aborted
loading the bitmaps, and thus also aborted bdrv_open_common()).
Therefore, the only explanation is that the bitmap was deleted in the
meantime, and that means we should also delete it in the qcow2 file.
Right. Or, alternatively, these bitmaps may be deleted on corresponding
BdrvDirtyBitmap deletion.
Right, that would work, too.

Max



--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]