Re: [libvirt] [PATCH] qemu: Report errors from iohelper
On 25.10.2012 23:56, Eric Blake wrote:
On 10/24/2012 09:49 AM, Michal Privoznik wrote:
> Currently, we use iohelper when saving/restoring a domain.
> However, if there's some kind of error (like I/O) it is not
> propagated to libvirt. Since it is not qemu who is doing
> the actual write() it will not get error. The iohelper does.
> Therefore we should check for iohelper errors as it makes
> libvirt more user friendly.
> ---
>
> diff to v1:
> -incorporate the event loop to read iohelper's stderr
> @@ -3282,9 +3283,11 @@ doCoreDump(struct qemud_driver *driver,
>
> + /* deliberately don't use virCommandNonblockingFDs here as it is all or
> + * nothing. And we want iohelper's stdin to block. */
> + if (virSetNonBlock(ret->err_fd) < 0) {
This comment is confusing, since you say we don't use nonblocking and
then immediately set our side of stderr to nonblocking. Maybe you meant
to say that we leave helper's stdin or stdout alone (default blocking)
because they are not tied to our event loop, but set our side of stderr
to be nonblocking so we can read it from our event loop.
ACK once you fix up that comment.
Thanks, pushed with this:
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 36b3420..9593151 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -291,7 +291,9 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
goto error;
/* deliberately don't use virCommandNonblockingFDs here as it is all or
- * nothing. And we want iohelper's stdin to block. */
+ * nothing. And we want iohelper's stdin and stdout to block (default).
+ * However, stderr is read within event loop and therefore it must be
+ * nonblocking.*/
if (virSetNonBlock(ret->err_fd) < 0) {
virReportSystemError(errno, "%s",
_("Failed to set non-blocking "