Re: [Qemu-devel] [PATCH 2/5] add basic backup support to block driver

2012-11-23 Thread Dietmar Maurer
Is there a 1:1 relationship between BackupInfo and BackupBlockJob? Then it would be nicer to move BlockupInfo fields into BackupBlockJob (which is empty right now). Then you also don't need to add fields to BlockDriverState because you know that if your blockjob is running you can access it

Re: [Qemu-devel] [PATCH 2/5] add basic backup support to block driver

2012-11-22 Thread Stefan Hajnoczi
On Wed, Nov 21, 2012 at 10:01:01AM +0100, Dietmar Maurer wrote: Two comments from skimming the code, not a full review. +/* #define DEBUG_BACKUP */ + +#ifdef DEBUG_BACKUP +#define DPRINTF(fmt, ...) \ +do { printf(backup: fmt, ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt,

Re: [Qemu-devel] [PATCH 2/5] add basic backup support to block driver

2012-11-22 Thread Dietmar Maurer
Is there a 1:1 relationship between BackupInfo and BackupBlockJob? Then it would be nicer to move BlockupInfo fields into BackupBlockJob (which is empty right now No, a backup create several block jobs - one for each blockdev you want to backup. Those jobs run in parallel.

[Qemu-devel] [PATCH 2/5] add basic backup support to block driver

2012-11-21 Thread Dietmar Maurer
Function bdrv_backup_init() creates a block job to backup a block device. We call brdv_co_backup_cow() for each write during backup. That function reads the original data and pass it to backup_dump_cb(). The tracked_request infrastructure is used to serialize access. Currently backup cluster