| Message ID | 1331494004-26177-5-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sun, 11 Mar 2012 21:26:43 +0200 Alon Levy <alevy@redhat.com> wrote: > Passes the Monitor ptr to the screendump implementation to all for > monitor suspend and resume for qxl to fix screendump regression. > > graphics_console_init signature change required touching every > implemented of screen_dump. There is no change other then an added > parameter. qxl will make use of it in the next patch. NACK on this one. The Monitor object should be restricted to HMP. This patch spreads it to what's going to be the QMP implementation of screendump. The first step here should be to convert the screendump command to the qapi, and lock the HMP shell in hmp_screendump(). However, this brings a new interesting problem: the HMP implementation is actually a QMP client, meaning that it won't have a way to figure out screendump completion either :) Some solutions that come to my mind: 1. Pool the screendump file creation from a timer. Cons: it may return before the file is fully written to disk 2. Use inotify Cons: what about windows? 3. Introduce query-screendump that returns the last screendump status Cons: this is actually making screendump async Anthony, do you have any ideas? Btw, I've started doing the screendump conversion to the qapi, I'll post it soon.
On Tue, Mar 13, 2012 at 10:35:55AM -0300, Luiz Capitulino wrote: > On Sun, 11 Mar 2012 21:26:43 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > Passes the Monitor ptr to the screendump implementation to all for > > monitor suspend and resume for qxl to fix screendump regression. > > > > graphics_console_init signature change required touching every > > implemented of screen_dump. There is no change other then an added > > parameter. qxl will make use of it in the next patch. > > NACK on this one. > > The Monitor object should be restricted to HMP. This patch spreads it to > what's going to be the QMP implementation of screendump. > > The first step here should be to convert the screendump command to the qapi, > and lock the HMP shell in hmp_screendump(). > > However, this brings a new interesting problem: the HMP implementation is > actually a QMP client, meaning that it won't have a way to figure out > screendump completion either :) > > Some solutions that come to my mind: > > 1. Pool the screendump file creation from a timer. > > Cons: it may return before the file is fully written to disk > We know what the file size should be, so we can poll for the actual size. Actually why do we need to poll? we could add a "internal.screendump.complete" or "internal-query-screendump", no? > 2. Use inotify > > Cons: what about windows? > > 3. Introduce query-screendump that returns the last screendump status > > Cons: this is actually making screendump async > > > Anthony, do you have any ideas? > > Btw, I've started doing the screendump conversion to the qapi, I'll post it > soon. I've already sent patches once for a new qapi command, I don't mind you doing this of course.
On Tue, 13 Mar 2012 16:46:12 +0200 Alon Levy <alevy@redhat.com> wrote: > On Tue, Mar 13, 2012 at 10:35:55AM -0300, Luiz Capitulino wrote: > > On Sun, 11 Mar 2012 21:26:43 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > Passes the Monitor ptr to the screendump implementation to all for > > > monitor suspend and resume for qxl to fix screendump regression. > > > > > > graphics_console_init signature change required touching every > > > implemented of screen_dump. There is no change other then an added > > > parameter. qxl will make use of it in the next patch. > > > > NACK on this one. > > > > The Monitor object should be restricted to HMP. This patch spreads it to > > what's going to be the QMP implementation of screendump. > > > > The first step here should be to convert the screendump command to the qapi, > > and lock the HMP shell in hmp_screendump(). > > > > However, this brings a new interesting problem: the HMP implementation is > > actually a QMP client, meaning that it won't have a way to figure out > > screendump completion either :) > > > > Some solutions that come to my mind: > > > > 1. Pool the screendump file creation from a timer. > > > > Cons: it may return before the file is fully written to disk > > > > We know what the file size should be, so we can poll for the actual > size. Actually why do we need to poll? we could add a > "internal.screendump.complete" or "internal-query-screendump", no? Neither is possible because hmp.c really uses QMP as client, except that it's done via C. > > 2. Use inotify > > > > Cons: what about windows? > > > > 3. Introduce query-screendump that returns the last screendump status > > > > Cons: this is actually making screendump async > > > > > > Anthony, do you have any ideas? > > > > Btw, I've started doing the screendump conversion to the qapi, I'll post it > > soon. > > I've already sent patches once for a new qapi command, I don't mind you > doing this of course. It would actually help me if you do it, I have an initial version at: git://repo.or.cz/qemu/qmp-unstable.git qmp-wip/qapi-commands-conv/screendump/rfc It's old and it's on top of some uninteresting stuff, the only commits that matter there are 48e7c01b and 0f5509a8. But this rfc is just to have an idea how the final command will look like. From what I barely remember, I'd suggest you to do the following: 1. Pass the Error object to hw_screen_dump() 2. Convert the screendump command to the qapi 3. Report errors from ppm_save() via Error Important note: my code reports only QERR_OPEN_FILE_FAILED in ppm_save(), but the right thing to do is to report most likely errors like EACCESS, ENOSPC, EPERM, EIO etc.
On Tue, Mar 13, 2012 at 12:59:17PM -0300, Luiz Capitulino wrote: > On Tue, 13 Mar 2012 16:46:12 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Tue, Mar 13, 2012 at 10:35:55AM -0300, Luiz Capitulino wrote: > > > On Sun, 11 Mar 2012 21:26:43 +0200 > > > Alon Levy <alevy@redhat.com> wrote: > > > > > > > Passes the Monitor ptr to the screendump implementation to all for > > > > monitor suspend and resume for qxl to fix screendump regression. > > > > > > > > graphics_console_init signature change required touching every > > > > implemented of screen_dump. There is no change other then an added > > > > parameter. qxl will make use of it in the next patch. > > > > > > NACK on this one. > > > > > > The Monitor object should be restricted to HMP. This patch spreads it to > > > what's going to be the QMP implementation of screendump. > > > > > > The first step here should be to convert the screendump command to the qapi, > > > and lock the HMP shell in hmp_screendump(). > > > > > > However, this brings a new interesting problem: the HMP implementation is > > > actually a QMP client, meaning that it won't have a way to figure out > > > screendump completion either :) > > > > > > Some solutions that come to my mind: > > > > > > 1. Pool the screendump file creation from a timer. > > > > > > Cons: it may return before the file is fully written to disk > > > > > > > We know what the file size should be, so we can poll for the actual > > size. Actually why do we need to poll? we could add a > > "internal.screendump.complete" or "internal-query-screendump", no? > > Neither is possible because hmp.c really uses QMP as client, except that it's > done via C. > > > > 2. Use inotify > > > > > > Cons: what about windows? > > > > > > 3. Introduce query-screendump that returns the last screendump status > > > > > > Cons: this is actually making screendump async > > > > > > > > > Anthony, do you have any ideas? > > > > > > Btw, I've started doing the screendump conversion to the qapi, I'll post it > > > soon. > > > > I've already sent patches once for a new qapi command, I don't mind you > > doing this of course. > > It would actually help me if you do it, I have an initial version at: > > git://repo.or.cz/qemu/qmp-unstable.git qmp-wip/qapi-commands-conv/screendump/rfc > > It's old and it's on top of some uninteresting stuff, the only commits that > matter there are 48e7c01b and 0f5509a8. But this rfc is just to have an idea > how the final command will look like. > > From what I barely remember, I'd suggest you to do the following: > > 1. Pass the Error object to hw_screen_dump() > 2. Convert the screendump command to the qapi > 3. Report errors from ppm_save() via Error > > Important note: my code reports only QERR_OPEN_FILE_FAILED in ppm_save(), but > the right thing to do is to report most likely errors like EACCESS, ENOSPC, > EPERM, EIO etc. ok, I'll take it. Note that I'm going to not send the qxl screendump behavior changing patch again, I think all alternatives to a real async screendump suck, including libvirt changes, and the current behavior of giving an old screendump is good enough hopefully for the real users, which are: boxes - refreshing to get updated thumbnails for it's users - will have 5 seconds old screenshot until async monitor commands land. autotest - I hope they don't compare image contents (that would be hard to do), otherwise it's just for logging sake, and so it will lag a little, too bad.
On Tue, 13 Mar 2012 19:35:24 +0200 Alon Levy <alevy@redhat.com> wrote: > On Tue, Mar 13, 2012 at 12:59:17PM -0300, Luiz Capitulino wrote: > > On Tue, 13 Mar 2012 16:46:12 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > On Tue, Mar 13, 2012 at 10:35:55AM -0300, Luiz Capitulino wrote: > > > > On Sun, 11 Mar 2012 21:26:43 +0200 > > > > Alon Levy <alevy@redhat.com> wrote: > > > > > > > > > Passes the Monitor ptr to the screendump implementation to all for > > > > > monitor suspend and resume for qxl to fix screendump regression. > > > > > > > > > > graphics_console_init signature change required touching every > > > > > implemented of screen_dump. There is no change other then an added > > > > > parameter. qxl will make use of it in the next patch. > > > > > > > > NACK on this one. > > > > > > > > The Monitor object should be restricted to HMP. This patch spreads it to > > > > what's going to be the QMP implementation of screendump. > > > > > > > > The first step here should be to convert the screendump command to the qapi, > > > > and lock the HMP shell in hmp_screendump(). > > > > > > > > However, this brings a new interesting problem: the HMP implementation is > > > > actually a QMP client, meaning that it won't have a way to figure out > > > > screendump completion either :) > > > > > > > > Some solutions that come to my mind: > > > > > > > > 1. Pool the screendump file creation from a timer. > > > > > > > > Cons: it may return before the file is fully written to disk > > > > > > > > > > We know what the file size should be, so we can poll for the actual > > > size. Actually why do we need to poll? we could add a > > > "internal.screendump.complete" or "internal-query-screendump", no? > > > > Neither is possible because hmp.c really uses QMP as client, except that it's > > done via C. > > > > > > 2. Use inotify > > > > > > > > Cons: what about windows? > > > > > > > > 3. Introduce query-screendump that returns the last screendump status > > > > > > > > Cons: this is actually making screendump async > > > > > > > > > > > > Anthony, do you have any ideas? > > > > > > > > Btw, I've started doing the screendump conversion to the qapi, I'll post it > > > > soon. > > > > > > I've already sent patches once for a new qapi command, I don't mind you > > > doing this of course. > > > > It would actually help me if you do it, I have an initial version at: > > > > git://repo.or.cz/qemu/qmp-unstable.git qmp-wip/qapi-commands-conv/screendump/rfc > > > > It's old and it's on top of some uninteresting stuff, the only commits that > > matter there are 48e7c01b and 0f5509a8. But this rfc is just to have an idea > > how the final command will look like. > > > > From what I barely remember, I'd suggest you to do the following: > > > > 1. Pass the Error object to hw_screen_dump() > > 2. Convert the screendump command to the qapi > > 3. Report errors from ppm_save() via Error > > > > Important note: my code reports only QERR_OPEN_FILE_FAILED in ppm_save(), but > > the right thing to do is to report most likely errors like EACCESS, ENOSPC, > > EPERM, EIO etc. > > ok, I'll take it. Note that I'm going to not send the qxl screendump > behavior changing patch again, I think all alternatives to a real async > screendump suck, including libvirt changes, and the current behavior of > giving an old screendump is good enough hopefully for the real users, > which are: Fine with me, you can add a note in screendump's documentation in qapi-schema.json explaining qxl's behavior.
Hi, >> Some solutions that come to my mind: >> >> 1. Pool the screendump file creation from a timer. >> >> Cons: it may return before the file is fully written to disk >> > > We know what the file size should be, so we can poll for the actual > size. Actually why do we need to poll? we could add a > "internal.screendump.complete" or "internal-query-screendump", no? Marc-Andre currently looks at adding support for other file formats. I think it would be good to team up with him. First, with this applied you will not know the size in advance. Also one of the approaches discussed is to allow passing in a file handle. That is a possible way to handle async screendumps too: just write to the passed file handle and close it when done. Obvious drawback is that this will not cover the classic way of specifying the output filename as argument. cheers, Gerd
Hi, > ok, I'll take it. Note that I'm going to not send the qxl screendump > behavior changing patch again, I think all alternatives to a real async > screendump suck, including libvirt changes, and the current behavior of > giving an old screendump is good enough hopefully for the real users, > which are: Agree here. > boxes - refreshing to get updated thumbnails for it's users - will have > 5 seconds old screenshot until async monitor commands land. > autotest - I hope they don't compare image contents (that would be hard > to do), otherwise it's just for logging sake, and so it will lag a > little, too bad. autotest has (used to have?) a install mode where it looked at the screen. I think it isn't used any more these days and unattended install modes (kickstart etc.) of the guests are used instead. But even when checking the screen content it should still work fine, just take a little longer because the screen shots are a bit behind. cheers, Gerd
On Wed, Mar 14, 2012 at 09:25:29AM +0100, Gerd Hoffmann wrote: > Hi, > > > ok, I'll take it. Note that I'm going to not send the qxl screendump > > behavior changing patch again, I think all alternatives to a real async > > screendump suck, including libvirt changes, and the current behavior of > > giving an old screendump is good enough hopefully for the real users, > > which are: > > Agree here. > > > boxes - refreshing to get updated thumbnails for it's users - will have > > 5 seconds old screenshot until async monitor commands land. > > autotest - I hope they don't compare image contents (that would be hard > > to do), otherwise it's just for logging sake, and so it will lag a > > little, too bad. > > autotest has (used to have?) a install mode where it looked at the > screen. I think it isn't used any more these days and unattended > install modes (kickstart etc.) of the guests are used instead. But even > when checking the screen content it should still work fine, just take a > little longer because the screen shots are a bit behind. > Right. Actually I thought about install mode and assumed it would break it but you're right, it shouldn't. > cheers, > Gerd >
On Wed, Mar 14, 2012 at 09:14:46AM +0100, Gerd Hoffmann wrote: > Hi, > > >> Some solutions that come to my mind: > >> > >> 1. Pool the screendump file creation from a timer. > >> > >> Cons: it may return before the file is fully written to disk > >> > > > > We know what the file size should be, so we can poll for the actual > > size. Actually why do we need to poll? we could add a > > "internal.screendump.complete" or "internal-query-screendump", no? > > Marc-Andre currently looks at adding support for other file formats. > I think it would be good to team up with him. > > First, with this applied you will not know the size in advance. Also > one of the approaches discussed is to allow passing in a file handle. > That is a possible way to handle async screendumps too: just write to > the passed file handle and close it when done. Obvious drawback is that > this will not cover the classic way of specifying the output filename as > argument. This would not be a problem from libvirt's POV - we don't really want a file on disk anyway, nor do we want to pull the whole image into memory. Our ideal approach is to just have an pipe FD with QEMU, which we just incrementally read image data from, and forward to the client app via a libvirt stream object. Regards, Daniel
On Wed, 14 Mar 2012 08:37:13 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote: > > First, with this applied you will not know the size in advance. Also > > one of the approaches discussed is to allow passing in a file handle. > > That is a possible way to handle async screendumps too: just write to > > the passed file handle and close it when done. Obvious drawback is that > > this will not cover the classic way of specifying the output filename as > > argument. As Daniel explains below, this is not a drawback and there's no problem supporting multiple ways of returning the image. The real drawback of making this w/o async support is that you can't detect errors. > This would not be a problem from libvirt's POV - we don't really want a > file on disk anyway, nor do we want to pull the whole image into memory. > Our ideal approach is to just have an pipe FD with QEMU, which we just > incrementally read image data from, and forward to the client app via > a libvirt stream object.
On Wed, Mar 14, 2012 at 09:32:58AM -0300, Luiz Capitulino wrote: > On Wed, 14 Mar 2012 08:37:13 +0000 > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > First, with this applied you will not know the size in advance. Also > > > one of the approaches discussed is to allow passing in a file handle. > > > That is a possible way to handle async screendumps too: just write to > > > the passed file handle and close it when done. Obvious drawback is that > > > this will not cover the classic way of specifying the output filename as > > > argument. > > As Daniel explains below, this is not a drawback and there's no problem > supporting multiple ways of returning the image. > > The real drawback of making this w/o async support is that you can't detect > errors. > You also can't detect when the writing is done (unless you continuously try to parse the file yourself..) > > This would not be a problem from libvirt's POV - we don't really want a > > file on disk anyway, nor do we want to pull the whole image into memory. > > Our ideal approach is to just have an pipe FD with QEMU, which we just > > incrementally read image data from, and forward to the client app via > > a libvirt stream object. >
On Wed, Mar 14, 2012 at 03:14:10PM +0200, Alon Levy wrote: > On Wed, Mar 14, 2012 at 09:32:58AM -0300, Luiz Capitulino wrote: > > On Wed, 14 Mar 2012 08:37:13 +0000 > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > > First, with this applied you will not know the size in advance. Also > > > > one of the approaches discussed is to allow passing in a file handle. > > > > That is a possible way to handle async screendumps too: just write to > > > > the passed file handle and close it when done. Obvious drawback is that > > > > this will not cover the classic way of specifying the output filename as > > > > argument. > > > > As Daniel explains below, this is not a drawback and there's no problem > > supporting multiple ways of returning the image. > > > > The real drawback of making this w/o async support is that you can't detect > > errors. > > > > You also can't detect when the writing is done (unless you continuously > try to parse the file yourself..) That's one of the nice benefits of using a pipe - you'll just get EOF when reading from it, when QEMU is done. > > > > This would not be a problem from libvirt's POV - we don't really want a > > > file on disk anyway, nor do we want to pull the whole image into memory. > > > Our ideal approach is to just have an pipe FD with QEMU, which we just > > > incrementally read image data from, and forward to the client app via > > > a libvirt stream object. Daniel
On Wed, 14 Mar 2012 15:14:10 +0200 Alon Levy <alevy@redhat.com> wrote: > On Wed, Mar 14, 2012 at 09:32:58AM -0300, Luiz Capitulino wrote: > > On Wed, 14 Mar 2012 08:37:13 +0000 > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > > First, with this applied you will not know the size in advance. Also > > > > one of the approaches discussed is to allow passing in a file handle. > > > > That is a possible way to handle async screendumps too: just write to > > > > the passed file handle and close it when done. Obvious drawback is that > > > > this will not cover the classic way of specifying the output filename as > > > > argument. > > > > As Daniel explains below, this is not a drawback and there's no problem > > supporting multiple ways of returning the image. > > > > The real drawback of making this w/o async support is that you can't detect > > errors. > > > > You also can't detect when the writing is done (unless you continuously > try to parse the file yourself..) Oh, really? For some reason I had a pipe in my mind, but I honestly don't what's the semantics of fd passing in that regard.
On Wed, Mar 14, 2012 at 10:18:12AM -0300, Luiz Capitulino wrote: > On Wed, 14 Mar 2012 15:14:10 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Wed, Mar 14, 2012 at 09:32:58AM -0300, Luiz Capitulino wrote: > > > On Wed, 14 Mar 2012 08:37:13 +0000 > > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > > > > First, with this applied you will not know the size in advance. Also > > > > > one of the approaches discussed is to allow passing in a file handle. > > > > > That is a possible way to handle async screendumps too: just write to > > > > > the passed file handle and close it when done. Obvious drawback is that > > > > > this will not cover the classic way of specifying the output filename as > > > > > argument. > > > > > > As Daniel explains below, this is not a drawback and there's no problem > > > supporting multiple ways of returning the image. > > > > > > The real drawback of making this w/o async support is that you can't detect > > > errors. > > > > > > > You also can't detect when the writing is done (unless you continuously > > try to parse the file yourself..) > > Oh, really? For some reason I had a pipe in my mind, but I honestly don't > what's the semantics of fd passing in that regard. Like Daniel pointed out, I am dead wrong here, sorry for the noise.
diff --git a/console.c b/console.c index 6a463f5..3e386fc 100644 --- a/console.c +++ b/console.c @@ -173,7 +173,7 @@ void vga_hw_invalidate(void) active_console->hw_invalidate(active_console->hw); } -void vga_hw_screen_dump(const char *filename) +void vga_hw_screen_dump(const char *filename, Monitor *mon) { TextConsole *previous_active_console; bool cswitch; @@ -187,7 +187,7 @@ void vga_hw_screen_dump(const char *filename) console_select(0); } if (consoles[0] && consoles[0]->hw_screen_dump) { - consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); + consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, mon); } else { error_report("screen dump not implemented"); } diff --git a/console.h b/console.h index 4334db5..0d2cf30 100644 --- a/console.h +++ b/console.h @@ -343,7 +343,8 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch) typedef void (*vga_hw_update_ptr)(void *); typedef void (*vga_hw_invalidate_ptr)(void *); -typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch); +typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch, + Monitor *mon); typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *); DisplayState *graphic_console_init(vga_hw_update_ptr update, @@ -354,7 +355,7 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update, void vga_hw_update(void); void vga_hw_invalidate(void); -void vga_hw_screen_dump(const char *filename); +void vga_hw_screen_dump(const char *filename, Monitor *mon); void vga_hw_text_update(console_ch_t *chardata); int is_graphic_console(void); diff --git a/hw/blizzard.c b/hw/blizzard.c index c7d844d..8ccea7f 100644 --- a/hw/blizzard.c +++ b/hw/blizzard.c @@ -933,7 +933,7 @@ static void blizzard_update_display(void *opaque) } static void blizzard_screen_dump(void *opaque, const char *filename, - bool cswitch) + bool cswitch, Monitor *mon) { BlizzardState *s = (BlizzardState *) opaque; diff --git a/hw/g364fb.c b/hw/g364fb.c index 3a0b68f..f89000c 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -289,7 +289,8 @@ static void g364fb_reset(G364State *s) g364fb_invalidate_display(s); } -static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch) +static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { G364State *s = opaque; int y, x; diff --git a/hw/omap_dss.c b/hw/omap_dss.c index 86ed6ea..b4a1a93 100644 --- a/hw/omap_dss.c +++ b/hw/omap_dss.c @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, #if 0 s->state = graphic_console_init(omap_update_display, - omap_invalidate_display, omap_screen_dump, s); + omap_invalidate_display, + omap_screen_dump, + NULL, s); #endif return s; diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c index f172093..ed2325d 100644 --- a/hw/omap_lcdc.c +++ b/hw/omap_lcdc.c @@ -264,7 +264,8 @@ static int ppm_save(const char *filename, uint8_t *data, return 0; } -static void omap_screen_dump(void *opaque, const char *filename, bool cswitch) +static void omap_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { struct omap_lcd_panel_s *omap_lcd = opaque; if (cswitch) { diff --git a/hw/qxl.c b/hw/qxl.c index bcfd661..d21b508 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1486,7 +1486,8 @@ static void qxl_hw_invalidate(void *opaque) vga->invalidate(vga); } -static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch) +static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { PCIQXLDevice *qxl = opaque; VGACommonState *vga = &qxl->vga; @@ -1497,7 +1498,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch) qxl_render_update(qxl, filename); break; case QXL_MODE_VGA: - vga->screen_dump(vga, filename, cswitch); + vga->screen_dump(vga, filename, cswitch, mon); break; default: break; diff --git a/hw/sm501.c b/hw/sm501.c index 786e076..eedcb8e 100644 --- a/hw/sm501.c +++ b/hw/sm501.c @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, } /* create qemu graphic console */ - s->ds = graphic_console_init(sm501_update_display, NULL, - NULL, NULL, s); + s->ds = graphic_console_init(sm501_update_display, NULL, NULL, + NULL, s); } diff --git a/hw/tcx.c b/hw/tcx.c index ac7dcb4..e800cb5 100644 --- a/hw/tcx.c +++ b/hw/tcx.c @@ -56,8 +56,10 @@ typedef struct TCXState { uint8_t dac_index, dac_state; } TCXState; -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch); -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch); +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon); +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon); static void tcx_set_dirty(TCXState *s) { @@ -574,7 +576,8 @@ static int tcx_init1(SysBusDevice *dev) return 0; } -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch) +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { TCXState *s = opaque; FILE *f; @@ -601,7 +604,8 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch) return; } -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch) +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { TCXState *s = opaque; FILE *f; diff --git a/hw/vga.c b/hw/vga.c index 6dc98f6..9af231e 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -163,7 +163,8 @@ static uint32_t expand4[256]; static uint16_t expand2[256]; static uint8_t expand4to8[16]; -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch); +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon); static void vga_update_memory_access(VGACommonState *s) { @@ -2409,7 +2410,8 @@ int ppm_save(const char *filename, struct DisplaySurface *ds) /* save the vga display in a PPM image even if no display is available */ -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch) +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { VGACommonState *s = opaque; diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 142d9f4..803118c 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -1003,11 +1003,12 @@ static void vmsvga_invalidate_display(void *opaque) /* save the vga display in a PPM image even if no display is available */ -static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch) +static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch, + Monitor *mon) { struct vmsvga_state_s *s = opaque; if (!s->enable) { - s->vga.screen_dump(&s->vga, filename, cswitch); + s->vga.screen_dump(&s->vga, filename, cswitch, mon); return; } diff --git a/monitor.c b/monitor.c index cbdfbad..cdae23f 100644 --- a/monitor.c +++ b/monitor.c @@ -895,7 +895,7 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict, static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) { - vga_hw_screen_dump(qdict_get_str(qdict, "filename")); + vga_hw_screen_dump(qdict_get_str(qdict, "filename"), mon); return 0; }
Passes the Monitor ptr to the screendump implementation to all for monitor suspend and resume for qxl to fix screendump regression. graphics_console_init signature change required touching every implemented of screen_dump. There is no change other then an added parameter. qxl will make use of it in the next patch. compiles with ./configure Signed-off-by: Alon Levy <alevy@redhat.com> --- console.c | 4 ++-- console.h | 5 +++-- hw/blizzard.c | 2 +- hw/g364fb.c | 3 ++- hw/omap_dss.c | 4 +++- hw/omap_lcdc.c | 3 ++- hw/qxl.c | 5 +++-- hw/sm501.c | 4 ++-- hw/tcx.c | 12 ++++++++---- hw/vga.c | 6 ++++-- hw/vmware_vga.c | 5 +++-- monitor.c | 2 +- 12 files changed, 34 insertions(+), 21 deletions(-)