On 01/14/2015 07:26 AM, Vladimir Sementsov-Ogievskiy wrote:
On 09.01.2015 00:23, John Snow wrote:
On 12/11/2014 09:17 AM, Vladimir Sementsov-Ogievskiy wrote:
Add blk_create and blk_free to remove code duplicates. Otherwise,
duplicates will rise in the following patches because of BlkMigBlock
sturcture extendin.
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
block-migration.c | 56
+++++++++++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 5b4aa0f..d0c825f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -113,6 +113,30 @@ static void blk_mig_unlock(void)
qemu_mutex_unlock(&block_mig_state.lock);
}
+/* Only allocating and initializing structure fields, not copying
any data. */
+
+static BlkMigBlock *blk_create(BlkMigDevState *bmds, int64_t sector,
+ int nr_sectors)
+{
+ BlkMigBlock *blk = g_new(BlkMigBlock, 1);
+ blk->buf = g_malloc(BLOCK_SIZE);
+ blk->bmds = bmds;
+ blk->sector = sector;
+ blk->nr_sectors = nr_sectors;
+
+ blk->iov.iov_base = blk->buf;
+ blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
+ qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
+
+ return blk;
+}
+
+static void blk_free(BlkMigBlock *blk)
+{
+ g_free(blk->buf);
+ g_free(blk);
+}
+
/* Must run outside of the iothread lock during the bulk phase,
* or the VM will stall.
*/
@@ -285,15 +309,7 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
nr_sectors = total_sectors - cur_sector;
}
- blk = g_new(BlkMigBlock, 1);
- blk->buf = g_malloc(BLOCK_SIZE);
- blk->bmds = bmds;
- blk->sector = cur_sector;
- blk->nr_sectors = nr_sectors;
-
- blk->iov.iov_base = blk->buf;
- blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
- qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
+ blk = blk_create(bmds, cur_sector, nr_sectors);
blk_mig_lock();
block_mig_state.submitted++;
@@ -467,17 +483,9 @@ static int mig_save_device_dirty(QEMUFile *f,
BlkMigDevState *bmds,
} else {
nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK;
}
- blk = g_new(BlkMigBlock, 1);
- blk->buf = g_malloc(BLOCK_SIZE);
- blk->bmds = bmds;
- blk->sector = sector;
- blk->nr_sectors = nr_sectors;
+ blk = blk_create(bmds, sector, nr_sectors);
if (is_async) {
- blk->iov.iov_base = blk->buf;
- blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
- qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
-
I suppose in the (!is_async) branch we don't reference iov/qiov again,
but the functional difference caught my eye. It used to only be called
under the "is_async" branch, but now is going to be executed
unconditionally.
Is that fine?
It think it doesn't matter. I can add a parameter 'is_async' to
blk_create(), but what is worse - excess parameter or excess
initialization? And why not to initialize the whole structure in
blk_create() unconditionally?
If it's not a problem, leave it as-is. If I am not sure immediately
myself, I like to ask questions.
Your answer to the question can always be "Yes, that's fine!"
