[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] char: fix broken EAGAIN retry on OS-X due t
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] char: fix broken EAGAIN retry on OS-X due to errno clobbering |
Date: |
Thu, 31 Mar 2016 15:35:44 +0100 |
On 31 March 2016 at 15:29, Daniel P. Berrange <address@hidden> wrote:
> Some of the chardev I/O paths really want to write the
> complete data buffer even though the channel is in
> non-blocking mode. To achieve this they look for EAGAIN
> and g_usleep() for 100ms. Unfortunately the code is set
> to check errno == EAGAIN a second time, after the g_usleep()
> call has completed. On OS-X at least, g_usleep clobbers
> errno to ETIMEDOUT, causing the retry to be skipped.
>
> This failure to retry means the full data isn't written
> to the chardev backend, which causes various failures
> including making the tests/ahci-test qtest hang.
>
> Rather than playing games trying to reset errno just
> simplify the code to use a goto to retry instead of a
> a loop.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> qemu-char.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 270819a..6e623c3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -246,12 +246,12 @@ static int qemu_chr_fe_write_buffer(CharDriverState *s,
> const uint8_t *buf, int
>
> qemu_mutex_lock(&s->chr_write_lock);
> while (*offset < len) {
> - do {
> - res = s->chr_write(s, buf + *offset, len - *offset);
> - if (res == -1 && errno == EAGAIN) {
> - g_usleep(100);
> - }
> - } while (res == -1 && errno == EAGAIN);
> + retry:
> + res = s->chr_write(s, buf + *offset, len - *offset);
> + if (res < 0 && errno == EAGAIN) {
> + g_usleep(100);
> + goto retry;
> + }
>
> if (res <= 0) {
> break;
> @@ -333,12 +333,12 @@ int qemu_chr_fe_read_all(CharDriverState *s, uint8_t
> *buf, int len)
> }
>
> while (offset < len) {
> - do {
> - res = s->chr_sync_read(s, buf + offset, len - offset);
> - if (res == -1 && errno == EAGAIN) {
> - g_usleep(100);
> - }
> - } while (res == -1 && errno == EAGAIN);
> + retry:
> + res = s->chr_sync_read(s, buf + offset, len - offset);
> + if (res == -1 && errno == EAGAIN) {
> + g_usleep(100);
> + goto retry;
> + }
>
> if (res == 0) {
> break;
qemu_chr_fe_write_log() also seems to have this broken retry pattern.
thanks
-- PMM