| Message ID | 5f493b816595f0f6fe50a3f83e46432ab48d881b.1602182956.git.qemu_oss@crudebyte.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9pfs: add tests using local fs driver | expand |
On 08/10/2020 20.34, Christian Schoenebeck wrote: > This new function is purely for debugging purposes. It prints the > current qos graph to stdout and allows to identify problems in the > created qos graph e.g. when writing new qos tests. > > Coloured output is used to mark available nodes in green colour, > whereas unavailable nodes are marked in red colour. > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > tests/qtest/libqos/qgraph.c | 56 +++++++++++++++++++++++++++++++++++++ > tests/qtest/libqos/qgraph.h | 20 +++++++++++++ > 2 files changed, 76 insertions(+) > > diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c > index 61faf6b27d..af93e38dcb 100644 > --- a/tests/qtest/libqos/qgraph.c > +++ b/tests/qtest/libqos/qgraph.c > @@ -805,3 +805,59 @@ void qos_delete_cmd_line(const char *name) > node->command_line = NULL; > } > } > + > +#define RED(txt) ( \ > + "\033[0;91m" txt \ > + "\033[0m" \ > +) > + > +#define GREEN(txt) ( \ > + "\033[0;92m" txt \ > + "\033[0m" \ > +) I don't think this is very portable - and it will only make logs ugly to read in text editors. Could you please simply drop these macros? Thomas
On Samstag, 24. Oktober 2020 08:04:20 CEST Thomas Huth wrote: > On 08/10/2020 20.34, Christian Schoenebeck wrote: > > This new function is purely for debugging purposes. It prints the > > current qos graph to stdout and allows to identify problems in the > > created qos graph e.g. when writing new qos tests. > > > > Coloured output is used to mark available nodes in green colour, > > whereas unavailable nodes are marked in red colour. > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > > > tests/qtest/libqos/qgraph.c | 56 +++++++++++++++++++++++++++++++++++++ > > tests/qtest/libqos/qgraph.h | 20 +++++++++++++ > > 2 files changed, 76 insertions(+) > > > > diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c > > index 61faf6b27d..af93e38dcb 100644 > > --- a/tests/qtest/libqos/qgraph.c > > +++ b/tests/qtest/libqos/qgraph.c > > @@ -805,3 +805,59 @@ void qos_delete_cmd_line(const char *name) > > > > node->command_line = NULL; > > > > } > > > > } > > > > + > > +#define RED(txt) ( \ > > + "\033[0;91m" txt \ > > + "\033[0m" \ > > +) > > + > > +#define GREEN(txt) ( \ > > + "\033[0;92m" txt \ > > + "\033[0m" \ > > +) > > I don't think this is very portable - and it will only make logs ugly to > read in text editors. Could you please simply drop these macros? > > Thomas The precise way I did it here is definitely unclean. And the use case is trivial, so on doubt I could just drop it of course. But allow me one attempt to promote coloured terminal output in general: These are ANSI color escape sequences, a standard with its youngest revision dating back to 1991. It is a well supported standard on all major platforms nowadays: https://en.wikipedia.org/wiki/ANSI_escape_code It works on macOS's standard terminal for at least 20 years, with cmd.exe on Windows 10, on essentially all Linux and BSD distros, and even on many web based CI platforms. So what about introducing some globally shared macros for coloured output instead? Then there would be one central place for changing coloured output support for the entire code base; and I would change the macros to fallback to plain text output if there is any doubt the terminal would not support it. Besides, QEMU just switched to meson which uses coloured output as well, as do clang, GCC, git and many other tools in your build chain. Best regards, Christian Schoenebeck
On 24/10/2020 13.24, Christian Schoenebeck wrote: > On Samstag, 24. Oktober 2020 08:04:20 CEST Thomas Huth wrote: >> On 08/10/2020 20.34, Christian Schoenebeck wrote: >>> This new function is purely for debugging purposes. It prints the >>> current qos graph to stdout and allows to identify problems in the >>> created qos graph e.g. when writing new qos tests. >>> >>> Coloured output is used to mark available nodes in green colour, >>> whereas unavailable nodes are marked in red colour. >>> >>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> >>> --- >>> >>> tests/qtest/libqos/qgraph.c | 56 +++++++++++++++++++++++++++++++++++++ >>> tests/qtest/libqos/qgraph.h | 20 +++++++++++++ >>> 2 files changed, 76 insertions(+) >>> >>> diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c >>> index 61faf6b27d..af93e38dcb 100644 >>> --- a/tests/qtest/libqos/qgraph.c >>> +++ b/tests/qtest/libqos/qgraph.c >>> @@ -805,3 +805,59 @@ void qos_delete_cmd_line(const char *name) >>> >>> node->command_line = NULL; >>> >>> } >>> >>> } >>> >>> + >>> +#define RED(txt) ( \ >>> + "\033[0;91m" txt \ >>> + "\033[0m" \ >>> +) >>> + >>> +#define GREEN(txt) ( \ >>> + "\033[0;92m" txt \ >>> + "\033[0m" \ >>> +) >> >> I don't think this is very portable - and it will only make logs ugly to >> read in text editors. Could you please simply drop these macros? >> >> Thomas > > The precise way I did it here is definitely unclean. And the use case is > trivial, so on doubt I could just drop it of course. > > But allow me one attempt to promote coloured terminal output in general: These > are ANSI color escape sequences, a standard with its youngest revision dating > back to 1991. It is a well supported standard on all major platforms nowadays: > > https://en.wikipedia.org/wiki/ANSI_escape_code > > It works on macOS's standard terminal for at least 20 years, with cmd.exe on > Windows 10, on essentially all Linux and BSD distros, and even on many web > based CI platforms. > > So what about introducing some globally shared macros for coloured output > instead? Then there would be one central place for changing coloured output > support for the entire code base; and I would change the macros to fallback to > plain text output if there is any doubt the terminal would not support it. > > Besides, QEMU just switched to meson which uses coloured output as well, as do > clang, GCC, git and many other tools in your build chain. Sure, colored output is nice, but we certainly also need a way to disable it, e.g. if you want to collect the log in a file and then have a look at it in a text editor. Thomas
On 10/28/20 12:51 AM, Thomas Huth wrote: >>>> +#define GREEN(txt) ( \ >>>> + "\033[0;92m" txt \ >>>> + "\033[0m" \ >>>> +) >>> >>> I don't think this is very portable - and it will only make logs ugly to >>> read in text editors. Could you please simply drop these macros? >>> > Sure, colored output is nice, but we certainly also need a way to disable > it, e.g. if you want to collect the log in a file and then have a look at it > in a text editor. Agreed. GNU libtextstyle (https://www.gnu.org/software/gettext/libtextstyle/manual/libtextstyle.html) is a much more portable way to do colored output where it becomes easy to enable/disable or even adjust the colors to user preferences. Sadly, it is GPLv3+, and thus unusable for qemu. But the bare minimum that you must have when making colored output gated on whether stdout is a terminal (that is, any program that does color should have a --color=off|auto|on command-line option, and that in turn implies function calls rather than macros to properly encapsulate the decision logic.
On Mittwoch, 28. Oktober 2020 14:00:21 CET Eric Blake wrote: > On 10/28/20 12:51 AM, Thomas Huth wrote: > >>>> +#define GREEN(txt) ( \ > >>>> + "\033[0;92m" txt \ > >>>> + "\033[0m" \ > >>>> +) > >>> > >>> I don't think this is very portable - and it will only make logs ugly to > >>> read in text editors. Could you please simply drop these macros? > > > > Sure, colored output is nice, but we certainly also need a way to disable > > it, e.g. if you want to collect the log in a file and then have a look at > > it in a text editor. > > Agreed. GNU libtextstyle > (https://www.gnu.org/software/gettext/libtextstyle/manual/libtextstyle.html) > is a much more portable way to do colored output where it becomes easy to > enable/disable or even adjust the colors to user preferences. Sadly, it is > GPLv3+, and thus unusable for qemu. But the bare minimum that you must > have when making colored output gated on whether stdout is a > terminal (that is, any program that does color should have a > --color=off|auto|on command-line option, and that in turn implies > function calls rather than macros to properly encapsulate the decision > logic. Not sure if it would make sense adding another dependency just for colour support in QEMU anyway, because rendering the right output sequence is not the big issue, nor auto detecting tty colour support, nor handling user configs. A large number of apps already do that in-tree / inline. The challenge in QEMU though (in contrast to stand-alone apps) is integrating this meaningful for all the (quite different) output channels in QEMU, e.g. host logs, test case output, different modes, etc., while catching misusage and retaining a simple API. I postpone the colour issue for that reason and drop colour from these patches for now. I'll probably rather come up with a dedicated series attempt just for colour at some later point. Best regards, Christian Schoenebeck
diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c index 61faf6b27d..af93e38dcb 100644 --- a/tests/qtest/libqos/qgraph.c +++ b/tests/qtest/libqos/qgraph.c @@ -805,3 +805,59 @@ void qos_delete_cmd_line(const char *name) node->command_line = NULL; } } + +#define RED(txt) ( \ + "\033[0;91m" txt \ + "\033[0m" \ +) + +#define GREEN(txt) ( \ + "\033[0;92m" txt \ + "\033[0m" \ +) + +void qos_dump_graph(void) +{ + GList *keys; + GList *l; + QOSGraphEdgeList *list; + QOSGraphEdge *e, *next; + QOSGraphNode *dest_node, *node; + + qos_printf("ALL QGRAPH EDGES: {\n"); + keys = g_hash_table_get_keys(edge_table); + for (l = keys; l != NULL; l = l->next) { + const gchar *key = l->data; + qos_printf("\t src='%s'\n", key); + list = get_edgelist(key); + QSLIST_FOREACH_SAFE(e, list, edge_list, next) { + dest_node = g_hash_table_lookup(node_table, e->dest); + qos_printf("\t\t|-> dest='%s' type=%d (node=%p)", + e->dest, e->type, dest_node); + if (!dest_node) { + qos_printf_literal(RED(" <------- ERROR !")); + } + qos_printf_literal("\n"); + } + } + g_list_free(keys); + qos_printf("}\n"); + + qos_printf("ALL QGRAPH NODES: {\n"); + keys = g_hash_table_get_keys(node_table); + for (l = keys; l != NULL; l = l->next) { + const gchar *key = l->data; + node = g_hash_table_lookup(node_table, key); + qos_printf("\t name='%s' ", key); + if (node->qemu_name) { + qos_printf_literal("qemu_name='%s' ", node->qemu_name); + } + qos_printf_literal("type=%d cmd_line='%s' [%s]\n", + node->type, node->command_line, + node->available ? GREEN("available") : + RED("UNAVAILBLE") + ); + } + g_list_free(keys); + qos_printf("}\n"); +} diff --git a/tests/qtest/libqos/qgraph.h b/tests/qtest/libqos/qgraph.h index f472949f68..07a32535f1 100644 --- a/tests/qtest/libqos/qgraph.h +++ b/tests/qtest/libqos/qgraph.h @@ -586,5 +586,25 @@ QOSGraphObject *qos_machine_new(QOSGraphNode *node, QTestState *qts); QOSGraphObject *qos_driver_new(QOSGraphNode *node, QOSGraphObject *parent, QGuestAllocator *alloc, void *arg); +/** + * Just for debugging purpose: prints all currently existing nodes and + * edges to stdout. + * + * All qtests add themselves to the overall qos graph by calling qgraph + * functions that add device nodes and edges between the individual graph + * nodes for tests. As the actual graph is assmbled at runtime by the qos + * subsystem, it is sometimes not obvious how the overall graph looks like. + * E.g. when writing new tests it may happen that those new tests are simply + * ignored by the qtest framework. + * + * This function allows to identify problems in the created qgraph. Keep in + * mind: only tests with a path down from the actual test case node (leaf) up + * to the graph's root node are actually executed by the qtest framework. And + * the qtest framework uses QMP to automatically check which QEMU drivers are + * actually currently available, and accordingly qos marks certain pathes as + * 'unavailable' in such cases (e.g. when QEMU was compiled without support for + * a certain feature). + */ +void qos_dump_graph(void); #endif
This new function is purely for debugging purposes. It prints the current qos graph to stdout and allows to identify problems in the created qos graph e.g. when writing new qos tests. Coloured output is used to mark available nodes in green colour, whereas unavailable nodes are marked in red colour. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- tests/qtest/libqos/qgraph.c | 56 +++++++++++++++++++++++++++++++++++++ tests/qtest/libqos/qgraph.h | 20 +++++++++++++ 2 files changed, 76 insertions(+)