Re: [PATCH 01/14] ram.c: Let the compress threads return a CompressResult enum

[Lukas Straub]

On Mon, 3 Apr 2023 09:25:41 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 2/4/23 19:56, Lukas Straub wrote:
> > This will be used in the next commits to move save_page_header()
> > out of compress code.
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >   migration/ram.c | 34 ++++++++++++++++++++++------------
> >   1 file changed, 22 insertions(+), 12 deletions(-)
> > 
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 9d1817ab7b..ca561e62bd 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -493,10 +493,17 @@ MigrationOps *migration_ops;
> > 
> >   CompressionStats compression_counters;
> > 
> > +enum CompressResult {
> > +    RES_NONE = 0,  
> 
> What about RES_INVALID?

I think RES_NONE is more accurate, because having no result is a common
case. The submit side first handles the result from a previous
compression and then submits the new compression request. And for
example, when submitting the very first request to the thread there
won't be a previous result. Or when submitting after the threads where
flushed.

I just opted to return RES_NONE on error, because it seems more correct.

> > +    RES_ZEROPAGE = 1,
> > +    RES_COMPRESS = 2
> > +};  
> 
> 
> > -static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock 
> > *block,
> > -                                 ram_addr_t offset, uint8_t *source_buf)
> > +static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
> > +                                           RAMBlock *block, ram_addr_t 
> > offset,
> > +                                           uint8_t *source_buf)
> >   {  
> 
> 
> >       if (ret < 0) {
> >           qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> >           error_report("compressed data failed!");
> > +        return RES_NONE;
> >       }
> > -    return false;
> > +    return RES_COMPRESS;
> >   }  
> 



--

pgpGn4YENYCwp.pgp
Description: OpenPGP digital signature