Re: [PATCH] qemu: Report errors from iohelper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]