|
From: | Max Reitz |
Subject: | Re: [Qemu-devel] [PATCH] use bdrv_flush to provide barrier semantic in block/vdi.c for metadata updates |
Date: | Wed, 06 May 2015 18:42:00 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
@subject: It's a bit long, and it's
missing a prefix telling what this patch is about. I would have
probably used "block/vdi: Use bdrv_flush after metadata updates"
or something like that.
On 01.05.2015 17:03, phoeagon wrote:
Hm, this doesn't look quite right. :-)
These should be only "---", I guess, so the block below is omitted from the commit message.
This overwrites the return value from bdrv_write(), which I don't think is right. We could either ignore bdrv_flush()'s return value, or make it something like "if (ret < 0) { bdrv_flush(bs->file); } else { ret = bdrv_flush(bs->file); }" or "ret_flush = bdrv_flush(bs->file); if (!(ret < 0)) { ret = ret_flush; }". Or skip the flush in case bdrv_write() failed ("if (ret < 0) { return ret; } ret = bdrv_flush(bs->file);"), like bdrv_pwrite_sync() does. The idea of the change (adding the flush) looks good, though. Max
|
[Prev in Thread] | Current Thread | [Next in Thread] |