On 25/03/2015 10:36, Wen Congyang wrote: > > +void backup_do_checkpoint(BlockJob *job, Error **errp) > +{ > + BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); > + > + if (job->driver != &backup_job_driver) { > + error_setg(errp, "It is not backup job"); > + return; > + } > + > + hbitmap_reset_all(backup_job->bitmap); > +}
Please add instead a block_job_do_checkpoint API, and a do_checkpoint function pointer to BlockJobDriver. > +{ > +#if 0 > + hbitmap_reset(hb, 0, hb->size << hb->granularity); > +#else > + uint64_t size = hb->size; > + unsigned int i; > + > + /* Same as hbitmap_alloc() except memset() */ > + for (i = HBITMAP_LEVELS; i-- > 0; ) { This can be "--i >= 1"... > + size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1); > + memset(hb->levels[i], 0, size * sizeof(unsigned long)); > + } > + > + assert(size == 1); > + hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1); ... if you use "=" instead of "|=" here. > +#endif > +} Please pick one implementation (no #if), and also add a testcase to tests/test-hbitmap.c. Paolo