Re: [Qemu-devel] [RFC 2/3] QMP: rate limit BLOCK_IO_ERROR

[Daniel P. Berrange]

On Wed, Jul 23, 2014 at 09:17:17AM -0400, Luiz Capitulino wrote:
> This event has the same characteristics of the other rate-limited
> events, mainly we can emit dozens of it. Rate limit it then.
> 
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
>  monitor.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/monitor.c b/monitor.c
> index 5bc70a6..33abe6c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -589,6 +589,7 @@ static void monitor_qapi_event_init(void)
>      monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000);
>      monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000);
>      monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000);
> +    monitor_qapi_event_throttle(QAPI_EVENT_BLOCK_IO_ERROR, 1000);


The rate limiting code only rate limits at the granularity of
individual event types. If there is context sensitive data associated
with events then the rate limiting will cause problems for applications
tracking the events.

eg consider with the simpler RTC CHANGE events if we get

   QAPI_EVENT_RTC_CHANGE offset=30
   QAPI_EVENT_RTC_CHANGE offset=700
   QAPI_EVENT_RTC_CHANGE offset=340

then rate limiting will mean that the application only receives

   QAPI_EVENT_RTC_CHANGE offset=340

This is fine because the application will always end up with a correct
view of the current system state.


For the BLOCK IO ERROR events this does not work because the events are
device and operation specific.

  QAPI_EVENT_BLOCK_IO_ERROR dev=ide0-hd1 op=read action=stop
  QAPI_EVENT_BLOCK_IO_ERROR dev=scsi1-hd2 op=write action=stop
  QAPI_EVENT_BLOCK_IO_ERROR dev=ide0-hd1 op=write action=stop

with throttling the app wll only receive

  QAPI_EVENT_BLOCK_IO_ERROR dev=ide0-hd1 op=write action=stop

which means it will have an *incorrect* view of the system state because
the info about  scsi1-hd2 is irretrievably lost, likewise info about the
read operation of ide0-hd1.

If you want to throttle BLOCK IO ERROR events, then you need to make the
monitor throttling more intelligent, so that it hashes on all the contextual
state. In this case you'd have to throttle based on (event, dev, op) to get
correct application behaviour.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|