From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:42033) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0BPB-0000q2-QW for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:34:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0BP5-0003w0-U6 for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:34:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:20613) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0BP5-0003vv-Io for qemu-devel@nongnu.org; Wed, 22 Feb 2012 07:34:39 -0500 Date: Wed, 22 Feb 2012 13:34:29 +0100 From: Alon Levy Message-ID: <20120222123429.GE607@garlic.redhat.com> References: <1329860377-6284-1-git-send-email-alevy@redhat.com> <1329860377-6284-10-git-send-email-alevy@redhat.com> <4F44D594.8000004@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F44D594.8000004@redhat.com> Subject: Re: [Qemu-devel] [RFC v4 9/9] qxl-render: call ppm_save on bh List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: spice-devel@freedesktop.org, yhalperi@redhat.com, qemu-devel@nongnu.org, elmarco@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. > > > +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? You don't get the cookie in the update_area bottom half. It's solemnly for update_area_complete. It's not easy to associate a cookie with it, since it may be called by the server without previous provocation from qemu. I'll take another look. > > cheers, > Gerd > >