Re: Xen 4.5 development update (September update). Feature freeze slip by two weeks.

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 09/11/2014 10:37 AM, Jan Beulich wrote:
>>>> On 10.09.14 at 21:26, <rcojocaru@bitdefender.com> wrote:
>> On 09/10/14 20:05, konrad.wilk@oracle.com wrote:
>>> *  Introspection of HVM guests (ok)
>>>    v6
>>>   -  Razvan Cojocaru
>>
>> Will post a new series tomorrow. Three out of 5 patches seem to be Acked
>> and haven't had any comments in a while (the first three), and I hope to
>> be able to do away completely with patch 4/5 (per-domain page fault
>> injection) for now, and just use the existing HVMOP_inject_trap
>> interface - but this needs a bit more testing to make sure.
> 
> To be honest, despite considering patches 1-3 in acceptable shape
> for being committed from a purely mechanical standpoint, I'm still
> struggling with general reservations towards them: The whole series
> continues to be very focused on special purpose behavior you want
> for your product; recent comments from George emphasize that.
> 
> As I said before, it would be helpful if you adjusted your thinking to
> take general usability of added functionality at least in mind, and if
> you made absolutely certain that code changes you do have no
> adverse effect on other users. This includes you not rushing out
> updates to your series which then you need to supersede within
> hours due to oversights or further adjustments. Take your time to
> think through as many possibilities as you can (namely including
> scenarios that your product does _not_ care about, but others
> may), at once taking a certain level of burden off those
> subsequently reviewing your patches.

Thank you for your comments. Hopefully I can address at least some of
them here:

1. About patches 1-3, for mem_event clients that don't hook into
mem_access using xc_mem_access_enable_introspection() rather than
xc_mem_access_enable() there is no impact and no overhead. MSR
interception will be disabled at the correct time, no calls to the
"emulate with no writes" code will be made, and the additional
information in mem_events will be simply disregarded by non-interested
parties.

2. About George's comments on patch 5/5: they are spot-on, but again,
with just moving an if() to make the code clearer, and gating the GLA
event filter on introspection being enabled there is again absolutely no
impact or behaviour change for non-introspection clients. I can further
add a bool_t parameter to xc_mem_access_enable_introspection(), called,
for example, filter_gla_events, and make sure that even for clients who
do use xc_mem_access_enable_introspection() all mem_events will be
received if desired.

3. About thinking about the code in specific terms: I try my best not
to, and I hope our discussion so far proves that we're working together
with a common goal to be as generic and with little impact on
non-introspection clients as possible. Speaking of that, thanks to
George and Tim's suggestion I'm investigating the possibility of letting
go of patch 4/5 (which was the biggest problem), in the interest of
easing the review load and making the least modifications possible to Xen.

4. And finally, about the frequent series updates: the simple fact is
that this is the first time I've posted a series on xen-devel, and I'm
getting acquainted with the process and the community as we speak. I do
take responsability for previously posting two reviews in a day, and I
understand in hindsight that it put more stress on reviewers, for which
I apologize. At the time I just thought that moving fast is desirable
(and practical) for everyone.


Thanks,
Razvan Cojocaru