[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/4] Added tests for close and change of logfile.
From: |
Robert Foley |
Subject: |
Re: [PATCH 4/4] Added tests for close and change of logfile. |
Date: |
Thu, 7 Nov 2019 16:33:15 -0500 |
Agree with all the suggestions below. These are great ideas. Will
make the changes.
Thanks,
-Rob Foley
On Thu, 7 Nov 2019 at 11:32, Alex Bennée <address@hidden> wrote:
>
>
> Robert Foley <address@hidden> 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 <address@hidden>
> > ---
> > tests/test-logging.c | 74 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 74 insertions(+)
> >
> > diff --git a/tests/test-logging.c b/tests/test-logging.c
> > index a12585f70a..a3190ff92c 100644
> > --- a/tests/test-logging.c
> > +++ b/tests/test-logging.c
> > @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data)
> > error_free_or_abort(&err);
> > }
> >
> > +static void test_logfile_write(gconstpointer data)
> > +{
> > + QemuLogFile *logfile;
> > + gchar const *dir = data;
> > + Error *err = NULL;
> > + gchar *file_path;
> > + gchar *file_path1;
>
> with g_autofree char *file_path you can avoid the free down bellow.
>
> > + FILE *orig_fd;
> > +
> > + 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);
>
> Maybe better would be:
>
> logfile2 = atomic_rcu_read(&qemu_logfile);
> g_assert(logfile->fd == orig_fd);
> g_assert(logfile2->fd != logfile->fd);
> fprintf(logfile2->fd, "%s 2nd write to file\n", __func__);
> fflush(logfile2->fd);
>
> <snip>
> > + g_assert(logfile->fd == orig_fd);
> > + fprintf(logfile->fd, "%s 2nd write to file\n", __func__);
> > + fflush(logfile->fd);
> > + rcu_read_unlock();
> > +
> > + g_free(file_path);
> > + g_free(file_path1);
> > +}
> > +
> > +static void test_logfile_lock(gconstpointer data)
> > +{
> > + FILE *logfile;
> > + gchar const *dir = data;
> > + Error *err = NULL;
> > + gchar *file_path;
>
> g_autofree
>
> > +
> > + 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);
> > + g_free(file_path);
> > +}
> > +
> > /* Remove a directory and all its entries (non-recursive). */
> > static void rmdir_full(gchar const *root)
> > {
> > @@ -134,6 +204,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
- Re: [PATCH 1/4] Add a mutex to guarantee single writer to qemu_logfile handle., (continued)
- Re: [PATCH 4/4] Added tests for close and change of logfile.,
Robert Foley <=
[PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile handle., Robert Foley, 2019/11/07
Re: [PATCH 0/4] Make the qemu_logfile handle thread safe., no-reply, 2019/11/07
Re: [PATCH 0/4] Make the qemu_logfile handle thread safe., Alex Bennée, 2019/11/07