On Tue, Mar 24, 2009 at 6:27 PM, Yaniv Kamay <ykamay@xxxxxxxxxx> wrote:
> Glauber Costa wrote:
>>
>> @@ -1223,12 +1244,44 @@ static int kvm_get_dirty_bitmap_cb(unsigned
>> long start, unsigned long len,
>> int kvm_update_dirty_pages_log(void)
>> {
>> int r = 0;
>> + ram_addr_t now = 0;
>> + ram_addr_t end = phys_ram_size;
>> + ram_addr_t offset;
>> + target_phys_addr_t area_start;
>> + ram_addr_t area_size;
>> + unsigned char *dirty_bitmap = kvm_dirty_bitmap;
>> +
>> + if (!dirty_bitmap) {
>> + printf("%s: no dirty bitmap\n", __FUNCTION__);
>> + return -1;
>> + }
>> nitpick: you probably want fprintf(stderr...).
>>
>>
>> + while (now < end && !find_phys_area(now, &offset, &area_start,
>> &area_size)) {
>> + if ((offset & ~TARGET_PAGE_MASK) || (area_start &
>> ~TARGET_PAGE_MASK) ||
>> + (area_size &
>> ~TARGET_PAGE_MASK)) {
>> + printf("%s: invalid mem area\n", __FUNCTION__);
>> same here.
>>
>>
>>
>> + if ((now += offset) >= end) {
>> + break;
>> + }
>> +
>> + if (area_size > end - now) {
>> + return -1;
>> + }
>> any reason why you're handling those two differently?
>>
>
> The first is orderly end of mappings and the later is error.
ok. I believe the code reads better if you handle all the errors together.
but this is nitpick too. The idea of the patch looks correct to me.
>>
>> --
>> Glauber Costa.
>> "Free as in Freedom"
>> http://glommer.net
>>
>> "The less confident you are, the more serious you have to act."
>>
>
>
--
Glauber Costa.
"Free as in Freedom"
http://glommer.net
"The less confident you are, the more serious you have to act."
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
