The qtest server right now can only be created using the -qtest
and -qtest-log options. Allow an alternative way to create it
using "-object qtest,chardev=...,log=...".
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
softmmu/qtest.c | 144 ++++++++++++++++++++++++++++++++++++++++++++----
softmmu/vl.c | 5 +-
2 files changed, 135 insertions(+), 14 deletions(-)
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 7965dc9a16..d255c9681a 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -27,6 +27,8 @@
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "qemu/cutils.h"
+#include "qapi/qmp/qerror.h"
+#include "qom/object_interfaces.h"
#include CONFIG_DEVICES
#ifdef CONFIG_PSERIES
#include "hw/ppc/spapr_rtas.h"
@@ -849,18 +851,9 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
break;
}
}
-void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
-{
- Chardev *chr;
-
- chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
-
- if (chr == NULL) {
- error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
- qtest_chrdev);
- return;
- }
+static bool qtest_server_start(Chardev *chr, const char *qtest_log, Error **errp)
+{
if (qtest_log) {
if (strcmp(qtest_log, "none") != 0) {
qtest_log_fp = fopen(qtest_log, "w+");
@@ -869,7 +862,9 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
qtest_log_fp = stderr;
}
- qemu_chr_fe_init(&qtest_chr, chr, errp);
+ if (!qemu_chr_fe_init(&qtest_chr, chr, errp)) {
+ return false;
+ }
qemu_chr_fe_set_handlers(&qtest_chr, qtest_can_read, qtest_read,
qtest_event, NULL, &qtest_chr, NULL, true);
qemu_chr_fe_set_echo(&qtest_chr, true);
@@ -879,8 +874,25 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
if (!qtest_server_send) {
qtest_server_set_send_handler(qtest_server_char_be_send, &qtest_chr);
}
+ return true;
+}
+
+void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
+{
+ Chardev *chr;
+
+ chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
+
+ if (chr == NULL) {
+ error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
+ qtest_chrdev);
+ return;
+ }
+
+ qtest_server_start(chr, qtest_log, errp);
}
+
void qtest_server_set_send_handler(void (*send)(void*, const char*),
void *opaque)
{
@@ -905,3 +917,111 @@ void qtest_server_inproc_recv(void *dummy, const char *buf)
g_string_truncate(gstr, 0);
}
}
+
+#define TYPE_QTEST "qtest"
+
+OBJECT_DECLARE_SIMPLE_TYPE(QTest, QTEST)
+
+struct QTest {
+ Object parent;
+
+ bool complete;
+ char *chr_name;
+ Chardev *chr;
+ char *log;
+};
+
+static void qtest_complete(UserCreatable *uc, Error **errp)
+{
+ QTest *q = QTEST(uc);
+ if (qtest_driver()) {
+ error_setg(errp, "Only one instance of qtest can be created");
+ return;
+ }
+ if (!q->chr_name) {
+ error_setg(errp, "No backend specified");
+ return;
+ }
+
+ if (!qtest_server_start(q->chr, q->log, errp)) {
+ return;
+ }
+ q->complete = true;
+}
+
+static void qtest_set_log(Object *obj, const char *value, Error **errp)
+{
+ QTest *q = QTEST(obj);
+
+ if (q->complete) {
+ error_setg(errp, QERR_PERMISSION_DENIED);
+ } else {
+ g_free(q->log);
+ q->log = g_strdup(value);
+ }
+}
+
+static char *qtest_get_log(Object *obj, Error **errp)
+{
+ QTest *q = QTEST(obj);
+
+ return g_strdup(q->log);
+}
+
+static void qtest_set_chardev(Object *obj, const char *value, Error **errp)
+{
+ QTest *q = QTEST(obj);
+ Chardev *chr;
+
+ if (q->complete) {
+ error_setg(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ chr = qemu_chr_find(value);
+ if (!chr) {
+ error_setg(errp, "Cannot find character device '%s'", value);
+ return;
+ }
+
+ g_free(q->chr_name);
+ q->chr_name = g_strdup(value);
+ q->chr = chr;
+}
+
+static char *qtest_get_chardev(Object *obj, Error **errp)
+{
+ QTest *q = QTEST(obj);
+
+ return g_strdup(q->chr_name);
+}
+
+static void qtest_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+ ucc->complete = qtest_complete;
+
+ object_class_property_add_str(oc, "chardev",
+ qtest_get_chardev, qtest_set_chardev);
+ object_class_property_add_str(oc, "log",
+ qtest_get_log, qtest_set_log);
+}
+
+static const TypeInfo qtest_info = {
+ .name = TYPE_QTEST,
+ .parent = TYPE_OBJECT,
+ .class_init = qtest_class_init,
+ .instance_size = sizeof(QTest),
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { }
+ }
+};
+
+static void register_types(void)
+{
+ type_register_static(&qtest_info);
+}
+
+type_init(register_types);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 0f7222af31..e5f3c42049 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1685,8 +1685,9 @@ static bool object_create_early(const char *type, QemuOpts *opts)
* add one, state the reason in a comment!
*/
- /* Reason: rng-egd property "chardev" */
- if (g_str_equal(type, "rng-egd")) {
+ /* Reason: property "chardev" */
+ if (g_str_equal(type, "rng-egd") ||
+ g_str_equal(type, "qtest")) {
return false;
}
--
2.26.2
On Wed, 2 Dec 2020 03:18:50 -0500
Paolo Bonzini <pbonzini@redhat.com> wrote:
> The qtest server right now can only be created using the -qtest
> and -qtest-log options. Allow an alternative way to create it
> using "-object qtest,chardev=...,log=...".
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> softmmu/qtest.c | 144 ++++++++++++++++++++++++++++++++++++++++++++----
> softmmu/vl.c | 5 +-
> 2 files changed, 135 insertions(+), 14 deletions(-)
>
> diff --git a/softmmu/qtest.c b/softmmu/qtest.c
> index 7965dc9a16..d255c9681a 100644
> --- a/softmmu/qtest.c
> +++ b/softmmu/qtest.c
> @@ -27,6 +27,8 @@
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> #include "qemu/cutils.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qom/object_interfaces.h"
> #include CONFIG_DEVICES
> #ifdef CONFIG_PSERIES
> #include "hw/ppc/spapr_rtas.h"
> @@ -849,18 +851,9 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
> break;
> }
> }
> -void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
> -{
> - Chardev *chr;
> -
> - chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
> -
> - if (chr == NULL) {
> - error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
> - qtest_chrdev);
> - return;
> - }
>
> +static bool qtest_server_start(Chardev *chr, const char *qtest_log, Error **errp)
> +{
> if (qtest_log) {
> if (strcmp(qtest_log, "none") != 0) {
> qtest_log_fp = fopen(qtest_log, "w+");
> @@ -869,7 +862,9 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
> qtest_log_fp = stderr;
> }
>
> - qemu_chr_fe_init(&qtest_chr, chr, errp);
> + if (!qemu_chr_fe_init(&qtest_chr, chr, errp)) {
> + return false;
> + }
> qemu_chr_fe_set_handlers(&qtest_chr, qtest_can_read, qtest_read,
> qtest_event, NULL, &qtest_chr, NULL, true);
> qemu_chr_fe_set_echo(&qtest_chr, true);
> @@ -879,8 +874,25 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
> if (!qtest_server_send) {
> qtest_server_set_send_handler(qtest_server_char_be_send, &qtest_chr);
> }
> + return true;
> +}
> +
> +void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
> +{
> + Chardev *chr;
> +
> + chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
> +
> + if (chr == NULL) {
> + error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
> + qtest_chrdev);
> + return;
> + }
> +
> + qtest_server_start(chr, qtest_log, errp);
why not create qtest object here instead of trying to preserve old way,
or create it directly at the place that calls qtest_server_init()?
> }
>
> +
> void qtest_server_set_send_handler(void (*send)(void*, const char*),
> void *opaque)
> {
> @@ -905,3 +917,111 @@ void qtest_server_inproc_recv(void *dummy, const char *buf)
> g_string_truncate(gstr, 0);
> }
> }
> +
> +#define TYPE_QTEST "qtest"
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(QTest, QTEST)
> +
> +struct QTest {
> + Object parent;
> +
> + bool complete;
> + char *chr_name;
> + Chardev *chr;
> + char *log;
> +};
> +
> +static void qtest_complete(UserCreatable *uc, Error **errp)
> +{
> + QTest *q = QTEST(uc);
> + if (qtest_driver()) {
> + error_setg(errp, "Only one instance of qtest can be created");
> + return;
> + }
> + if (!q->chr_name) {
> + error_setg(errp, "No backend specified");
> + return;
> + }
> +
> + if (!qtest_server_start(q->chr, q->log, errp)) {
> + return;
> + }
> + q->complete = true;
> +}
> +
> +static void qtest_set_log(Object *obj, const char *value, Error **errp)
> +{
> + QTest *q = QTEST(obj);
> +
> + if (q->complete) {
> + error_setg(errp, QERR_PERMISSION_DENIED);
> + } else {
> + g_free(q->log);
> + q->log = g_strdup(value);
> + }
> +}
> +
> +static char *qtest_get_log(Object *obj, Error **errp)
> +{
> + QTest *q = QTEST(obj);
> +
> + return g_strdup(q->log);
> +}
> +
> +static void qtest_set_chardev(Object *obj, const char *value, Error **errp)
> +{
> + QTest *q = QTEST(obj);
> + Chardev *chr;
> +
> + if (q->complete) {
> + error_setg(errp, QERR_PERMISSION_DENIED);
> + return;
> + }
> +
> + chr = qemu_chr_find(value);
> + if (!chr) {
> + error_setg(errp, "Cannot find character device '%s'", value);
> + return;
> + }
> +
> + g_free(q->chr_name);
> + q->chr_name = g_strdup(value);
> + q->chr = chr;
> +}
> +
> +static char *qtest_get_chardev(Object *obj, Error **errp)
> +{
> + QTest *q = QTEST(obj);
> +
> + return g_strdup(q->chr_name);
> +}
> +
> +static void qtest_class_init(ObjectClass *oc, void *data)
> +{
> + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> +
> + ucc->complete = qtest_complete;
> +
> + object_class_property_add_str(oc, "chardev",
> + qtest_get_chardev, qtest_set_chardev);
> + object_class_property_add_str(oc, "log",
> + qtest_get_log, qtest_set_log);
> +}
> +
> +static const TypeInfo qtest_info = {
> + .name = TYPE_QTEST,
> + .parent = TYPE_OBJECT,
> + .class_init = qtest_class_init,
> + .instance_size = sizeof(QTest),
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_USER_CREATABLE },
> + { }
> + }
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&qtest_info);
> +}
> +
> +type_init(register_types);
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 0f7222af31..e5f3c42049 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1685,8 +1685,9 @@ static bool object_create_early(const char *type, QemuOpts *opts)
> * add one, state the reason in a comment!
> */
>
> - /* Reason: rng-egd property "chardev" */
> - if (g_str_equal(type, "rng-egd")) {
> + /* Reason: property "chardev" */
> + if (g_str_equal(type, "rng-egd") ||
> + g_str_equal(type, "qtest")) {
> return false;
> }
>
On 07/12/20 17:24, Igor Mammedov wrote:
>> +void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
>> +{
>> + Chardev *chr;
>> +
>> + chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
>> +
>> + if (chr == NULL) {
>> + error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
>> + qtest_chrdev);
>> + return;
>> + }
>> +
>> + qtest_server_start(chr, qtest_log, errp);
> why not create qtest object here instead of trying to preserve old way,
> or create it directly at the place that calls qtest_server_init()?
Because I wasn't sure of where to put it in the QOM object tree. So I
punted and left it for later.
Paolo
On Mon, 7 Dec 2020 17:43:16 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 07/12/20 17:24, Igor Mammedov wrote:
> >> +void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
> >> +{
> >> + Chardev *chr;
> >> +
> >> + chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
> >> +
> >> + if (chr == NULL) {
> >> + error_setg(errp, "Failed to initialize device for qtest: \"%s\"",
> >> + qtest_chrdev);
> >> + return;
> >> + }
> >> +
> >> + qtest_server_start(chr, qtest_log, errp);
> > why not create qtest object here instead of trying to preserve old way,
> > or create it directly at the place that calls qtest_server_init()?
>
> Because I wasn't sure of where to put it in the QOM object tree. So I
> punted and left it for later.
but you implicitly decided where it should be (with -object qtest),
it goes to /objects.
So I'd wouldn't put anywhere else to be consistent.
>
> Paolo
>
Il lun 7 dic 2020, 17:57 Igor Mammedov <imammedo@redhat.com> ha scritto:
> On Mon, 7 Dec 2020 17:43:16 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> > On 07/12/20 17:24, Igor Mammedov wrote:
> > >> +void qtest_server_init(const char *qtest_chrdev, const char
> *qtest_log, Error **errp)
> > >> +{
> > >> + Chardev *chr;
> > >> +
> > >> + chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
> > >> +
> > >> + if (chr == NULL) {
> > >> + error_setg(errp, "Failed to initialize device for qtest:
> \"%s\"",
> > >> + qtest_chrdev);
> > >> + return;
> > >> + }
> > >> +
> > >> + qtest_server_start(chr, qtest_log, errp);
> > > why not create qtest object here instead of trying to preserve old way,
> > > or create it directly at the place that calls qtest_server_init()?
> >
> > Because I wasn't sure of where to put it in the QOM object tree. So I
> > punted and left it for later.
>
> but you implicitly decided where it should be (with -object qtest),
> it goes to /objects.
> So I'd wouldn't put anywhere else to be consistent.
>
No, /objects is for stuff created with -object exclusively. I suppose I
could have the "well-known path" be /machine/qtest, and it would be either
a child (for -qtest) or a link to /objects/some-id (for -object qtest).
Should I implement that (as a separate patch on top of this one)?
Paolo
> >
> > Paolo
> >
>
>
On Mon, 7 Dec 2020 18:22:55 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il lun 7 dic 2020, 17:57 Igor Mammedov <imammedo@redhat.com> ha scritto:
>
> > On Mon, 7 Dec 2020 17:43:16 +0100
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > > On 07/12/20 17:24, Igor Mammedov wrote:
> > > >> +void qtest_server_init(const char *qtest_chrdev, const char
> > *qtest_log, Error **errp)
> > > >> +{
> > > >> + Chardev *chr;
> > > >> +
> > > >> + chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
> > > >> +
> > > >> + if (chr == NULL) {
> > > >> + error_setg(errp, "Failed to initialize device for qtest:
> > \"%s\"",
> > > >> + qtest_chrdev);
> > > >> + return;
> > > >> + }
> > > >> +
> > > >> + qtest_server_start(chr, qtest_log, errp);
> > > > why not create qtest object here instead of trying to preserve old way,
> > > > or create it directly at the place that calls qtest_server_init()?
> > >
> > > Because I wasn't sure of where to put it in the QOM object tree. So I
> > > punted and left it for later.
> >
> > but you implicitly decided where it should be (with -object qtest),
> > it goes to /objects.
> > So I'd wouldn't put anywhere else to be consistent.
> >
>
> No, /objects is for stuff created with -object exclusively. I suppose I
> could have the "well-known path" be /machine/qtest, and it would be either
> a child (for -qtest) or a link to /objects/some-id (for -object qtest).
> Should I implement that (as a separate patch on top of this one)?
yes
>
> Paolo
>
>
>
> > >
> > > Paolo
> > >
> >
> >
© 2016 - 2026 Red Hat, Inc.