Re: [Qemu-devel] [PATCH v4 17/20] console: make screendump asynchronous

[Marc-André Lureau]

On Wed, Apr 10, 2019 at 12:49 PM Gerd Hoffmann <address@hidden> wrote:
>
> > +static void qmp_screendump_finish(QemuConsole *con, struct qmp_screendump 
> > *dump)
> > +{
> > +    Error *err = NULL;
> > +    DisplaySurface *surface;
> > +    Monitor *prev_mon = cur_mon;
>
> Why this is needed?
>

ppm_save() calls qemu_open() which may lookup fd associated to the
monitor: monitor_fdset_get_fd().

Interestingly, it seems fdset are not coupled with the current
monitor, so it's probably unnecessary to update the monitor to the one
associated with the command invocation.

> > +        /*
> > +         * FIXME: async save with coroutine? it would have to copy or
> > +         * lock the surface.
> > +         */
> > +        ppm_save(dump->filename, surface, &err);
>
> DisplaySurface is just a thin layer above pixman images these days.
> Pixman images are reference counted, so you can
> pixman_image_ref(surface->image) to make sure it doesn't disappear
> underneath you, then pass the pixman image to ppm_save.

ppm_save() is still synchronous. I suppose you suggested that for a
future async version. (note that in this case, ref the surface is
probably not sufficient, as it could be mutated while it is being
saved)

-- 
Marc-André Lureau