On 11/12/2013 03:46 AM, Kevin Wolf wrote: >> +++ b/block/qapi.c >> @@ -204,14 +204,6 @@ void bdrv_query_info(BlockDriverState *bs, >> info->io_status = bs->iostatus; >> } >> >> - if (bs->dirty_bitmap) { >> - info->has_dirty = true; >> - info->dirty = g_malloc0(sizeof(*info->dirty)); >> - info->dirty->count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE; >> - info->dirty->granularity = >> - ((int64_t) BDRV_SECTOR_SIZE << >> hbitmap_granularity(bs->dirty_bitmap)); >> - } >> - >> if (bs->drv) { >> info->has_inserted = true; >> info->inserted = g_malloc0(sizeof(*info->inserted)); > > The dirty field should probably be removed from qapi-schema.json if it > never gets set any more. > > It was optional, so perhaps removing it doesn't cause any trouble > indeed, but I'd like to hear Eric on this matter before merging the > patch. Though if libvirt does make use of it, we have a problem because > it doesn't really make sense any more after these changes.
Removing an optional output-only field is okay (it is the same as stating that we are ALWAYS taking the option of not including it). Since it has always been marked optional, management can't be relying on it; more specifically, libvirt does not have any code checking for it. So I'm okay with removing it from the schema as a backwards-compatible change. [Removing an optional input field is wrong, but BlockInfo is an output type, not an input type.] Eventually, libvirt would like to use persistent dirty bitmaps, in order to resume long-running operations such as drive-mirror even across migration between different qemu processes, but I don't think we are quite there yet, and I also don't think that removal of 'dirty' from BlockInfo impedes in that goal. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature