Re: [Qemu-devel] [PATCH] qemu-char: reset errno before qemu char write or read action

[xinhua.Cao <caoxinhua@huawei.com>]

在 2018/7/2 16:46, Paolo Bonzini 写道:
> On 02/07/2018 02:49, xinhua.Cao wrote:
>> In the tcp_chr_write function, we checked errno,
>> but errno was not reset before a read or write operation.
>> Therefore, this check of errno's actions is often
>> incorrect after EAGAIN has occurred.
>> We reset errno before reading and writing to
>> ensure the correctness of errno's judgment
> You should explain why this is a problem, because all the places you
> modified are checking that the read or write has returned -1.  In that
> case, errno must have been modified and it is unnecessary to write 0.
>
> Thanks,
>
> Paolo
We found this problem on qemu-2.6. At that time,
we backport the patch 9fc53a10 to qemu-2.6 and
found that when the virtual machine was started,
the fds of the ovs process increased a lot.
we check tcp_chr_write function, it is found
that errno is not reset. Therefore, when errno is
set to EAGAIN, write_msgfds will not be free subsequently.
In the qemu-2.6 version, another free write_msgfds is in vhost_user_write.
Vhost_user_write in qemu-2.6 check fd_num before calling 
qemu_chr_fe_set_msgfds.
fd_num is 0 in many cases, so it won't be cleaned up here.
There have been a lot of cases of sending fds to ovs.

Thanks,

xinhua.Cao
>
>> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
>> ---
>>   chardev/char-fe.c | 1 +
>>   chardev/char.c    | 2 ++
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
>> index b1f228e..d96ca6f 100644
>> --- a/chardev/char-fe.c
>> +++ b/chardev/char-fe.c
>> @@ -69,6 +69,7 @@ int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len)
>>   
>>       while (offset < len) {
>>       retry:
>> +        errno = 0;
>>           res = CHARDEV_GET_CLASS(s)->chr_sync_read(s, buf + offset,
>>                                                     len - offset);
>>           if (res == -1 && errno == EAGAIN) {
>> diff --git a/chardev/char.c b/chardev/char.c
>> index 76d866e..3387442 100644
>> --- a/chardev/char.c
>> +++ b/chardev/char.c
>> @@ -85,6 +85,7 @@ static void qemu_chr_write_log(Chardev *s, const uint8_t *buf, size_t len)
>>   
>>       while (done < len) {
>>       retry:
>> +        errno = 0;
>>           ret = write(s->logfd, buf + done, len - done);
>>           if (ret == -1 && errno == EAGAIN) {
>>               g_usleep(100);
>> @@ -109,6 +110,7 @@ static int qemu_chr_write_buffer(Chardev *s,
>>       qemu_mutex_lock(&s->chr_write_lock);
>>       while (*offset < len) {
>>       retry:
>> +        errno = 0;
>>           res = cc->chr_write(s, buf + *offset, len - *offset);
>>           if (res < 0 && errno == EAGAIN && write_all) {
>>               g_usleep(100);
>>
>
> .
>