Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs

["Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>]

On Fri, Feb 12, 2016 at 7:57 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 12.02.16 at 13:50, <tlengyel@xxxxxxxxxxx> wrote:
> On Feb 12, 2016 03:41, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> In which case ASSERT(is_hvm_vcpu(curr)) would be the common
>> way to document this (at once avoiding the open coding of
>> is_hvm_vcpu()).
>>
>
> I don't think we need an assert here. The function is fine for pv guests as
> well up to that point. Filling in the rest of the registers for pv guests
> can be done when pv events are implemented. Maybe a comment saying so is
> warranted.

I disagree: Either you mean to support PV in the function, in which
case all fields should be filled, or you don't, in which case the
ASSERT() would at once document that PV is intended to not be
supported right now. With the conditional as in your patch any
future reader may either think "bug" or get confused as to the
actual intentions here.

Alright, sounds good to me.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel