Bug 6805 - errno should not be used for async io. Moving code, write_behind is broken ...
Summary: errno should not be used for async io. Moving code, write_behind is broken ...
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.4.2
Hardware: Other Linux
: P3 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-13 08:47 UTC by Olaf Flebbe
Modified: 2009-10-31 04:35 UTC (History)
0 users

See Also:
vl: review+


Attachments
Patch relative to v3-4-test (5.32 KB, patch)
2009-10-13 08:48 UTC, Olaf Flebbe
no flags Details
Patch relative to master (5.10 KB, text/x-patch)
2009-10-13 08:49 UTC, Olaf Flebbe
no flags Details
Slightly modified patch to correct the compile warning. (5.35 KB, patch)
2009-10-13 16:09 UTC, Jeremy Allison
no flags Details
Patch for 3.3.x. (5.61 KB, patch)
2009-10-13 18:51 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Olaf Flebbe 2009-10-13 08:47:43 UTC
Hi,

I had the problem that samba told me wrong error codes while debugging changes to async i/o code. It gave me an EINTR which was introduced by an select()  interrupted by an aio signal, rather the correct code.

Root of the problem:
The errno variable should not be used to retrieve error codes for async i/o requests. It should use the codes retrieved by aio_error() before.

See for instance 
http://books.google.de/books?id=4Kb_1sKprCMC&pg=PA317&lpg=PA317&dq=aio_error&source=bl&ots=v7iGD2z_LO&sig=liqepJUKRz1HgSX6sh_2PDWvsKU&hl=de&ei=LOzOSvH3HZOb_AaOv8XZAg&sa=X&oi=book_result&ct=result&resnum=8#v=onepage&q=aio_error&f=false

This requires to move the ECANCELED detection to a different place and to handle the error code from aio_error() correctly.

With this patch samba should display correct error codes. 

While inspecting the write_behind code I suspect that error handling for this is non-existing, it does not even report errors. So I placed a TODO line where error handling should happen.
Comment 1 Olaf Flebbe 2009-10-13 08:48:09 UTC
Created attachment 4833 [details]
Patch relative to v3-4-test
Comment 2 Olaf Flebbe 2009-10-13 08:49:46 UTC
Created attachment 4834 [details]
Patch relative to master
Comment 3 Jeremy Allison 2009-10-13 15:31:17 UTC
Perfectly correct - thanks ! Pushed to master and 3.5.0. I'll check the 3.4.x patch next.
Jeremy.
Comment 4 Jeremy Allison 2009-10-13 16:09:47 UTC
Created attachment 4840 [details]
Slightly modified patch to correct the compile warning.
Comment 5 Jeremy Allison 2009-10-13 18:51:37 UTC
Created attachment 4841 [details]
Patch for 3.3.x.

Patch for 3.3.x (too late for 3.3.9 - possible for 3.3.10 if we do another 3.3.x release).
Jeremy.
Comment 6 Jeremy Allison 2009-10-13 18:56:10 UTC
Volker please review the patch in attachment #4 [details] for possible inclusion in 3.4.3:

https://bugzilla.samba.org/attachment.cgi?id=4840&action=view

Please re-assign to Karolin if you agree. 3.4.3 code freeze is Oct. 15th so I think this can go in before that.

Thanks,

Jeremy.
Comment 7 Volker Lendecke 2009-10-14 04:14:55 UTC
+1 on both the 3.3 and 3.4 patches.

Volker
Comment 8 Karolin Seeger 2009-10-15 07:36:31 UTC
Pushed to v3-3-test and v3-4-test.
Closing out bug report.

Thanks!