From: "Alex Bennée" <alex.bennee@linaro.org>
To: Robert Foley <robert.foley@linaro.org>
Cc: peter.puhov@linaro.org, qemu-devel@nongnu.org
Subject: Re: [PATCH v1 4/5] Added tests for close and change of logfile.
Date: Tue, 12 Nov 2019 18:21:25 +0000 [thread overview]
Message-ID: <87imnp7y6y.fsf@linaro.org> (raw)
In-Reply-To: <20191112150105.2498-5-robert.foley@linaro.org>
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
next prev parent reply other threads:[~2019-11-12 18:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-12 15:01 [PATCH v1 0/5] Make the qemu_logfile handle thread safe Robert Foley
2019-11-12 15:01 ` [PATCH v1 1/5] Add a mutex to guarantee single writer to qemu_logfile handle Robert Foley
2019-11-12 17:09 ` Alex Bennée
2019-11-16 11:57 ` Richard Henderson
2019-11-18 12:39 ` Robert Foley
2019-11-12 15:01 ` [PATCH v1 2/5] qemu_log_lock/unlock now preserves the " Robert Foley
2019-11-12 17:17 ` Alex Bennée
2019-11-16 12:26 ` Richard Henderson
2019-11-12 15:01 ` [PATCH v1 3/5] Add use of RCU for qemu_logfile Robert Foley
2019-11-12 17:36 ` Alex Bennée
2019-11-12 19:20 ` Robert Foley
2019-11-13 15:24 ` Alex Bennée
2019-11-12 15:01 ` [PATCH v1 4/5] Added tests for close and change of logfile Robert Foley
2019-11-12 18:21 ` Alex Bennée [this message]
2019-11-12 15:01 ` [PATCH v1 5/5] Fix double free issue in qemu_set_log_filename() Robert Foley
2019-11-12 18:23 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87imnp7y6y.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=peter.puhov@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=robert.foley@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).