Re: [Qemu-devel] [v3 04/13] qemu-file: Add tow function will be used in migration

[Li, Liang Z]

> > +size_t migrate_qemu_add_compression_data(QEMUFile *f,
> > +        const uint8_t *p, size_t size, int level)
> 
> It's an odd name, QEMUFile is only used by migration anyway; maybe
> qemufile_add_compression_data ?
> 
> > +{
> > +    size_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int);
> > +
> > +    if (compress2(f->buf + f->buf_index + sizeof(int), &blen, (Bytef *)p,
> > +            size, level) != Z_OK) {
> > +        error_report("Compress Failed!");
> > +        return 0;
> > +    }
> 
> What are the 'sizeof(int)'s for?  It's unusual because we normally keep the
> format of the stream the same even if one side was a 32bit qemu and the
> other 64bit.
> How do you know that there is enough space for the compress - i.e. what
> happens if f->buf_index is too high?

The compress2 will return failed if that happened. The spare space should be
checked before calling compress2, I will make a change.

> > +    qemu_put_be32(f, blen);
> > +    f->buf_index += blen;
> > +    return blen + sizeof(int);
> > +}
> > +
> > +int migrate_qemu_flush(QEMUFile *f_des, QEMUFile *f_src) {
> > +    int len = 0;
> > +
> > +    if (f_src->buf_index > 0) {
> > +        len = f_src->buf_index;
> > +        qemu_put_buffer(f_des, f_src->buf, f_src->buf_index);
> > +        f_src->buf_index = 0;
> > +    }
> > +    return len;
> > +}
> 
> Can you explain a bit more here how you're using the src file; I think you're
> using it as kind of a dummy buffer; but it needs to be documented
> somewhere.  Again I'm also not sure of the name of this function.
> 
Yes, it is for dummy buffer use, and I am not satisfied with the function name
too.