"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> This is still a work in progress, but get everything sent as expected
>> and it is faster than the code that is already there.
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> ---
>> migration/ram.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 104 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index fdb5bf07a5..efbb253c1a 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -747,6 +747,100 @@ MultifdMethods multifd_none_ops = {
>> .recv_pages = none_recv_pages
>> };
>>
>> +/* Multifd zlib compression */
>> +
>
> Comment the return value?
Once there, commented all the functions.
>> +static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used)
>> +{
>> + struct iovec *iov = p->pages->iov;
>> + z_stream *zs = &p->zs;
>> + uint32_t out_size = 0;
>> + int ret;
>> + int i;
>
> uint32_t to match 'used' ?
Done
>> + for (i = 0; i < used; i++) {
>> + uint32_t available = p->zbuff_len - out_size;
>> + int flush = Z_NO_FLUSH;
>> +
>> + if (i == used - 1) {
>> + flush = Z_SYNC_FLUSH;
>> + }
>> +
>> + zs->avail_in = iov[i].iov_len;
>> + zs->next_in = iov[i].iov_base;
>> +
>> + zs->avail_out = available;
>> + zs->next_out = p->zbuff + out_size;
>> +
>> + ret = deflate(zs, flush);
>> + if (ret != Z_OK) {
>> + printf("problem with deflate? %d\n", ret);
>
> If it's an error it should probably be at least an fprintf(stderr or
> err_ something.
We don't have any error arround really, we need one. Searching for it.
> Should this also check that the avail_in/next_in has consumed the whole
> of the input?
I am not checking because _it_ is supposed to b doing it right. We can
test it through, specially in reception.
>> + qemu_mutex_unlock(&p->mutex);
>
> Can you explain and/or comment whyit's unlocked here in the error path?
Uh, oh ....
Leftover for when it was done inline inside the main function.
Removed.
>> + return -1;
>> + }
>> + out_size += available - zs->avail_out;
>> + }
>> + p->next_packet_size = out_size;
>
> Some traces_ wouldn't hurt.
Humm, you are right here.
Thanks, Juan.
