[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver |
Date: |
Tue, 21 May 2013 15:34:28 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, May 20, 2013 at 01:30:37PM +0200, Paolo Bonzini wrote:
> Il 15/05/2013 16:34, Stefan Hajnoczi ha scritto:
> > + wait_for_overlapping_requests(job, start, end);
> > + cow_request_begin(&cow_request, job, start, end);
> > +
> > + for (; start < end; start++) {
> > + if (hbitmap_get(job->bitmap, start)) {
> > + DPRINTF("brdv_co_backup_cow skip C%" PRId64 "\n", start);
> > + continue; /* already copied */
> > + }
> > +
> > + /* immediately set bitmap (avoid coroutine race) */
> > + hbitmap_set(job->bitmap, start, 1);
> > +
>
> HBitmap already has code for finding the next set bit, but you're not
> using it.
>
> If you reverse the direction of the bitmap, you can use an HBitmapIter here:
>
> >
> > + start = 0;
> > + end = DIV_ROUND_UP(bdrv_getlength(bs) / BDRV_SECTOR_SIZE,
> > + BACKUP_BLOCKS_PER_CLUSTER);
> > +
> > + job->bitmap = hbitmap_alloc(end, 0);
> > +
> > + before_write = bdrv_add_before_write_cb(bs, backup_before_write);
> > +
> > + DPRINTF("backup_run start %s %" PRId64 " %" PRId64 "\n",
> > + bdrv_get_device_name(bs), start, end);
> > +
> > + for (; start < end; start++) {
>
> ... instead of iterating through each item manually.
/**
* hbitmap_iter_init:
[...]
* position of the iterator is also okay. However, concurrent resetting of
* bits can lead to unexpected behavior if the iterator has not yet reached
* those bits.
*/
void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
This worries me. We would initialize the bitmap to all 1s. Backing up
a cluster resets the bit. But the documentation says it is not safe to
reset bits while iterating?
Stefan
- [Qemu-devel] [PATCH v3 5/8] blockdev: rename BlkTransactionStates to singular, (continued)
- [Qemu-devel] [PATCH v3 5/8] blockdev: rename BlkTransactionStates to singular, Stefan Hajnoczi, 2013/05/15
- [Qemu-devel] [PATCH v3 3/8] block: add drive-backup QMP command, Stefan Hajnoczi, 2013/05/15
- [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Stefan Hajnoczi, 2013/05/15
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Wenchao Xia, 2013/05/15
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Paolo Bonzini, 2013/05/20
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver,
Stefan Hajnoczi <=
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Paolo Bonzini, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Dietmar Maurer, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Paolo Bonzini, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Dietmar Maurer, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Paolo Bonzini, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Dietmar Maurer, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Dietmar Maurer, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Paolo Bonzini, 2013/05/21
- Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver, Stefan Hajnoczi, 2013/05/22
[Qemu-devel] [PATCH v3 7/8] blockdev: add Abort transaction, Stefan Hajnoczi, 2013/05/15