Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.05.15 at 12:56, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 05/28/2015 11:39 AM, Jan Beulich wrote:
>>>>> On 28.05.15 at 07:33, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> Changes since V6:
>>>  - Removed "Reviewed-by: Tim Deegan <tim@xxxxxxx>", as the patch
>>>    went beyond cosmetic-only changes.
>>>  - Parenthesized hvm_event_cr in event.c, to prevent expansion of
>>>    the macro with the same name if event.h gets #included.
>> 
>> I'm sorry, but I specifically said that you need to include the header
>> declaring the function (and macro) in the file defining it, but ...
>> 
>>> --- a/xen/arch/x86/hvm/event.c
>>> +++ b/xen/arch/x86/hvm/event.c
>>> @@ -21,6 +21,7 @@
>>>  
>>>  #include <xen/vm_event.h>
>>>  #include <xen/paging.h>
>>> +#include <asm/monitor.h>
>>>  #include <public/vm_event.h>
>>>  
>>>  static void hvm_event_fill_regs(vm_event_request_t *req)
>> 
>> ... you didn't (and you also didn't in a prereq patch, which would have
>> been the alternative).
> 
> I see, sorry for the misunderstanding - I read your comment to mean that
> in case the header gets included at a later time there might be
> problems, which now won't happen. I will re-submit with the proper
> header included.

Thanks. Just to clarify: What I said was that your previous version
introduced a latent issue, hidden by another bug. The implied
consequence for someone like me was that both issues should be
fixed, rather than waiting for either to become a real one. Perhaps
I should have said so explicitly...

Jan


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