[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 7/9] Add qemu_put_buffer_no_copy
From: |
Orit Wasserman |
Subject: |
Re: [Qemu-devel] [PATCH v3 7/9] Add qemu_put_buffer_no_copy |
Date: |
Thu, 21 Mar 2013 19:39:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
On 03/21/2013 07:34 PM, Juan Quintela wrote:
> Orit Wasserman <address@hidden> wrote:
>> This allow us to add a buffer to the iovec to send without copying it
>> into the static buffer.
>>
>> Signed-off-by: Orit Wasserman <address@hidden>
>> ---
>> include/migration/qemu-file.h | 5 +++++
>> savevm.c | 37 ++++++++++++++++++++++++++++---------
>> 2 files changed, 33 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
>> index 8d3da9b..5168be2 100644
>> --- a/include/migration/qemu-file.h
>> +++ b/include/migration/qemu-file.h
>> @@ -75,6 +75,11 @@ int qemu_fclose(QEMUFile *f);
>> int64_t qemu_ftell(QEMUFile *f);
>> void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
>> void qemu_put_byte(QEMUFile *f, int v);
>> +/*
>> + * put_buffer without copying the buffer.
>> + * The buffer should be available till it is sent.
>> + */
>> +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size);
>>
>> static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
>> {
>> diff --git a/savevm.c b/savevm.c
>> index 83aa9e7..fbfb9e3 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -621,6 +621,30 @@ int qemu_fclose(QEMUFile *f)
>> return ret;
>> }
>>
>> +
>> +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size)
>> +{
>> + if (f->last_error) {
>> + return;
>> + }
>> +
>> + if (f->is_write == 0 && f->buf_index > 0) {
>> + fprintf(stderr,
>> + "Attempted to write to buffer while read buffer is not
>> empty\n");
>> + abort();
>> + }
>
> I don't understand this test at all (yes, I know that the test already
> existed).
>
> We shouldn't never arrived qemu_put_buffer() with a QEMUFile with
> f->is_write == 0.
>
> Change it for one assert()?
>
>> + f->iov[f->iovcnt].iov_base = f->buf + f->buf_index;
>> + f->iov[f->iovcnt++].iov_len = size;
>
> This is clearly wrong, or I have completely missunderstood it (I will
> give a 50% to each posiblitiy).
>
> Here we should be using "buf" and "size" passed as paramenters, f->buf
> and f->buf_index shouldn't be used, no?
you are right, it somehow moved to the next patch :(
I will send a fixed v4 after you finish review the series ..
>
>> +
>> + f->is_write = 1;
>
> is_write is completely redundant, and should just be a:
>
> f->ops->put_buffer test (or now with writev). We only set it up when
> there is anything to be written?
>
> But again, this is independent of this series.
>
I agree, maybe we need a cleanup series and remove it.
It will certainly simplify this code
Orit
- Re: [Qemu-devel] [PATCH v3 3/9] Update bytes_xfer in qemu_put_byte, (continued)
- [Qemu-devel] [PATCH v3 4/9] Store the data to send also in iovec, Orit Wasserman, 2013/03/21
- [Qemu-devel] [PATCH v3 5/9] Use writev ops if available, Orit Wasserman, 2013/03/21
- [Qemu-devel] [PATCH v3 6/9] More optimized qemu_put_be64/32/16, Orit Wasserman, 2013/03/21
- [Qemu-devel] [PATCH v3 7/9] Add qemu_put_buffer_no_copy, Orit Wasserman, 2013/03/21
- [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Orit Wasserman, 2013/03/21
- Re: [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Michael S. Tsirkin, 2013/03/21
- Re: [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Juan Quintela, 2013/03/21
- Re: [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Michael S. Tsirkin, 2013/03/21
- Re: [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Juan Quintela, 2013/03/21
- Re: [Qemu-devel] [PATCH v3 9/9] coalesce adjacent iovecs, Michael S. Tsirkin, 2013/03/21