qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 08/18] migration (outgoing): add error propagati


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 08/18] migration (outgoing): add error propagation for fd and exec protocols
Date: Fri, 05 Oct 2012 08:25:46 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1

Il 04/10/2012 20:24, Luiz Capitulino ha scritto:
> That DPRINTF() usage is really bizarre, it seems its purpose is to report
> an error to the user, but that's a debugging call.
> 
> I'd let it there and replace it later with proper tracing code, but that's
> quite minor for me. Please, at least mention you're dropping it in the log.

This one is not dropped, it becomes the reported error message.

>> >          goto err_after_popen;
>> >      }
>> >  
>> >      s->fd = fileno(f);
>> >      if (s->fd == -1) {
>> > -        DPRINTF("Unable to retrieve file descriptor for popen'd 
>> > handle\n");

This one is dropped, but I wanted to delete the check altogether.
fileno() should only fail if it detects somehow that its argument is not
a valid stream, which is obviously not the case.

Would that be better?  It would also fix the clobbering of errno.

>> >          goto err_after_open;
>> >      }
>> >  
>> > @@ -85,12 +83,12 @@ int exec_start_outgoing_migration(MigrationState *s, 
>> > const char *command)
>> >      s->write = file_write;
>> >  
>> >      migrate_fd_connect(s);
>> > -    return 0;
>> > +    return;
>> >  
>> >  err_after_open:
>> >      pclose(f);
>> >  err_after_popen:
>> > -    return -1;
>> > +    error_setg_errno(errp, errno, "failed to popen the migration target");
> The pclose() call will override errno.

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]