diff mbox series

[v1,4/5] Added tests for close and change of logfile.

Message ID 20191112150105.2498-5-robert.foley@linaro.org
State Superseded
Headers show
Series Make the qemu_logfile handle thread safe. | expand

Commit Message

Robert Foley Nov. 12, 2019, 3:01 p.m. UTC
One test ensures that the logfile handle is still valid even if
the logfile is changed during logging.
The other test validates that the logfile handle remains valid under
the logfile lock even if the logfile is closed.

Signed-off-by: Robert Foley <robert.foley@linaro.org>

--
v1
    - Changes for first round of code review comments.
    - Added in use of g_autofree, removed the g_free()s.
    - Added in use of logfile2 and changed sequence of asserts.
---
 tests/test-logging.c | 80 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

-- 
2.17.1

Comments

Alex Bennée Nov. 12, 2019, 6:21 p.m. UTC | #1
Robert Foley <robert.foley@linaro.org> writes:

> One test ensures that the logfile handle is still valid even if

> the logfile is changed during logging.

> The other test validates that the logfile handle remains valid under

> the logfile lock even if the logfile is closed.

>

> Signed-off-by: Robert Foley <robert.foley@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> --

> v1

>     - Changes for first round of code review comments.

>     - Added in use of g_autofree, removed the g_free()s.

>     - Added in use of logfile2 and changed sequence of asserts.

> ---

>  tests/test-logging.c | 80 ++++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 80 insertions(+)

>

> diff --git a/tests/test-logging.c b/tests/test-logging.c

> index a12585f70a..1e646f045d 100644

> --- a/tests/test-logging.c

> +++ b/tests/test-logging.c

> @@ -108,6 +108,82 @@ static void test_parse_path(gconstpointer data)

>      error_free_or_abort(&err);

>  }

>

> +static void test_logfile_write(gconstpointer data)

> +{

> +    QemuLogFile *logfile;

> +    QemuLogFile *logfile2;

> +    gchar const *dir = data;

> +    Error *err = NULL;

> +    g_autofree gchar *file_path;

> +    g_autofree gchar *file_path1;

> +    FILE *orig_fd;

> +

> +    /*

> +     * Before starting test, set log flags, to ensure the file gets

> +     * opened below with the call to qemu_set_log_filename().

> +     * In cases where a logging backend other than log is used,

> +     * this is needed.

> +     */

> +    qemu_set_log(CPU_LOG_TB_OUT_ASM);

> +    file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);

> +    file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);

> +

> +    /*

> +     * Test that even if an open file handle is changed,

> +     * our handle remains valid due to RCU.

> +     */

> +    qemu_set_log_filename(file_path, &err);

> +    g_assert(!err);

> +    rcu_read_lock();

> +    logfile = atomic_rcu_read(&qemu_logfile);

> +    orig_fd = logfile->fd;

> +    g_assert(logfile && logfile->fd);

> +    fprintf(logfile->fd, "%s 1st write to file\n", __func__);

> +    fflush(logfile->fd);

> +

> +    /* Change the logfile and ensure that the handle is still valid. */

> +    qemu_set_log_filename(file_path1, &err);

> +    g_assert(!err);

> +    logfile2 = atomic_rcu_read(&qemu_logfile);

> +    g_assert(logfile->fd == orig_fd);

> +    g_assert(logfile2->fd != logfile->fd);

> +    fprintf(logfile->fd, "%s 2nd write to file\n", __func__);

> +    fflush(logfile->fd);

> +    rcu_read_unlock();

> +}

> +

> +static void test_logfile_lock(gconstpointer data)

> +{

> +    FILE *logfile;

> +    gchar const *dir = data;

> +    Error *err = NULL;

> +    g_autofree gchar *file_path;

> +

> +    file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);

> +

> +    /*

> +     * Test the use of the logfile lock, such

> +     * that even if an open file handle is closed,

> +     * our handle remains valid for use due to RCU.

> +     */

> +    qemu_set_log_filename(file_path, &err);

> +    logfile = qemu_log_lock();

> +    g_assert(logfile);

> +    fprintf(logfile, "%s 1st write to file\n", __func__);

> +    fflush(logfile);

> +

