[libvirt] [PATCHv2 2/2] remote: react to failures on wakeupFD
Jim Meyering
jim at meyering.net
Thu Apr 15 17:25:17 UTC 2010
Eric Blake wrote:
> discussion: https://www.redhat.com/archives/libvir-list/2010-March/msg00443.html
>
> * src/remote/remote_driver.c (remoteIO, remoteIOEventLoop): Report
> failures on pipe used for wakeup.
> Reported by Chris Lalancette.
> ---
>
>> > - ignore_value(saferead(priv->wakeupReadFD, &ignore,
>> > - sizeof(ignore)));
>> > + if (saferead(priv->wakeupReadFD, &ignore, sizeof(ignore))
>> > + != sizeof(ignore)) {
>> > + virReportSystemError(errno ? errno : 0,
>> This looks fine, but "errno ? errno : 0" is equivalent to just "errno".
>> Which makes me think you'll want to separate the saferead-fails case
>> (where errno is nonzero) from the
>> saferead-returns-non-negative-<=-sizeof-ignore case (in which
>> case virReportSystemError would print "Success" for errno=0).
>
> Is this rewrite more what you had in mind? (No change to patch 1/2).
Thanks.
That looks better. No more "Success" diagnostic upon error.
> src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++----
> 1 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index ebcfcd8..e3df27b 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -9523,9 +9523,18 @@ remoteIOEventLoop(virConnectPtr conn,
> remoteDriverLock(priv);
>
> if (fds[1].revents) {
> + ssize_t s;
> DEBUG0("Woken up from poll by other thread");
> - ignore_value(saferead(priv->wakeupReadFD, &ignore,
> - sizeof(ignore)));
> + s = saferead(priv->wakeupReadFD, &ignore, sizeof(ignore));
> + if (s < 0) {
> + virReportSystemError(errno, "%s",
> + _("read on wakeup fd failed"));
> + goto error;
> + } else if (s != sizeof(ignore)) {
> + remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("read on wakeup fd failed"));
> + goto error;
> + }
> }
>
> if (ret < 0) {
> @@ -9661,6 +9670,7 @@ remoteIO(virConnectPtr conn,
> /* Stick ourselves on the end of the wait queue */
> struct remote_thread_call *tmp = priv->waitDispatch;
> char ignore = 1;
> + ssize_t s;
> while (tmp && tmp->next)
> tmp = tmp->next;
> if (tmp)
> @@ -9668,8 +9678,21 @@ remoteIO(virConnectPtr conn,
> else
> priv->waitDispatch = thiscall;
>
> - /* Force other thread to wakup from poll */
> - ignore_value(safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore)));
> + /* Force other thread to wakeup from poll */
> + s = safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore));
> + if (s < 0) {
> + char errout[1024];
> + remoteError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to wake up polling thread: %s"),
> + virStrerror(errno, errout, sizeof errout));
> + VIR_FREE(thiscall);
> + return -1;
> + } else if (s != sizeof(ignore)) {
> + remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to wake up polling thread"));
> + VIR_FREE(thiscall);
> + return -1;
> + }
>
> DEBUG("Going to sleep %d %p %p", thiscall->proc_nr, priv->waitDispatch, thiscall);
> /* Go to sleep while other thread is working... */
More information about the libvir-list
mailing list