Re: [Qemu-devel] [PATCH v2 1/3] block: add basic backup support to block driver

[Stefan Hajnoczi]

On Thu, May 30, 2013 at 5:37 AM, Fam Zheng <address@hidden> wrote:
> On Mon, 04/29 09:42, Stefan Hajnoczi wrote:
>> +
>> +static void coroutine_fn backup_run(void *opaque)
>> +{
>> +    BackupBlockJob *job = opaque;
>> +    BlockDriverState *bs = job->common.bs;
>> +    assert(bs);
>> +
>> +    int64_t start, end;
>> +
>> +    start = 0;
>> +    end = DIV_ROUND_UP(bdrv_getlength(bs) / BDRV_SECTOR_SIZE,
>> +                       BACKUP_BLOCKS_PER_CLUSTER);
>> +
>> +    job->bitmap = hbitmap_alloc(end, 0);
>> +
>> +    DPRINTF("backup_run start %s %" PRId64 " %" PRId64 "\n",
>> +            bdrv_get_device_name(bs), start, end);
>> +
>> +    int ret = 0;
>> +
>> +    for (; start < end; start++) {
>> +        if (block_job_is_cancelled(&job->common)) {
>> +            ret = -1;
>> +            break;
>> +        }
>> +
>> +        /* we need to yield so that qemu_aio_flush() returns.
>> +         * (without, VM does not reboot)
>> +         */
>> +        if (job->common.speed) {
>> +            uint64_t delay_ns = ratelimit_calculate_delay(
>> +                &job->limit, job->sectors_read);
>> +            job->sectors_read = 0;
>> +            block_job_sleep_ns(&job->common, rt_clock, delay_ns);
>> +        } else {
>> +            block_job_sleep_ns(&job->common, rt_clock, 0);
>> +        }
>> +
>> +        if (block_job_is_cancelled(&job->common)) {
>> +            ret = -1;
>> +            break;
>> +        }
>> +
>> +        if (hbitmap_get(job->bitmap, start)) {
>> +            continue; /* already copied */
>> +        }
>> +
>> +        DPRINTF("backup_run loop C%" PRId64 "\n", start);
>> +
>> +        /**
>> +         * This triggers a cluster copy
>> +         * Note: avoid direct call to brdv_co_backup_cow, because
>> +         * this does not call tracked_request_begin()
>> +         */
>> +        ret = bdrv_co_backup(bs, start*BACKUP_BLOCKS_PER_CLUSTER, 1);
>> +        if (ret < 0) {
>> +            break;
>> +        }
>> +        /* Publish progress */
>> +        job->common.offset += BACKUP_CLUSTER_SIZE;
>> +    }
>> +
>> +    /* wait until pending backup_do_cow()calls have completed */
>> +    qemu_co_rwlock_wrlock(&job->rwlock);
>> +    qemu_co_rwlock_unlock(&job->rwlock);
>> +
>> +    hbitmap_free(job->bitmap);
>> +
>> +    bdrv_delete(job->target);
>> +
>> +    DPRINTF("backup_run complete %d\n", ret);
>> +    block_job_completed(&job->common, ret);
>> +}
>
> What will ret value be when the source block is not aligned to cluster
> size?

Good catch, will fix.

Stefan