> On (Wed) Dec 01 2010 [11:59:35], Paul Brook wrote: > > > > > - qemu_chr_write(vcon->chr, buf, len); > > > > > + ret = qemu_chr_write(vcon->chr, buf, len); > > > > > + if (ret == -EAGAIN) { > > > > > + virtio_serial_throttle_port(port, true); > > > > > + } > > > > > > > > > > } > > > > > > > > This looks wrong. It will loose data in the case of a partial write > > > > (i.e. ret < len) > > > > > > That doesn't happen currently (qemu_chr_write doesn't return a value > > > > 0 but < len). > > > > > > I had code in there to handle it, but that would change behaviour for > > > current users of qemu_chr_write(), which is a risk. > > > > Doesn't that make the code almost completely pointless? > > Not really -- I did have code for partial writes, but removed it before > this submission (had it in previous versions). > > The (new) do_send loop: > > len = len1; > while (len > 0) { > ret = write(fd, buf, len); > if (ret < 0) { > if (errno == EAGAIN && nonblock) { > return -EAGAIN; > } > if (errno != EINTR && errno != EAGAIN) { > return -1; > } > } else if (ret == 0) { > break; > } else { > buf += ret; > len -= ret; > } > } > > when there's a partial write, it tries to do a write again, which will > fail with -EAGAIN.
Doesn't that cause the first partial chunk to be incorrectly transmitted twice? You may only return EAGAIN if no data was transmitted. Paul