Merge "Adds a -P option so dumpstate can report its progress."
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index b5931d7..f8ea394 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -602,6 +602,7 @@
" -e: play sound file instead of vibrate, at end of job\n"
" -q: disable vibrate\n"
" -B: send broadcast when finished (requires -o)\n"
+ " -P: send broadacast when started and update system properties on progress (requires -o and -B)\n"
);
}
@@ -712,16 +713,17 @@
/* parse arguments */
int c;
- while ((c = getopt(argc, argv, "dho:svqzpB")) != -1) {
+ while ((c = getopt(argc, argv, "dho:svqzpPB")) != -1) {
switch (c) {
- case 'd': do_add_date = 1; break;
- case 'z': do_zip_file = 1; break;
- case 'o': use_outfile = optarg; break;
- case 's': use_socket = 1; break;
+ case 'd': do_add_date = 1; break;
+ case 'z': do_zip_file = 1; break;
+ case 'o': use_outfile = optarg; break;
+ case 's': use_socket = 1; break;
case 'v': break; // compatibility no-op
- case 'q': do_vibrate = 0; break;
- case 'p': do_fb = 1; break;
- case 'B': do_broadcast = 1; break;
+ case 'q': do_vibrate = 0; break;
+ case 'p': do_fb = 1; break;
+ case 'P': do_update_progress = 1; break;
+ case 'B': do_broadcast = 1; break;
case '?': printf("\n");
case 'h':
usage();
@@ -729,11 +731,15 @@
}
}
- if ((do_zip_file || do_add_date || do_broadcast) && !use_outfile) {
+ if ((do_zip_file || do_add_date || do_update_progress || do_broadcast) && !use_outfile) {
usage();
exit(1);
}
+ if (do_update_progress && !do_broadcast) {
+ usage();
+ exit(1);
+ }
// If we are going to use a socket, do it as early as possible
// to avoid timeouts from bugreport.
@@ -741,6 +747,49 @@
redirect_to_socket(stdout, "dumpstate");
}
+ /* redirect output if needed */
+ std::string text_path, zip_path, tmp_path, entry_name;
+
+ /* pointer to the actual path, be it zip or text */
+ std::string path;
+
+ time_t now = time(NULL);
+
+ bool is_redirecting = !use_socket && use_outfile;
+
+ if (is_redirecting) {
+ text_path = use_outfile;
+ if (do_add_date) {
+ char date[80];
+ strftime(date, sizeof(date), "-%Y-%m-%d-%H-%M-%S", localtime(&now));
+ text_path += date;
+ }
+ if (do_fb) {
+ screenshot_path = text_path + ".png";
+ }
+ zip_path = text_path + ".zip";
+ text_path += ".txt";
+ tmp_path = text_path + ".tmp";
+ entry_name = basename(text_path.c_str());
+
+ ALOGD("Temporary path: %s\ntext path: %s\nzip path: %s\nzip entry: %s",
+ tmp_path.c_str(), text_path.c_str(), zip_path.c_str(), entry_name.c_str());
+
+ if (do_update_progress) {
+ if (!entry_name.empty()) {
+ std::vector<std::string> am_args = {
+ "--receiver-permission", "android.permission.DUMP",
+ "--es", "android.intent.extra.NAME", entry_name,
+ "--ei", "android.intent.extra.PID", std::to_string(getpid()),
+ "--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL),
+ };
+ send_broadcast("android.intent.action.BUGREPORT_STARTED", am_args);
+ } else {
+ ALOGE("Skipping started broadcast because entry name could not be inferred\n");
+ }
+ }
+ }
+
/* open the vibrator before dropping root */
std::unique_ptr<FILE, int(*)(FILE*)> vibrator(NULL, fclose);
if (do_vibrate) {
@@ -802,29 +851,7 @@
return -1;
}
- /* redirect output if needed */
- std::string text_path, zip_path, tmp_path, entry_name;
-
- /* pointer to the actual path, be it zip or text */
- std::string path;
-
- time_t now = time(NULL);
-
- if (!use_socket && use_outfile) {
- text_path = use_outfile;
- if (do_add_date) {
- char date[80];
- strftime(date, sizeof(date), "-%Y-%m-%d-%H-%M-%S", localtime(&now));
- text_path += date;
- }
- if (do_fb) {
- screenshot_path = text_path + ".png";
- }
- zip_path = text_path + ".zip";
- text_path += ".txt";
- tmp_path = text_path + ".tmp";
- entry_name = basename(text_path.c_str());
-
+ if (is_redirecting) {
ALOGD("Temporary path: %s\ntext path: %s\nzip path: %s\nzip entry: %s",
tmp_path.c_str(), text_path.c_str(), zip_path.c_str(), entry_name.c_str());
/* TODO: rather than generating a text file now and zipping it later,
@@ -844,7 +871,7 @@
}
/* close output if needed */
- if (!use_socket && use_outfile) {
+ if (is_redirecting) {
fclose(stdout);
}
@@ -870,11 +897,12 @@
}
/* tell activity manager we're done */
- if (do_broadcast && use_outfile) {
+ if (do_broadcast) {
if (!path.empty()) {
ALOGI("Final bugreport path: %s\n", path.c_str());
std::vector<std::string> am_args = {
"--receiver-permission", "android.permission.DUMP",
+ "--ei", "android.intent.extra.PID", std::to_string(getpid()),
"--es", "android.intent.extra.BUGREPORT", path
};
if (do_fb) {
@@ -884,7 +912,7 @@
}
send_broadcast("android.intent.action.BUGREPORT_FINISHED", am_args);
} else {
- ALOGE("Skipping broadcast because bugreport could not be generated\n");
+ ALOGE("Skipping finished broadcast because bugreport could not be generated\n");
}
}
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 18ee168..2ba5ccb 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -45,6 +45,28 @@
typedef void (for_each_pid_func)(int, const char *);
typedef void (for_each_tid_func)(int, int, const char *);
+/* Estimated total weight of bugreport generation.
+ *
+ * Each section contributes to the total weight by an individual weight, so the overall progress
+ * can be calculated by dividing the all completed weight by the total weight.
+ *
+ * This value is defined empirically and it need to be adjusted as more sections are added.
+ * It does not need to match the exact sum of all sections, but it should to be more than it,
+ * otherwise the calculated progress would be more than 100%.
+ */
+static const int WEIGHT_TOTAL = 4000;
+
+/* Most simple commands have 10 as timeout, so 5 is a good estimate */
+static const int WEIGHT_FILE = 5;
+
+/*
+ * TOOD: the dumpstate internal state is getting fragile; for example, this variable is defined
+ * here, declared at utils.cpp, and used on utils.cpp and dumpstate.cpp.
+ * It would be better to take advantage of the C++ migration and encapsulate the state in an object,
+ * but that will be better handled in a major C++ refactoring, which would also get rid of other C
+ * idioms (like using std::string instead of char*, removing varargs, etc...) */
+extern int do_update_progress;
+
/* prints the contents of a file */
int dump_file(const char *title, const char *path);
@@ -74,6 +96,9 @@
/* sends a broadcast using Activity Manager */
void send_broadcast(const std::string& action, const std::vector<std::string>& args);
+/* updates the overall progress of dumpstate by the given weight increment */
+void update_progress(int weight);
+
/* prints all the system properties */
void print_properties();
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 4316c96..0958aef 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -35,7 +35,9 @@
#include <vector>
#include <sys/prctl.h>
+#define LOG_TAG "dumpstate"
#include <cutils/debugger.h>
+#include <cutils/log.h>
#include <cutils/properties.h>
#include <cutils/sockets.h>
#include <private/android_filesystem_config.h>
@@ -266,7 +268,7 @@
}
printf(") ------\n");
}
- ON_DRY_RUN({ close(fd); return 0; });
+ ON_DRY_RUN({ update_progress(WEIGHT_FILE); close(fd); return 0; });
bool newline = false;
fd_set read_set;
@@ -304,6 +306,7 @@
}
}
}
+ update_progress(WEIGHT_FILE);
close(fd);
if (!newline) printf("\n");
@@ -455,7 +458,6 @@
return true;
}
-/* forks a command and waits for it to finish */
int run_command(const char *title, int timeout_seconds, const char *command, ...) {
fflush(stdout);
@@ -472,13 +474,19 @@
if (title) printf(") ------\n");
fflush(stdout);
- ON_DRY_RUN_RETURN(0);
+ ON_DRY_RUN({ update_progress(timeout_seconds); va_end(ap); return 0; });
- return run_command_always(title, timeout_seconds, args);
+ int status = run_command_always(title, timeout_seconds, args);
+ va_end(ap);
+ return status;
}
/* forks a command and waits for it to finish */
int run_command_always(const char *title, int timeout_seconds, const char *args[]) {
+ /* TODO: for now we're simplifying the progress calculation by using the timeout as the weight.
+ * It's a good approximation for most cases, except when calling dumpsys, where its weight
+ * should be much higher proportionally to its timeout. */
+ int weight = timeout_seconds;
const char *command = args[0];
uint64_t start = nanotime();
@@ -537,6 +545,9 @@
}
if (title) printf("[%s: %.3fs elapsed]\n\n", command, (float)elapsed / NANOS_PER_SEC);
+ if (weight > 0) {
+ update_progress(weight);
+ }
return status;
}
@@ -826,3 +837,29 @@
}
fclose(fp);
}
+
+/* overall progress */
+int progress = 0;
+int do_update_progress = 1; // Set by dumpstate.cpp
+
+// TODO: make this function thread safe if sections are generated in parallel.
+void update_progress(int delta) {
+ if (!do_update_progress) return;
+
+ progress += delta;
+
+ char key[PROPERTY_KEY_MAX];
+ char value[PROPERTY_VALUE_MAX];
+ sprintf(key, "dumpstate.%d.progress", getpid());
+ sprintf(value, "%d", progress);
+
+ // stderr is ignored on normal invocations, but useful when calling /system/bin/dumpstate
+ // directly for debuggging.
+ fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, WEIGHT_TOTAL);
+
+ int status = property_set(key, value);
+ if (status) {
+ ALOGW("Could not update progress by setting system property %s to %s: %d\n",
+ key, value, status);
+ }
+}