On Wed, Apr 10, 2019 at 12:49 PM Gerd Hoffmann <kra...@redhat.com> 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