Re: Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 07/13/2015 05:25 PM, Wei Liu wrote:
> On Mon, Jul 13, 2015 at 05:10:21PM +0300, Razvan Cojocaru wrote:
>> On 07/13/2015 04:51 PM, Wei Liu wrote:
>>> On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
>>>> Hello,
>>>>
>>>> I'd like to ask for a freeze exception for the "vm_event memory
>>>> introspection helpers" series.
>>>>
>>>> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
>>>> [PATCH 2/3] xen/vm_event: Support for guest-requested events
>>>> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
>>>> vm_event reply
>>>>
>>>> All patches have been acked by at least one person (though patch 1 is
>>>> still under some discussion).
>>>>
>>>> 1. Benefits of the series making it in this release:
>>>>
>>>> * Probably the most important benefit is that 4.6 development has been
>>>> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
>>>> in a consistent manner (all register-write vm_events are pre-write events).
>>>>
>>>> * There are 3rd parties interested in these features (Tamas, for
>>>> example, has already expressed interest in uses of patch 1/1).
>>>>
>>>
>>> These aren't really good arguments for getting this in for 4.6. It would
>>> be easy for you and Tamas to carry three patches for a while.
>>>
>>> What are the impact to end users (end users -- meaning either system
>>> administrator that sets up Xen and other developers who want to use the
>>> new interface you introduce)?  Does this series consist of the last
>>> missing pieces of a feature?
>>
>> Not just missing pieces of a feature, the guest-requested (formerly
>> VMCALL) vm_event patch is crucial for memory introspection, and without
>> the first patch it's very difficult to resume monitoring Windows guests
>> that have been put in hybernate-mode (instead of powered off).
>>
> 
> Right, so with these three patches, memory introspection is a complete
> feature (as in, to cover most use cases)? Or is it still a work in
> progress even after these patches are merged?

Indeed, with these three patches x86 memory introspection is a complete
feature. There are still tweaks, of course, such as the ones mentioned
before (ARM support would be nice, some optimizations for REP emulation,
extending the emulator to be able to handle everything, etc.), but the
important part would be complete with these.

>> I don't follow the impact question, sorry. If an administrator doesn't
>> use these features, the impact (in terms of speed and resources) should
>> be next to nothing.
>>
> 
> OK, let me be specific. If I'm to build a product by either using Xen as
> out-of-box virtualisation platform or building my own software on top of
> Xen, would it make any *big* difference if I have your three patches?

I would say yes, and this from experience. I've been maintaining a
series of patches internally for a while now (I think since Xen 4.0),
and I've come across two main categories of non-trivial issues:

1. Things tend to change a lot with Xen, especially lately, so at least
for individual patches it has come more often than I would have thought
to almost complete rewrites when going up to the next Xen version.

2. Even with a lot of work and testing, I still think a patch that has
been reviewed for a few months on xen-devel is better that an in-house
one. Xen is one of the best written and most elegant C projects I've
seen, but it's still evolving quickly and largely undocumented, so
there's always the chance a lone developer will miss something somewhere.

So yes, again, I'd say that being able to use Xen out-of-the box for
introspection is the (much) better choice, at least in my experience.

>> If a freeze exception for the whole series is not an option at this
>> point, would it be appropriate to just drop patches requiring more
>> review and get the more acked ones in?
>>
> 
> I'm not saying it's not possible. I'm trying to make sense of what is
> going on at the moment.
> 
> Maybe other maintainers can help enlighten me and express their
> preference? And I guess with your maintainer's hat on it would be a
> "yes, let's merge them" from you. :-)

I understand. Thanks for having this conversation!


Thanks,
Razvan