Re: [Qemu-devel] [RFC v4 9/9] qxl-render: call ppm_save on bh

[Alon Levy <alevy@redhat.com>]

On Wed, Feb 22, 2012 at 12:46:28PM +0100, Gerd Hoffmann wrote:
> On 02/21/12 22:39, Alon Levy wrote:
> > This changes the behavior of the monitor command. After the previous
> > patch, there is no longer an option of deadlock with virt-manager, but
> > ppm_save is called too early, before the update has completed. With this
> > patch it is called at the correct moment, but that means there is a race
> > between the monitor command completing and the screendump file being saved.
> > 
> > The only solution is to use an asynchronous monitor command. For a
> > previous round of this see:
> >  http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02810.html
> > 
> > Since that's contentious, I'm suggesting we do something that is almost
> > correct and doesn't hang, instead of correct and hangs. The screendump
> > user can inotify on the directory and the file if need be. For casual
> > monitor usage there is no difference.
> 
> I still think we should defer that and figure how to fix that properly,
> either using (internally) async monitor commands via qapi, or using an
> event.

I'm looking at QAPI. But in general I think it's better to have a
correct image then a timely image. Without this patch you get an old
image, unless you do what autotest does and request one every few
seconds, then you won't really notice the difference (it would be one
frame offset, kinda).

> 
> > +typedef struct QXLPPMSaveBHData {
> > +    PCIQXLDevice *qxl;
> > +    QXLCookie *cookie;
> > +} QXLPPMSaveBHData;
> 
> Is there a need for a separate struct?  I think you can just stick the
> filename into the QXLCookie struct, then write out screen shots in the
> update area bottom half, no?
> 
> cheers,
>   Gerd
>