Re: [Qemu-devel] [PATCH 01/51] ram: Update all functions comments

[Juan Quintela]

"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Peter Xu (address@hidden) wrote:
>> Hi, Juan,
>> 
>> Got several nitpicks below... (along with some questions)
>> 
>> On Thu, Mar 23, 2017 at 09:44:54PM +0100, Juan Quintela wrote:
>> 
>> [...]
>
>> > @@ -1157,11 +1186,12 @@ static bool get_queued_page(MigrationState
>> > *ms, PageSearchStatus *pss,
>> >  }
>> >  
>> >  /**
>> > - * flush_page_queue: Flush any remaining pages in the ram request queue
>> > - *    it should be empty at the end anyway, but in error cases there may 
>> > be
>> > - *    some left.
>> > + * flush_page_queue: flush any remaining pages in the ram request queue
>> 
>> Here the comment says (just like mentioned in function name) that we
>> will "flush any remaining pages in the ram request queue", however in
>> the implementation, we should be only freeing everything in
>> src_page_requests. The problem is "flush" let me think about "flushing
>> the rest of the pages to the other side"... while it's not.
>> 
>> Would it be nice we just rename the function into something else, like
>> migration_page_queue_free()? We can tune the comments correspondingly
>> as well.
>
> Yes that probably would be a better name.

done
>> > - * Allocate data structures etc needed by incoming migration with 
>> > postcopy-ram
>> > - * postcopy-ram's similarly names postcopy_ram_incoming_init does the work
>> > +/**
>> > + * ram_postococpy_incoming_init: allocate postcopy data structures
>> > + *
>> > + * Returns 0 for success and negative if there was one error
>> > + *
>> > + * @mis: current migration incoming state
>> > + *
>> > + * Allocate data structures etc needed by incoming migration with
>> > + * postcopy-ram postcopy-ram's similarly names
>> > + * postcopy_ram_incoming_init does the work
>> 
>> This sentence is slightly hard to understand... But I think the
>> function name explained itself enough though. :)
>
> A '.' after the first 'postcopy-ram' would make it more readable.
>
> Dave

Done.  Once there, I spelled postcopy correctly O:-)