[Qemu-devel] Missing system calls retries in case of EINTR

[Qemu-devel] Missing system calls retries in case of EINTR

[Yigael Fleishman]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

Running qemu on a Ubuntu Feisty host I've encountered an occasional
failure-to-launch problem.
I've used the "-serial /dev/ttyS0" at the command line, and qemu would often
exit printing "could not open serial device".
Looking in the srcs I've noticed that  qemu_chr_open_tty does not retry the
'open' syscall if it fails and errno=EINTR.
I've added a retry for such case and it seems to solve that specific problem
(see the following patch).
Furthermore, I've noticed that all open, read and some write calls in
vl.care not retried in such cases.
It seems to me retries should be implemented in those cases, however I might
still be missing something here.

--- /tmp/p4diff/vl.c#1.32670    2007-06-25 17:04:53.000000000 -0700
+++ vl.c        2007-06-25 17:04:29.000000000 -0700
@@ -1786,9 +1786,19 @@
     CharDriverState *chr;
     int fd;

-    fd = open(filename, O_RDWR | O_NONBLOCK);
-    if (fd < 0)
-        return NULL;
+    /* The following code wraps the 'open' syscall and retries it in case
it is interrupted. */
+    while (1)
+    {
+       fd = open(filename, O_RDWR | O_NONBLOCK );
+
+       if (fd < 0)
+       {
+           if (errno != EINTR)
+               return NULL;
+       }
+       else
+           break;
+    }
     fcntl(fd, F_SETFL, O_NONBLOCK);
     tty_serial_init(fd, 115200, 'N', 8, 1);
     chr = qemu_chr_open_fd(fd, fd);

[-- Attachment #2: Type: text/html, Size: 2145 bytes --]

Re: [Qemu-devel] Missing system calls retries in case of EINTR

[Rob Landley]

On Monday 25 June 2007 20:14:24 Yigael Fleishman wrote:
> Running qemu on a Ubuntu Feisty host I've encountered an occasional
> failure-to-launch problem.
> I've used the "-serial /dev/ttyS0" at the command line, and qemu would
> often exit printing "could not open serial device".
> Looking in the srcs I've noticed that  qemu_chr_open_tty does not retry the
> 'open' syscall if it fails and errno=EINTR.
> I've added a retry for such case and it seems to solve that specific
> problem (see the following patch).
> Furthermore, I've noticed that all open, read and some write calls in
> vl.care not retried in such cases.

Please don't play whack-a-mole with this.  You probably want 
sigaction(SA_RESTART) when running system emulation.

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

Re: [Qemu-devel] Missing system calls retries in case of EINTR

[Yigael Fleishman]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

Rob,
Thanks, that should also work.
Seems do_sigaction() in linux-user/signal.c has some code which does that
(though obviously not for the signal that I'm receiving).
Any idea why SA_RESTART calls are not invoked by default to restart syscalls
for all signals?
--Yigael


On 7/6/07, Rob Landley <rob@landley.net> wrote:
>
> On Monday 25 June 2007 20:14:24 Yigael Fleishman wrote:
> > Running qemu on a Ubuntu Feisty host I've encountered an occasional
> > failure-to-launch problem.
> > I've used the "-serial /dev/ttyS0" at the command line, and qemu would
> > often exit printing "could not open serial device".
> > Looking in the srcs I've noticed that  qemu_chr_open_tty does not retry
> the
> > 'open' syscall if it fails and errno=EINTR.
> > I've added a retry for such case and it seems to solve that specific
> > problem (see the following patch).
> > Furthermore, I've noticed that all open, read and some write calls in
> > vl.care not retried in such cases.
>
> Please don't play whack-a-mole with this.  You probably want
> sigaction(SA_RESTART) when running system emulation.
>
> Rob
> --
> "One of my most productive days was throwing away 1000 lines of code."
>   - Ken Thompson.
>

[-- Attachment #2: Type: text/html, Size: 1633 bytes --]

Re: [Qemu-devel] Missing system calls retries in case of EINTR

[Rob Landley]

Talking about sigaction(SA_RESTART) vs looping around -EINTR everywhere:

On Tuesday 10 July 2007 8:57:34 pm Yigael Fleishman wrote:
> Rob,
> Thanks, that should also work.
> Seems do_sigaction() in linux-user/signal.c has some code which does that
> (though obviously not for the signal that I'm receiving).
> Any idea why SA_RESTART calls are not invoked by default to restart
> syscalls for all signals?

Fallout from old 1980's-era Unixes that didn't implement SA_RESTART, 
basically.  Standards like SUSv3 say to humor them, because they paid for the 
standard...

http://www.ussg.iu.edu/hypermail/linux/kernel/0507.1/1065.html

> --Yigael

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

Re: [Qemu-devel] Missing system calls retries in case of EINTR

[Yigael Fleishman]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

Rob
Thanks for the background info. I wasn't very specific in my last question.
Actually I meant to ask why doesn't qemu code set sigaction(SA_RESTART) for
_all_ signals when running in system emulation mode (or maybe it does, which
means there's even larger problem...)
--Yigael




On 7/11/07, Rob Landley <rob@landley.net> wrote:
>
> Talking about sigaction(SA_RESTART) vs looping around -EINTR everywhere:
>
> On Tuesday 10 July 2007 8:57:34 pm Yigael Fleishman wrote:
> > Rob,
> > Thanks, that should also work.
> > Seems do_sigaction() in linux-user/signal.c has some code which does
> that
> > (though obviously not for the signal that I'm receiving).
> > Any idea why SA_RESTART calls are not invoked by default to restart
> > syscalls for all signals?
>
> Fallout from old 1980's-era Unixes that didn't implement SA_RESTART,
> basically.  Standards like SUSv3 say to humor them, because they paid for
> the
> standard...
>
> http://www.ussg.iu.edu/hypermail/linux/kernel/0507.1/1065.html
>
> > --Yigael
>
> Rob
> --
> "One of my most productive days was throwing away 1000 lines of code."
>   - Ken Thompson.
>

[-- Attachment #2: Type: text/html, Size: 1582 bytes --]