17.09.2020 19:35, Alberto Garcia wrote:
On Wed 09 Sep 2020 08:59:26 PM CEST, Vladimir Sementsov-Ogievskiy
<vsement...@virtuozzo.com> wrote:
-/* qcow2_load_dirty_bitmaps()
- * Return value is a hint for caller: true means that the Qcow2 header was
- * updated. (false doesn't mean that the header should be updated by the
- * caller, it just means that updating was not needed or the image cannot be
- * written to).
- * On failure the function returns false.
- */
-bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+/* Return true on success, false on failure. */
+bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated,
+ Error **errp)
I think that the documentation should clarify under what conditions
'header_updated' is modified.
if (s->nb_bitmaps == 0) {
/* No bitmaps - nothing to do */
- return false;
+ return true;
}
Here is it not for example (should it be set to false?).
Ha, I think, it just shows that patch is wrong :) We should set header_updated
at least on every success path. Or better always (if it is non-NULL of course).
Thanks for careful review!
- if (bm_list == NULL) {
+ if (!bm_list) {
return false;
}
This looks like a cosmetic change unrelated to the rest of the patch.
Berto
--
Best regards,
Vladimir
