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
