Re: [Qemu-devel] [PATCH 3/4] monitor, console: add QEVENT_SCREEN_DUMP_COMPLETE

[Alon Levy <alevy@redhat.com>]

On Tue, Feb 28, 2012 at 05:01:13PM -0300, Luiz Capitulino wrote:
> On Fri, 24 Feb 2012 23:22:04 +0200
> Alon Levy <alevy@redhat.com> wrote:
> 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> > ---
> >  QMP/qmp-events.txt |   14 ++++++++++++++
> >  console.c          |   18 ++++++++++++++++++
> >  console.h          |    1 +
> >  monitor.c          |    2 ++
> >  monitor.h          |    1 +
> >  5 files changed, 36 insertions(+), 0 deletions(-)
> > 
> > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> > index 9286af5..0d1c9f4 100644
> > --- a/QMP/qmp-events.txt
> > +++ b/QMP/qmp-events.txt
> > @@ -335,3 +335,17 @@ Example:
> >                 "len": 10737418240, "offset": 134217728,
> >                 "speed": 0 },
> >       "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
> > +
> > +SCREEN_DUMP_COMPLETE
> > +--------------------
> > +
> > +Emitted when screen-dump-async completes.
> > +
> > +Data:
> > +
> > +- "filename": Name of file containing screen dump (json-string)
> > +
> > +Example:
> > +
> > +{ "event": "SCREEN_DUMP_COMPLETE",
> > +     "data": { "filename": "/tmp/a.ppm" } }
> 
> This example is incomplete, a QMP event contains more fields.

timestamp, right?

> 
> > diff --git a/console.c b/console.c
> > index 6a463f5..6750538 100644
> > --- a/console.c
> > +++ b/console.c
> > @@ -24,6 +24,8 @@
> >  #include "qemu-common.h"
> >  #include "console.h"
> >  #include "qemu-timer.h"
> > +#include "qjson.h"
> > +#include "monitor.h"
> >  
> >  //#define DEBUG_CONSOLE
> >  #define DEFAULT_BACKSCROLL 512
> > @@ -1707,3 +1709,19 @@ PixelFormat qemu_default_pixelformat(int bpp)
> >      }
> >      return pf;
> >  }
> > +
> > +void monitor_protocol_screen_dump_complete_event(const char *filename)
> > +{
> > +    QObject *event_data;
> > +    /*
> > +     * TODO: Only good for a single user, or a very aware user
> > +     * that changes the filename to distinguish between different
> > +     * screendumps.
> 
> This comment pertains to the event documentation.

Move to documentation, check.

> 
> > +     * Should include support for different monitors on the same device,
> > +     * and for different devices. Got stuck trying to figure a canonical string
> > +     * represnetation of a device.
> > +     */
> 
> I think the above comment is not worth it.

This half of the comment? ok.

> 
> > +    event_data = qobject_from_jsonf("{ 'filename': %s }", filename);
> > +    monitor_protocol_event(QEVENT_SCREEN_DUMP_COMPLETE, event_data);
> > +    qobject_decref(event_data);
> > +}
> > diff --git a/console.h b/console.h
> > index a95b581..c22803c 100644
> > --- a/console.h
> > +++ b/console.h
> > @@ -353,6 +353,7 @@ void vga_hw_update(void);
> >  void vga_hw_invalidate(void);
> >  void vga_hw_screen_dump(const char *filename);
> >  void vga_hw_text_update(console_ch_t *chardata);
> > +void monitor_protocol_screen_dump_complete_event(const char *filename);
> >  
> >  int is_graphic_console(void);
> >  int is_fixedsize_console(void);
> > diff --git a/monitor.c b/monitor.c
> > index 953e748..1a65c41 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -493,6 +493,8 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
> >              break;
> >          case QEVENT_WAKEUP:
> >              event_name = "WAKEUP";
> > +        case QEVENT_SCREEN_DUMP_COMPLETE:
> > +            event_name = "SCREEN_DUMP_COMPLETE";
> >              break;
> >          default:
> >              abort();
> > diff --git a/monitor.h b/monitor.h
> > index 0d49800..227ebf2 100644
> > --- a/monitor.h
> > +++ b/monitor.h
> > @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
> >      QEVENT_DEVICE_TRAY_MOVED,
> >      QEVENT_SUSPEND,
> >      QEVENT_WAKEUP,
> > +    QEVENT_SCREEN_DUMP_COMPLETE,
> >      QEVENT_MAX,
> >  } MonitorEvent;
> >  
>