qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>,
	Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async
Date: Tue, 6 Mar 2012 10:53:42 -0300	[thread overview]
Message-ID: <20120306105342.5921e785@doriath.home> (raw)
In-Reply-To: <4F561661.7080805@codemonkey.ws>

On Tue, 06 Mar 2012 07:51:29 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 03/06/2012 07:16 AM, Alon Levy wrote:
> > On Tue, Mar 06, 2012 at 09:24:27AM -0300, Luiz Capitulino wrote:
> >> On Tue, 06 Mar 2012 08:36:34 +0100
> >> Gerd Hoffmann<kraxel@redhat.com>  wrote:
> >>
> >>>    Hi,
> >>>
> >>>>> How would the parallel execution facility be opaque to the implementer?
> >>>>> screendump returns, screendump_async needs to pass a closure. You can
> >>>>> automatically generate any amount of code, but you can only have a
> >>>>> single function implementation with longjmp/coroutine, or having a
> >>>>> saparate thread per command but that would mean taking locks for
> >>>>> anything not trivial, which avoids the no-change-to-implementation. Is
> >>>>> this what you have in mind?
> >>>>
> >>>> It would not be opaque to the implementer.  But it would avoid
> >>>> introducing new commands and events, instead we have a unified mechanism
> >>>> to signal completion.
> >>>
> >>> Ok.  We have a async mechanism today: .mhandler.cmd_async = ...
> >>>
> >>> I know it has its problems like no cancelation and is deprecated and
> >>> all.  But still: how about using it as interim until QAPI-based async
> >>> monitor support is ready?  We could unbreak qxl screendumps without
> >>> having to introduce a new (but temporary!) screendump_async command +
> >>> completion event.
> >>
> >> There are a few problems here, but the blocking one is that a command
> >> can't go from sync to async. This is an incompatible change.
> >>
> >> If you mind adding the temporary command and if this issue is so rare
> >> that none can reproduce it, then I'd suggest to wait for 1.2.
> >>
> >
> > There are two options really:
> >   1. revert the patches that changed qxl screendump to save the ppm
> >   before (possibly) updating the framebuffer.
> >   2. introduce a new command that is really async
> >
> >   The third option, what Gerd proposes, doesn't break the blocking chain
> >   going from the A, the dual purpose spice client and libvirt client,
> >   through libvirt, qemu, spice and back to A.
> >
> > If no one can reproduce the block then it would seem 1 makes sense.
> 
> So let's start with a reproducible test case that demonstrates the problem 
> before we start introducing new commands then if there's doubt about the nature 
> of the problem.

Completely agree, I was going to suggest that too.

  reply	other threads:[~2012-03-06 13:54 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-05 14:16 [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async Alon Levy
2012-03-05 14:16 ` [Qemu-devel] [PATCH v2 2/2] add qmp screendump-async Alon Levy
2012-03-05 14:33 ` [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async Anthony Liguori
2012-03-05 15:17   ` Alon Levy
2012-03-05 15:56   ` [Qemu-devel] [PATCH v3 0/3] screendump async command Alon Levy
2012-03-05 15:56     ` [Qemu-devel] [PATCH v3 1/3] monitor, console: add QEVENT_SCREEN_DUMP_COMPLETE Alon Levy
2012-03-05 15:56     ` [Qemu-devel] [PATCH v3 2/3] console: add hw_screen_dump_async Alon Levy
2012-03-05 15:56     ` [Qemu-devel] [PATCH v3 3/3] add qmp screendump-async Alon Levy
2012-03-05 17:17     ` [Qemu-devel] [PATCH v3 0/3] screendump async command Anthony Liguori
2012-03-05 17:20   ` [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async Avi Kivity
2012-03-05 17:27     ` Anthony Liguori
2012-03-05 17:29       ` Avi Kivity
2012-03-05 17:56         ` Luiz Capitulino
2012-03-05 18:16         ` Anthony Liguori
2012-03-05 18:22           ` Avi Kivity
2012-03-05 19:32             ` Anthony Liguori
2012-03-05 17:31       ` Luiz Capitulino
2012-03-05 18:09         ` Alon Levy
2012-03-05 18:17           ` Avi Kivity
2012-03-05 18:58             ` Alon Levy
2012-03-05 19:45               ` Luiz Capitulino
2012-03-06  7:36             ` Gerd Hoffmann
2012-03-06  7:43               ` Alon Levy
2012-03-06  7:56               ` Alon Levy
2012-03-06  8:10                 ` Gerd Hoffmann
2012-03-06  9:35                   ` Alon Levy
2012-03-06 12:24               ` Luiz Capitulino
2012-03-06 13:16                 ` Alon Levy
2012-03-06 13:51                   ` Anthony Liguori
2012-03-06 13:53                     ` Luiz Capitulino [this message]
2012-03-06 14:23                       ` Alon Levy
2012-03-06 15:07                         ` Anthony Liguori
2012-03-06 15:56                     ` Alon Levy
2012-03-06 16:02                       ` Alon Levy
2012-03-06 16:16                       ` Anthony Liguori
2012-03-06 16:26                         ` Alon Levy
2012-03-06 16:47                           ` Anthony Liguori
2012-03-07  6:57                         ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120306105342.5921e785@doriath.home \
    --to=lcapitulino@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=avi@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).