> +    /*

> +     * Initiate a close file and make sure our handle remains

> +     * valid since we still have the logfile lock.

> +     */

> +    qemu_log_close();

> +    fprintf(logfile, "%s 2nd write to file\n", __func__);

> +    fflush(logfile);

> +    qemu_log_unlock(logfile);

> +

> +    g_assert(!err);

> +}

> +

>  /* Remove a directory and all its entries (non-recursive). */

>  static void rmdir_full(gchar const *root)

>  {

> @@ -134,6 +210,10 @@ int main(int argc, char **argv)

>

>      g_test_add_func("/logging/parse_range", test_parse_range);

>      g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path);

> +    g_test_add_data_func("/logging/logfile_write_path",

> +                         tmp_path, test_logfile_write);

> +    g_test_add_data_func("/logging/logfile_lock_path",

> +                         tmp_path, test_logfile_lock);

>

>      rc = g_test_run();



--
Alex Bennée
diff mbox series

Patch

diff --git a/tests/test-logging.c b/tests/test-logging.c
index a12585f70a..1e646f045d 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -108,6 +108,82 @@  static void test_parse_path(gconstpointer data)
     error_free_or_abort(&err);
 }
 
+static void test_logfile_write(gconstpointer data)
+{
+    QemuLogFile *logfile;
+    QemuLogFile *logfile2;
+    gchar const *dir = data;
+    Error *err = NULL;
+    g_autofree gchar *file_path;
+    g_autofree gchar *file_path1;
+    FILE *orig_fd;
+
+    /*
+     * Before starting test, set log flags, to ensure the file gets
+     * opened below with the call to qemu_set_log_filename().
+     * In cases where a logging backend other than log is used,
+     * this is needed.
+     */
+    qemu_set_log(CPU_LOG_TB_OUT_ASM);
+    file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
+    file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
+
+    /*
+     * Test that even if an open file handle is changed,
+     * our handle remains valid due to RCU.
+     */
+    qemu_set_log_filename(file_path, &err);
+    g_assert(!err);
+    rcu_read_lock();
+    logfile = atomic_rcu_read(&qemu_logfile);
+    orig_fd = logfile->fd;
+    g_assert(logfile && logfile->fd);
+    fprintf(logfile->fd, "%s 1st write to file\n", __func__);
+    fflush(logfile->fd);
+
+    /* Change the logfile and ensure that the handle is still valid. */
+    qemu_set_log_filename(file_path1, &err);
+    g_assert(!err);
+    logfile2 = atomic_rcu_read(&qemu_logfile);
+    g_assert(logfile->fd == orig_fd);
+    g_assert(logfile2->fd != logfile->fd);
+    fprintf(logfile->fd, "%s 2nd write to file\n", __func__);
+    fflush(logfile->fd);
+    rcu_read_unlock();
+}
+
+static void test_logfile_lock(gconstpointer data)
+{
+    FILE *logfile;
+    gchar const *dir = data;
+    Error *err = NULL;
+    g_autofree gchar *file_path;
+
+    file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);
+
+    /*
+     * Test the use of the logfile lock, such
+     * that even if an open file handle is closed,
+     * our handle remains valid for use due to RCU.
+     */
+    qemu_set_log_filename(file_path, &err);
+    logfile = qemu_log_lock();
+    g_assert(logfile);
+    fprintf(logfile, "%s 1st write to file\n", __func__);
+    fflush(logfile);
+
+    /*
+     * Initiate a close file and make sure our handle remains
+     * valid since we still have the logfile lock.
+     */
+    qemu_log_close();
+    fprintf(logfile, "%s 2nd write to file\n", __func__);
+    fflush(logfile);
+    qemu_log_unlock(logfile);
+
+    g_assert(!err);
+}
+
 /* Remove a directory and all its entries (non-recursive). */
 static void rmdir_full(gchar const *root)
 {
@@ -134,6 +210,10 @@  int main(int argc, char **argv)
 
     g_test_add_func("/logging/parse_range", test_parse_range);
     g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path);
+    g_test_add_data_func("/logging/logfile_write_path",
+                         tmp_path, test_logfile_write);
+    g_test_add_data_func("/logging/logfile_lock_path",
+                         tmp_path, test_logfile_lock);
 
     rc = g_test_run();