Commit daa76aa4 authored by Markus Armbruster's avatar Markus Armbruster
Browse files

log: Fix qemu_set_log_filename() error handling 



When qemu_set_log_filename() detects an invalid file name, it reports
an error, closes the log file (if any), and starts logging to stderr
(unless daemonized or nothing is being logged).

This is wrong.  Asking for an invalid log file on the command line
should be fatal.  Asking for one in the monitor should fail without
messing up an existing logfile.

Fix by converting qemu_set_log_filename() to Error.  Pass it
&error_fatal, except for hmp_logfile report errors.

This also permits testing without a subprocess, so do that.

Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Message-Id: <1466011636-6112-4-git-send-email-armbru@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
parent bd6fee9f

bsd-user/main.c

+2 −1
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include "qemu/osdep.h"

#include <machine/trap.h>



#include "qapi/error.h"

#include "qemu.h"

#include "qemu/path.h"

#include "qemu/help_option.h"
@@ -847,7 +848,7 @@ int main(int argc, char **argv) 


    /* init debug */

    qemu_log_needs_buffers();

    qemu_set_log_filename(log_file);

    qemu_set_log_filename(log_file, &error_fatal);

    if (log_mask) {

        int mask;

include/qemu/log.h

+1 −1
Original line number Diff line number Diff line
@@ -106,7 +106,7 @@ extern const QEMULogItem qemu_log_items[]; 


void qemu_set_log(int log_flags);

void qemu_log_needs_buffers(void);

void qemu_set_log_filename(const char *filename);

void qemu_set_log_filename(const char *filename, Error **errp);

void qemu_set_dfilter_ranges(const char *ranges, Error **errp);

bool qemu_log_in_addr_range(uint64_t addr);

int qemu_str_to_log_mask(const char *str);

linux-user/main.c

+2 −1
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include <sys/syscall.h>

#include <sys/resource.h>



#include "qapi/error.h"

#include "qemu.h"

#include "qemu/path.h"

#include "qemu/cutils.h"
@@ -3845,7 +3846,7 @@ static void handle_arg_log(const char *arg) 


static void handle_arg_log_filename(const char *arg)

{

    qemu_set_log_filename(arg);

    qemu_set_log_filename(arg, &error_fatal);

}



static void handle_arg_set_env(const char *arg)

monitor.c

+6 −1
Original line number Diff line number Diff line
@@ -1111,7 +1111,12 @@ void qmp_client_migrate_info(const char *protocol, const char *hostname, 


static void hmp_logfile(Monitor *mon, const QDict *qdict)

{

    qemu_set_log_filename(qdict_get_str(qdict, "filename"));

    Error *err = NULL;



    qemu_set_log_filename(qdict_get_str(qdict, "filename"), &err);

    if (err) {

        error_report_err(err);

    }

}



static void hmp_log(Monitor *mon, const QDict *qdict)

tests/test-logging.c

+8 −33
Original line number Diff line number Diff line
@@ -75,49 +75,24 @@ static void test_parse_range(void) 
    error_free_or_abort(&err);

}



#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS

/* As the only real failure from a bad log filename path spec is

 * reporting to the user we have to use the g_test_trap_subprocess

 * mechanism and check no errors reported on stderr.

 */

static void test_parse_path_subprocess(void)

{

    /* All these should work without issue */

    qemu_set_log_filename("/tmp/qemu.log");

    qemu_set_log_filename("/tmp/qemu-%d.log");

    qemu_set_log_filename("/tmp/qemu.log.%d");

}

static void test_parse_path(void)

{

    g_test_trap_subprocess ("/logging/parse_path/subprocess", 0, 0);

    g_test_trap_assert_passed();

    g_test_trap_assert_stdout("");

    g_test_trap_assert_stderr("");

}

static void test_parse_invalid_path_subprocess(void)

{

    qemu_set_log_filename("/tmp/qemu-%d%d.log");

}

static void test_parse_invalid_path(void)

{

    g_test_trap_subprocess ("/logging/parse_invalid_path/subprocess", 0, 0);

    g_test_trap_assert_passed();

    g_test_trap_assert_stdout("");

    g_test_trap_assert_stderr("Bad logfile format: /tmp/qemu-%d%d.log\n");

    Error *err = NULL;



    qemu_set_log_filename("/tmp/qemu.log", &error_abort);

    qemu_set_log_filename("/tmp/qemu-%d.log", &error_abort);

    qemu_set_log_filename("/tmp/qemu.log.%d", &error_abort);



    qemu_set_log_filename("/tmp/qemu-%d%d.log", &err);

    error_free_or_abort(&err);

}

#endif /* CONFIG_HAS_GLIB_SUBPROCESS_TESTS */



int main(int argc, char **argv)

{

    g_test_init(&argc, &argv, NULL);



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

#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS

    g_test_add_func("/logging/parse_path", test_parse_path);

    g_test_add_func("/logging/parse_path/subprocess", test_parse_path_subprocess);

    g_test_add_func("/logging/parse_invalid_path", test_parse_invalid_path);

    g_test_add_func("/logging/parse_invalid_path/subprocess", test_parse_invalid_path_subprocess);

#endif



    return g_test_run();

}