[buildd-tools-devel] [PATCH 06/17] [sbuild_session] Don't rollback non-run scripts
Jan-Marek Glogowski
glogow at fbihome.de
Tue Jun 30 18:05:44 UTC 2009
This just rolls back the scripts, which have run, including the
failed one.
This helps especially with the killprocs script. If a second
loopback session was started, but mount failed (as expected),
killprocs killed the processes from the current running session.
---
sbuild/sbuild-run-parts.cc | 60 +++++++++++----
sbuild/sbuild-run-parts.h | 16 +++-
sbuild/sbuild-session.cc | 177 ++++++++++++++++++++++++++++++++++++++++---
sbuild/sbuild-session.h | 33 +++++++-
test/sbuild-run-parts.cc | 18 +++-
5 files changed, 260 insertions(+), 44 deletions(-)
diff --git a/sbuild/sbuild-run-parts.cc b/sbuild/sbuild-run-parts.cc
index 86f251a..f70e049 100644
--- a/sbuild/sbuild-run-parts.cc
+++ b/sbuild/sbuild-run-parts.cc
@@ -120,9 +120,17 @@ run_parts::set_reverse (bool reverse)
int
run_parts::run (string_list const& command,
- environment const& env)
+ environment const& env,
+ std::string & failed_program)
{
- int exit_status = 0;
+ int exit_status = EXIT_FAILURE;
+ bool found_offset = failed_program.empty();
+
+ if (found_offset && this->programs.empty())
+ return EXIT_SUCCESS;
+
+ string_list real_command = command;
+ real_command.insert( real_command.begin(), std::string() );
if (!this->reverse)
{
@@ -130,17 +138,27 @@ run_parts::run (string_list const& command,
pos != this->programs.end();
++pos)
{
- string_list real_command;
- real_command.push_back(*pos);
- for (string_list::const_iterator spos = command.begin();
- spos != command.end();
- ++spos)
- real_command.push_back(*spos);
+ if (!found_offset)
+ {
+ if (failed_program == *pos)
+ {
+ found_offset = true;
+ failed_program.clear();
+ exit_status = EXIT_SUCCESS;
+ }
+ else
+ continue;
+ }
+
+ *(real_command.begin()) = *pos;
exit_status = run_child(*pos, real_command, env);
if (exit_status && this->abort_on_error)
- return exit_status;
+ {
+ failed_program = *pos;
+ break;
+ }
}
}
else
@@ -149,17 +167,27 @@ run_parts::run (string_list const& command,
pos != this->programs.rend();
++pos)
{
- string_list real_command;
- real_command.push_back(*pos);
- for (string_list::const_iterator spos = command.begin();
- spos != command.end();
- ++spos)
- real_command.push_back(*spos);
+ if (!found_offset)
+ {
+ if (failed_program == *pos)
+ {
+ found_offset = true;
+ failed_program.clear();
+ exit_status = EXIT_SUCCESS;
+ }
+ else
+ continue;
+ }
+
+ *(real_command.begin()) = *pos;
exit_status = run_child(*pos, real_command, env);
if (exit_status && this->abort_on_error)
- return exit_status;
+ {
+ failed_program = *pos;
+ break;
+ }
}
}
diff --git a/sbuild/sbuild-run-parts.h b/sbuild/sbuild-run-parts.h
index 7b5b6e6..4fcb338 100644
--- a/sbuild/sbuild-run-parts.h
+++ b/sbuild/sbuild-run-parts.h
@@ -108,17 +108,23 @@ namespace sbuild
set_reverse (bool reverse);
/**
- * Run all scripts in the specified directory. If abort_on_error
- * is true, execution will stop at the first script to fail.
+ * Run all scripts in the specified directory.
+ *
+ * If abort_on_error is true, execution will stop at the first script
+ * to fail. If failed_program is set, all program execution is skipped
+ * until the specified program.
*
* @param command the command to run.
* @param env the environment to use.
- * @returns the exit status of the scripts. This will be 0 on
- * success, or the exit status of the last failing script.
+ * @param failed_program will be set, if abort_on_error is true
+ * @returns the exit status of the scripts. This will be 0 on success,
+ * or the exit status of the last failing script. If failed_program is
+ * set but not found, exit status is failed.
*/
int
run(string_list const& command,
- environment const& env);
+ environment const& env,
+ std::string & failed_program);
/**
* Output the environment to an ostream.
diff --git a/sbuild/sbuild-session.cc b/sbuild/sbuild-session.cc
index 379b051..e0c7143 100644
--- a/sbuild/sbuild-session.cc
+++ b/sbuild/sbuild-session.cc
@@ -76,6 +76,7 @@ namespace
emap(session::CHILD_CORE, N_("Child dumped core")),
emap(session::CHILD_FAIL, N_("Child exited abnormally (reason unknown; not a signal or core dump)")),
emap(session::CHILD_FORK, N_("Failed to fork child")),
+ emap(session::CHILD_PIPE, N_("Failed to create pipe for child communication")),
// TRANSLATORS: %4% = signal name
emap(session::CHILD_SIGNAL, N_("Child terminated by signal '%4%'")),
emap(session::CHILD_WAIT, N_("Wait for child failed")),
@@ -1087,6 +1088,13 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
int exit_status = 0;
pid_t pid;
+ int pipefd[2];
+
+ if (pipe(pipefd) == -1)
+ {
+ this->chroot_status = false;
+ throw error(session_chroot->get_name(), CHILD_PIPE, strerror(errno));
+ }
if ((pid = fork()) == -1)
{
@@ -1095,12 +1103,19 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
}
else if (pid == 0)
{
+ int status = EXIT_FAILURE;
+
+ // Close read end
+ close(pipefd[0]);
+
try
{
// The setup scripts don't use our syslog fd.
closelog();
- chdir("/");
+ if (chdir ("/") != 0)
+ throw error("/", CHDIR, strerror(errno));
+
/* This is required to ensure the scripts run with uid=0 and gid=0,
otherwise setuid programs such as mount(8) will fail. This
should always succeed, because our euid=0 and egid=0.*/
@@ -1108,9 +1123,24 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
setgid(0);
initgroups("root", 0);
- int status = rp.run(arg_list, env);
-
- _exit (status);
+ status = rp.run(arg_list, env, this->failed_program);
+ if (status != EXIT_SUCCESS)
+ {
+ ssize_t offset = 0;
+ ssize_t left = this->failed_program.length();
+ while (left > 0)
+ {
+ ssize_t count = write(pipefd[1],
+ this->failed_program.data() + offset, left);
+ if (count == -1)
+ left = 0;
+ else if (count > 0)
+ {
+ left -= count;
+ offset += count;
+ }
+ }
+ }
}
catch (std::exception const& e)
{
@@ -1121,11 +1151,22 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
sbuild::log_error()
<< _("An unknown exception occurred") << std::endl;
}
- _exit(EXIT_FAILURE);
+
+ // Close write end
+ close(pipefd[1]);
+
+ _exit(status);
}
else
{
- wait_for_child(pid, exit_status);
+ // Close write end
+ close(pipefd[1]);
+
+ this->failed_program.clear();
+ wait_for_child_data(pid, exit_status, pipefd[0], this->failed_program);
+
+ // Close read end
+ close(pipefd[0]);
}
try
@@ -1180,7 +1221,7 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
<< session_chroot->get_persona()<< std::endl;
/* Enter the chroot */
- if (chdir (location.c_str()))
+ if (chdir (location.c_str()) != 0)
throw error(location, CHDIR, strerror(errno));
log_debug(DEBUG_NOTICE) << "Changed directory to " << location << std::endl;
if (::chroot (location.c_str()))
@@ -1213,7 +1254,7 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
dpos != dlist.end();
++dpos)
{
- if (chdir ((*dpos).c_str()) < 0)
+ if (chdir ((*dpos).c_str()) != 0)
{
error e(*dpos, CHDIR, strerror(errno));
@@ -1280,12 +1321,12 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
}
void
-session::wait_for_child (pid_t pid,
- int& child_status)
+session::wait_for_child (pid_t pid,
+ int& child_status)
{
child_status = EXIT_FAILURE; // Default exit status
- int status;
+ int status, wait_status;
bool child_killed = false;
while (1)
@@ -1310,12 +1351,15 @@ session::wait_for_child (pid_t pid,
child_killed = true;
}
- if (waitpid(pid, &status, 0) == -1)
+ wait_status = waitpid(pid, &status, 0);
+ if (wait_status == -1)
{
if (errno == EINTR && (sighup_called || sigterm_called))
continue; // Kill child and wait again.
else
- throw error(CHILD_WAIT, strerror(errno));
+ {
+ throw error(CHILD_WAIT, strerror(errno));
+ }
}
else if (sighup_called)
{
@@ -1344,6 +1388,113 @@ session::wait_for_child (pid_t pid,
child_status = WEXITSTATUS(status);
}
+static void
+read_and_close (int read_fd,
+ std::string& data,
+ bool close_fd = true)
+{
+ char buffer[NAME_MAX];
+
+ if (read_fd <= -1)
+ return;
+
+ while (1)
+ {
+ ssize_t count = read(read_fd, buffer, NAME_MAX);
+ if (count <= 0)
+ break;
+ data.append(buffer, count);
+ }
+
+ if (close_fd)
+ close(read_fd);
+}
+
+void
+session::wait_for_child_data (pid_t pid,
+ int& child_status,
+ int read_fd,
+ std::string& data)
+{
+ child_status = EXIT_FAILURE; // Default exit status
+
+ int status, wait_status;
+ bool child_killed = false;
+ int wait_option = (read_fd > -1) ? WNOHANG : 0;
+
+ data.clear();
+
+ while (1)
+ {
+ if ((sighup_called || sigterm_called) && !child_killed)
+ {
+ if (sighup_called)
+ {
+ error e(SIGNAL_CATCH, strsignal(SIGHUP),
+ _("terminating immediately"));
+ log_exception_error(e);
+ kill(pid, SIGHUP);
+ }
+ else // SIGTERM
+ {
+ error e(SIGNAL_CATCH, strsignal(SIGTERM),
+ _("terminating immediately"));
+ log_exception_error(e);
+ kill(pid, SIGTERM);
+ }
+ this->chroot_status = false;
+ child_killed = true;
+ }
+
+ wait_status = waitpid(pid, &status, wait_option);
+ if (wait_status == -1)
+ {
+ if (errno == EINTR && (sighup_called || sigterm_called))
+ continue; // Kill child and wait again.
+ else
+ {
+ read_and_close(read_fd, data);
+ throw error(CHILD_WAIT, strerror(errno));
+ }
+ }
+ else if (sighup_called)
+ {
+ sighup_called = false;
+ read_and_close(read_fd, data);
+ throw error(SIGNAL_CATCH, strsignal(SIGHUP));
+ }
+ else if (sigterm_called)
+ {
+ sigterm_called = false;
+ read_and_close(read_fd, data);
+ throw error(SIGNAL_CATCH, strsignal(SIGTERM));
+ }
+ else if (wait_option)
+ {
+ if (wait_status == pid)
+ break;
+
+ read_and_close(read_fd, data, false);
+ }
+ else
+ break;
+ }
+
+ read_and_close(read_fd, data);
+
+ if (!WIFEXITED(status))
+ {
+ if (WIFSIGNALED(status))
+ throw error(CHILD_SIGNAL, strsignal(WTERMSIG(status)));
+ else if (WCOREDUMP(status))
+ throw error(CHILD_CORE);
+ else
+ throw error(CHILD_FAIL);
+ }
+
+ child_status = WEXITSTATUS(status);
+}
+
void
session::run_chroot (sbuild::chroot::ptr& session_chroot)
{
diff --git a/sbuild/sbuild-session.h b/sbuild/sbuild-session.h
index f3f2ed7..c9c8985 100644
--- a/sbuild/sbuild-session.h
+++ b/sbuild/sbuild-session.h
@@ -64,6 +64,7 @@ namespace sbuild
CHILD_CORE, ///< Child dumped core.
CHILD_FAIL, ///< Child exited abnormally (reason unknown)
CHILD_FORK, ///< Failed to fork child.
+ CHILD_PIPE, ///< Failed to create pipe for child communication.
CHILD_SIGNAL, ///< Child terminated by signal.
CHILD_WAIT, ///< Wait for child failed.
CHROOT, ///< Failed to change root to directory.
@@ -389,8 +390,28 @@ namespace sbuild
* @param child_status the place to store the child exit status.
*/
void
- wait_for_child (pid_t pid,
- int& child_status);
+ wait_for_child (pid_t pid,
+ int& child_status);
+
+ /**
+ * Wait for a child process to complete, check its exit status and
+ * get the data from the childs pipe.
+ *
+ * Reads data from a child pipe while waiting and stores it in the
+ * std::string @data.
+ *
+ * An error will be thrown on failure.
+ *
+ * @param pid the pid to wait for.
+ * @param child_status the place to store the child exit status.
+ * @param read_fd readable pipe fd to get data from.
+ * @param data reference to a std:string to store the data.
+ */
+ void
+ wait_for_child_data (pid_t pid,
+ int& child_status,
+ int read_fd,
+ std::string& data);
/**
* Set the SIGHUP handler.
@@ -453,7 +474,7 @@ namespace sbuild
/// The current chroot status.
int chroot_status;
/// Lock status for locks acquired during chroot setup.
- bool lock_status;
+ bool lock_status;
/// The child exit status.
int child_status;
/// The session operation to perform.
@@ -467,9 +488,11 @@ namespace sbuild
/// Signal saved while sigterm handler is set.
struct sigaction saved_sigterm_signal;
/// Saved terminal settings.
- struct termios saved_termios;
+ struct termios saved_termios;
/// Are the saved terminal settings valid?
- bool termios_ok;
+ bool termios_ok;
+ /// Last failed program / script
+ std::string failed_program;
protected:
/// Current working directory.
diff --git a/test/sbuild-run-parts.cc b/test/sbuild-run-parts.cc
index 208337c..8ba878d 100644
--- a/test/sbuild-run-parts.cc
+++ b/test/sbuild-run-parts.cc
@@ -80,22 +80,26 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_SUCCESS);
+ CPPUNIT_ASSERT(failed_program.empty());
command.clear();
command.push_back("fail");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "20test2");
command.clear();
command.push_back("fail2");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "30test3");
}
void test_run2()
@@ -104,12 +108,14 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_SUCCESS);
+ CPPUNIT_ASSERT(failed_program.empty());
}
void test_run3()
@@ -118,12 +124,14 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "50invalid");
}
};
--
1.6.3.2
More information about the Buildd-tools-devel
mailing list