| Message ID | 20170323160315.19696-1-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 23, 2017 at 11:03:15AM -0500, Eric Blake wrote: > Commit 9a6d1ac assumed that 'qom-type' could be removed from QemuOpts > with no ill effects. However, this command line proves otherwise: > > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ > -object rng-random,filename=/dev/urandom,id=rng0 \ > -device virtio-rng-pci,rng=rng0 > qemu-system-x86_64: -object rng-random,filename=/dev/urandom,id=rng0: Parameter 'qom-type' is missing > > Fix the regression by restoring qom-type in opts after its temporary > removal that was needed for the duration of user_creatable_add_opts(). > > Reported-by: Richard W. M. Jones <rjones@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > qom/object_interfaces.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index 9c271ad..d4253a8 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > } > if (!id) { > error_setg(errp, QERR_MISSING_PARAMETER, "id"); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > return NULL; > } > @@ -134,6 +135,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > visit_free(v); > > qemu_opts_set_id(opts, (char *) id); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > QDECREF(pdict); > return obj; Thanks Eric. Yes, this fixes the problem I observed before with libguestfs. Tested-by: Richard W.M. Jones <rjones@redhat.com> Rich.
Eric Blake <eblake@redhat.com> writes: > Commit 9a6d1ac assumed that 'qom-type' could be removed from QemuOpts > with no ill effects. However, this command line proves otherwise: > > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ > -object rng-random,filename=/dev/urandom,id=rng0 \ > -device virtio-rng-pci,rng=rng0 > qemu-system-x86_64: -object rng-random,filename=/dev/urandom,id=rng0: Parameter 'qom-type' is missing Shows that our test coverage is still inadequate. Also shows that I should do more manual testing myself. Sorry for the inconvenience! > Fix the regression by restoring qom-type in opts after its temporary > removal that was needed for the duration of user_creatable_add_opts(). > > Reported-by: Richard W. M. Jones <rjones@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > qom/object_interfaces.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index 9c271ad..d4253a8 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > } > if (!id) { > error_setg(errp, QERR_MISSING_PARAMETER, "id"); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > return NULL; > } > @@ -134,6 +135,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > visit_free(v); > > qemu_opts_set_id(opts, (char *) id); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > QDECREF(pdict); > return obj; The awkwardness increases a bit more, but I don't have better ideas. Reviewed-by: Markus Armbruster <armbru@redhat.com> Peter, can you apply this as a build fix?
On 03/23/17 17:03, Eric Blake wrote: > Commit 9a6d1ac assumed that 'qom-type' could be removed from QemuOpts > with no ill effects. However, this command line proves otherwise: > > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ > -object rng-random,filename=/dev/urandom,id=rng0 \ > -device virtio-rng-pci,rng=rng0 > qemu-system-x86_64: -object rng-random,filename=/dev/urandom,id=rng0: Parameter 'qom-type' is missing > > Fix the regression by restoring qom-type in opts after its temporary > removal that was needed for the duration of user_creatable_add_opts(). > > Reported-by: Richard W. M. Jones <rjones@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > qom/object_interfaces.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index 9c271ad..d4253a8 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > } > if (!id) { > error_setg(errp, QERR_MISSING_PARAMETER, "id"); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > return NULL; > } > @@ -134,6 +135,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) > visit_free(v); > > qemu_opts_set_id(opts, (char *) id); > + qemu_opt_set(opts, "qom-type", type, &error_abort); > g_free(type); > QDECREF(pdict); > return obj; > Tested-by: Laszlo Ersek <lersek@redhat.com>
On 23 March 2017 at 16:24, Markus Armbruster <armbru@redhat.com> wrote: > Eric Blake <eblake@redhat.com> writes: > >> Commit 9a6d1ac assumed that 'qom-type' could be removed from QemuOpts >> with no ill effects. However, this command line proves otherwise: >> >> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ >> -object rng-random,filename=/dev/urandom,id=rng0 \ >> -device virtio-rng-pci,rng=rng0 >> qemu-system-x86_64: -object rng-random,filename=/dev/urandom,id=rng0: Parameter 'qom-type' is missing > > Shows that our test coverage is still inadequate. Also shows that I > should do more manual testing myself. Sorry for the inconvenience! > >> Fix the regression by restoring qom-type in opts after its temporary >> removal that was needed for the duration of user_creatable_add_opts(). >> >> Reported-by: Richard W. M. Jones <rjones@redhat.com> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> --- >> qom/object_interfaces.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c >> index 9c271ad..d4253a8 100644 >> --- a/qom/object_interfaces.c >> +++ b/qom/object_interfaces.c >> @@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) >> } >> if (!id) { >> error_setg(errp, QERR_MISSING_PARAMETER, "id"); >> + qemu_opt_set(opts, "qom-type", type, &error_abort); >> g_free(type); >> return NULL; >> } >> @@ -134,6 +135,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) >> visit_free(v); >> >> qemu_opts_set_id(opts, (char *) id); >> + qemu_opt_set(opts, "qom-type", type, &error_abort); >> g_free(type); >> QDECREF(pdict); >> return obj; > > The awkwardness increases a bit more, but I don't have better ideas. > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Peter, can you apply this as a build fix? Sure; applied to master. -- PMM
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 9c271ad..d4253a8 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) } if (!id) { error_setg(errp, QERR_MISSING_PARAMETER, "id"); + qemu_opt_set(opts, "qom-type", type, &error_abort); g_free(type); return NULL; } @@ -134,6 +135,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) visit_free(v); qemu_opts_set_id(opts, (char *) id); + qemu_opt_set(opts, "qom-type", type, &error_abort); g_free(type); QDECREF(pdict); return obj;
Commit 9a6d1ac assumed that 'qom-type' could be removed from QemuOpts with no ill effects. However, this command line proves otherwise: $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ -object rng-random,filename=/dev/urandom,id=rng0 \ -device virtio-rng-pci,rng=rng0 qemu-system-x86_64: -object rng-random,filename=/dev/urandom,id=rng0: Parameter 'qom-type' is missing Fix the regression by restoring qom-type in opts after its temporary removal that was needed for the duration of user_creatable_add_opts(). Reported-by: Richard W. M. Jones <rjones@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- qom/object_interfaces.c | 2 ++ 1 file changed, 2 insertions(+)