Re: [Xen-devel] [PATCH] x86/hvm: add support for broadcast of buffered ioreqs...

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.07.15 at 17:42, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 10 July 2015 16:39
>> To: Paul Durrant
>> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxx; Keir (Xen.org)
>> Subject: Re: [PATCH] x86/hvm: add support for broadcast of buffered
>> ioreqs...
>> 
>> >>> On 10.07.15 at 15:45, <paul.durrant@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/io.c
>> > +++ b/xen/arch/x86/hvm/io.c
>> > @@ -60,8 +60,7 @@ void send_timeoffset_req(unsigned long timeoff)
>> >      if ( timeoff == 0 )
>> >          return;
>> >
>> > -    if ( !hvm_buffered_io_send(&p) )
>> > -        printk("Unsuccessful timeoffset update\n");
>> > +    hvm_broadcast_ioreq(&p, 1);
>> >  }
>> 
>> The rest of the patch looks okay, but I'm not happy with the deletion
>> of this message, as it served a purpose (ignoring the fact that one
>> didn't know the affected domain etc). I would think
>> hvm_broadcast_ioreq() should have a return value, indicating all
>> succeeded, some succeeded, or all failed. And perhaps servers
>> without bufioreq page should then rather count as "succeeded".
>> 
>> Otoh I can see that "some succeeded" may not be really useful
>> here, as the caller won't know whether the one(s) that failed are
>> important, or whether they would have dropped the notification
>> on the floor only anyway (like qemuu appears to do right now).
>> 
> 
> Ok, how about I just have it return a value indicating if at least one send 
> failed, and then add a gprintk() on that?

Sounds reasonable.

Jan


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