On 19.08.21 13:06, Jan Beulich wrote:
> On 19.08.2021 12:20, Juergen Gross wrote:
>> On 05.07.21 17:13, Jan Beulich wrote:
>>> In send_memory_live() the precise value the dirty_count struct field
>>> gets initialized to doesn't matter much (apart from the triggering of
>>> the log message in send_dirty_pages(), see below), but it is important
>>> that it not be zero on the first iteration (or else send_dirty_pages()
>>> won't get called at all). Saturate the initializer value at the maximum
>>> value the field can hold.
>>>
>>> While there also initialize struct precopy_stats' respective field to a
>>> more sane value: We don't really know how many dirty pages there are at
>>> that point.
>>>
>>> In suspend_and_send_dirty() and verify_frames() the local variables
>>> don't need initializing at all, as they're only an output from the
>>> hypercall which gets invoked first thing.
>>>
>>> In send_checkpoint_dirty_pfn_list() the local variable can be dropped
>>> altogether: It's optional to xc_logdirty_control() and not used anywhere
>>> else.
>>>
>>> Note that in case the clipping actually takes effect, the "Bitmap
>>> contained more entries than expected..." log message will trigger. This
>>> being just an informational message, I don't think this is overly
>>> concerning.
>>
>> Is there any real reason why the width of the stats fields can't be
>> expanded to avoid clipping? This could avoid the need to set the
>> initial value to -1, which seems one of the more controversial changes.
> 
> While not impossible, it comes with a price tag, as we'd either need
> to decouple xc_shadow_op_stats_t from struct xen_domctl_shadow_op_stats
> or alter the underlying domctl. Neither of which looked either

I was thinking about the domctl.

Apart of struct xen_sysctl_page_offline_op this seems to be the only
left domctl/sysctl structure limiting guest or host size to values
being relevant. Changing those would be a sensible thing to do IMO.

> appealing or necessary to me; instead I'm still struggling with
> Andrew's comments, yet I didn't receive any clarification of further
> explanation. Plus I continue to think that statistics output like this
> shouldn't be assumed to be precise anyway, and for practical purposes
> I don't think it really matters how large the counts actually are once
> they've moved into the billions.

In case the underlying problem can easily be avoided be changing the
type of the field I don't see why this shouldn't be done. Especially as
in this case a misleading message will just be avoided.


Juergen