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