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. > > > +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?
The bh is there so I don't have to reset the opaque for the update_area bh - only way to do that is to create a new bh everytime, since there can be multiple update_area bh's scheduled at the same time. The struct can go, if I change qxl -> qxl0, which I guess is fine for this command. > > cheers, > Gerd > >
