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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----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?

  Paul

> Jan


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