| Message ID | c9a6671914385d0ec6dcd3aee1371d73e09ee33a.1602182956.git.qemu_oss@crudebyte.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | 9pfs: add tests using local fs driver | expand |
On Donnerstag, 8. Oktober 2020 20:34:56 CEST Christian Schoenebeck wrote: > Add new member variable 'qemu_name' to struct QOSGraphNode. > > This new member may be optionally set in case a different > name for the node (which must always be a unique name) vs. > its actually associated QEMU (QMP) device name is required. > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > tests/qtest/libqos/qgraph.c | 1 + > tests/qtest/libqos/qgraph_internal.h | 1 + > 2 files changed, 2 insertions(+) So what shall happen with these libqos patches 1..7? Is that a nack, or postpone for now? Best regards, Christian Schoenebeck
On 19/10/2020 12.35, Christian Schoenebeck wrote: > On Donnerstag, 8. Oktober 2020 20:34:56 CEST Christian Schoenebeck wrote: >> Add new member variable 'qemu_name' to struct QOSGraphNode. >> >> This new member may be optionally set in case a different >> name for the node (which must always be a unique name) vs. >> its actually associated QEMU (QMP) device name is required. >> >> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> >> --- >> tests/qtest/libqos/qgraph.c | 1 + >> tests/qtest/libqos/qgraph_internal.h | 1 + >> 2 files changed, 2 insertions(+) > > So what shall happen with these libqos patches 1..7? Is that a nack, or > postpone for now? I was hoping to see a review by Paolo or Laurent, who are much more familiar with qos than I am ... but after having a look at the patches, I think I can also give some feedback, too: Patch 1 and 2 sound basically ok to me (should maybe be squashed together, though), but the qos_node_create_driver_named() function currently seems to be unused so far? So I'd postpone these two patches to the point in time when you really need the qos_node_create_driver_named() function. The other patches are basically fine with me, too, but please avoid the hard-coded ESC codes that only work with certain terminals. Thomas
On Samstag, 24. Oktober 2020 08:08:59 CEST Thomas Huth wrote: > On 19/10/2020 12.35, Christian Schoenebeck wrote: > > On Donnerstag, 8. Oktober 2020 20:34:56 CEST Christian Schoenebeck wrote: > >> Add new member variable 'qemu_name' to struct QOSGraphNode. > >> > >> This new member may be optionally set in case a different > >> name for the node (which must always be a unique name) vs. > >> its actually associated QEMU (QMP) device name is required. > >> > >> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > >> --- > >> > >> tests/qtest/libqos/qgraph.c | 1 + > >> tests/qtest/libqos/qgraph_internal.h | 1 + > >> 2 files changed, 2 insertions(+) > > > > So what shall happen with these libqos patches 1..7? Is that a nack, or > > postpone for now? > > I was hoping to see a review by Paolo or Laurent, who are much more familiar > with qos than I am ... but after having a look at the patches, I think I > can also give some feedback, too: > > Patch 1 and 2 sound basically ok to me (should maybe be squashed together, > though), but the qos_node_create_driver_named() function currently seems to > be unused so far? So I'd postpone these two patches to the point in time > when you really need the qos_node_create_driver_named() function. I did use patches 1 & 2 in v1 of this series, as of v2 and higher I used a workaround for the actual 9pfs test case patches not to depend on these 2 libqos patches. This happened after Paolo's feedback, who wrote that qos patches 1 & 2 would be useful for other, future use cases, but argued it would not be appropriate for my intended use case: https://lore.kernel.org/qemu-devel/95ef57d0-b35e-f16a-f957-06bc3692cb7c@redhat.com/ I preserved patches 1 & 2 in this series though as he noted they might be useful for future purposes and applied his requested changes. I personally probably won't need thise 2 patches any time soon. So it's up to you what shall happen with them. I don't mind if you postpone or nack them. > The other patches are basically fine with me, too, but please avoid the > hard-coded ESC codes that only work with certain terminals. > > Thomas I'll respond to that on your patch 4 response. Best regards, Christian Schoenebeck
diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c index fc49cfa879..e42f3eaafa 100644 --- a/tests/qtest/libqos/qgraph.c +++ b/tests/qtest/libqos/qgraph.c @@ -153,6 +153,7 @@ static QOSGraphNode *create_node(const char *name, QOSNodeType type) static void destroy_node(void *val) { QOSGraphNode *node = val; + g_free(node->qemu_name); g_free(node->command_line); g_free(node); } diff --git a/tests/qtest/libqos/qgraph_internal.h b/tests/qtest/libqos/qgraph_internal.h index 968fa69450..974985dce9 100644 --- a/tests/qtest/libqos/qgraph_internal.h +++ b/tests/qtest/libqos/qgraph_internal.h @@ -56,6 +56,7 @@ struct QOSGraphNode { bool available; /* set by QEMU via QMP, used during graph walk */ bool visited; /* used during graph walk */ char *name; /* used to identify the node */ + char *qemu_name; /* optional: see qos_node_create_driver_named() */ char *command_line; /* used to start QEMU at test execution */ union { struct {
Add new member variable 'qemu_name' to struct QOSGraphNode. This new member may be optionally set in case a different name for the node (which must always be a unique name) vs. its actually associated QEMU (QMP) device name is required. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- tests/qtest/libqos/qgraph.c | 1 + tests/qtest/libqos/qgraph_internal.h | 1 + 2 files changed, 2 insertions(+)