Comment 4 for bug 1978955

Revision history for this message
Robie Basak (racb) wrote :

It seems odd to me to retry only up to five times. The usual way to handle EINTR is to retry indefinitely, and if the process were to keep receiving signals such that it couldn't make any progress at all, then would be expected behaviour here, and a bug in whatever is keeping up such a signal rate. This is what the (GNU-specific) TEMP_FAILURE_RETRY does, for example: https://www.gnu.org/software/libc/manual/html_node/Interrupted-Primitives.html

So I think the upstream fix could do with improving here.

But in the meantime I think it's fine to grab the upstream fix as-is, as it's unlikely to make a difference in this case.

I also noticed that the commit that actually landed upstream (https://github.com/ClusterLabs/libqb/commit/176eae8f13278a5a3dab3699b84e1dc9a8d4ae11) slightly mismatches the patch uploaded here (https://github.com/ClusterLabs/libqb/commit/2e259c9d6e13968665678745f1e774bc4ccf8806). But they look functionally equivalent in this case, so I think it's probably not worth fixing now.

There's a minor change in the EINTR failure case here. Before, the code would error unwind. Now, it continues regardless even when the retry count is exceeded. But I think that's also fine in this case because the nature of posix_fallocate is advisory. There's a minor difference in that a disk full error might then happen later, but that seems unlikely to result in a regression to me.