* Juan Quintela (quint...@redhat.com) wrote:
> > +/* **** functions for postcopy ***** */
> > +
> > +/*
> > + * Callback from postcopy_each_ram_send_discard for each RAMBlock
> > + * start,end: Indexes into the bitmap for the first and last bit
> > + * representing the named block
> > + */
> > +static int postcopy_send_discard_bm_ram(MigrationState *ms,
> > + PostcopyDiscardState *pds,
> > + unsigned long start, unsigned long
> > end)
> > +{
> > + unsigned long current;
> > +
> > + for (current = start; current <= end; ) {
> > + unsigned long set = find_next_bit(ms->sentmap, end + 1, current);
> > +
> > + if (set <= end) {
> > + unsigned long zero = find_next_zero_bit(ms->sentmap,
> > + end + 1, set + 1);
> > +
> > + if (zero > end) {
> > + zero = end + 1;
> > + }
> > + postcopy_discard_send_range(ms, pds, set, zero - 1);
> > + current = zero + 1;
> > + } else {
> > + current = set;
> > + }
> > + }
>
> I think I undrestood the logic here at the end....
>
> But if we change the meaning of postcopy_discard_send_range() from
> (begin, end), to (begin, length), I think everything goes clearer, no?
>
> if (set <= end) {
> unsigned long zero = find_next_zero_bit(ms->sentmap,
> end + 1, set + 1);
> unsigned long length;
>
> if (zero > end) {
> length = end - set;
> } else {
> lenght = zero - 1 - set;
> current = zero + 1;
> }
> postcopy_discard_send_range(ms, pds, set, len);
> } else {
> current = set;
> }
> }
>
> Y would clame that if we call one zero, the other would be called one.
> Or change to set/unset, but that is just me. Yes, I haven't tested, and
> it is possible that there is a off-by-one somewhere...
>
> Looking at postocpy_eand_ram_send_discard, I even think that it would be
> a good idea to pass length to this function.
OK, so I've ended up with (build tested only so far):
/*
* Callback from postcopy_each_ram_send_discard for each RAMBlock
* Note: At this point the 'unsentmap' is the processed bitmap combined
* with the dirtymap; so a '1' means it's either dirty or unsent.
* start,length: Indexes into the bitmap for the first bit
* representing the named block and length in target-pages
*/
static int postcopy_send_discard_bm_ram(MigrationState *ms,
PostcopyDiscardState *pds,
unsigned long start,
unsigned long length)
{
unsigned long end = start + length; /* one after the end */
unsigned long current;
for (current = start; current < end; ) {
unsigned long one = find_next_bit(ms->unsentmap, end, current);
if (one <= end) {
unsigned long zero = find_next_zero_bit(ms->unsentmap,
end, one + 1);
unsigned long discard_length;
if (zero >= end) {
discard_length = end - one;
} else {
discard_length = zero - one;
}
postcopy_discard_send_range(ms, pds, one, discard_length);
current = one + discard_length;
} else {
current = one;
}
}
return 0;
}
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
