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 "
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list