| Message ID | 1408119832-4354-1-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Aug 16, 2014 at 12:23:52AM +0800, Amos Kong wrote: > This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. > > The commit introduced a regression bug, the initialization order of virtio-rng > backend was changed. > > # x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \ > -chardev socket,host=localhost,port=1024,id=chr0 \ > -object rng-egd,chardev=chr0,id=rng0 > > qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not found > > Chardev 'chr0' isn't initialized when we try to open rng backend, More detail: The problem is that vl.c:main() calls object_create() on -object before -chardev options are processed. Moving the object_create() call after chardev is arbitrary and does not work if a chardev depends on an -object. It would have been nice to process command-line options in left-to-right order instead of grouping them by option type. I doubt we can change this now since it would break the command-line. > Conflicts: > hw/virtio/virtio-rng.c This shouldn't be here. I agree with reverting commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526.
On Tue, Aug 19, 2014 at 03:47:20PM +0100, Stefan Hajnoczi wrote: > On Sat, Aug 16, 2014 at 12:23:52AM +0800, Amos Kong wrote: > > This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. > > > > The commit introduced a regression bug, the initialization order of virtio-rng > > backend was changed. > > > > # x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \ > > -chardev socket,host=localhost,port=1024,id=chr0 \ > > -object rng-egd,chardev=chr0,id=rng0 Paolo posted a new patch [1] to fix this bug, above commandline works. > > qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not found > > > > Chardev 'chr0' isn't initialized when we try to open rng backend, > > More detail: > The problem is that vl.c:main() calls object_create() on -object before > -chardev options are processed. Moving the object_create() call after > chardev is arbitrary and does not work if a chardev depends on an > -object. Stefan, can you give an example of the complex relying? So Paolo's patch can't solve this problem? > It would have been nice to process command-line options in left-to-right > order instead of grouping them by option type. I doubt we can change > this now since it would break the command-line. > > > Conflicts: > > hw/virtio/virtio-rng.c > > This shouldn't be here. > > I agree with reverting commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526.
Stefan Hajnoczi <stefanha@gmail.com> writes: > On Sat, Aug 16, 2014 at 12:23:52AM +0800, Amos Kong wrote: >> This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. >> >> The commit introduced a regression bug, the initialization order of virtio-rng >> backend was changed. >> >> # x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \ >> -chardev socket,host=localhost,port=1024,id=chr0 \ >> -object rng-egd,chardev=chr0,id=rng0 >> >> qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not found >> >> Chardev 'chr0' isn't initialized when we try to open rng backend, > > More detail: > The problem is that vl.c:main() calls object_create() on -object before > -chardev options are processed. Moving the object_create() call after > chardev is arbitrary and does not work if a chardev depends on an > -object. > > It would have been nice to process command-line options in left-to-right > order instead of grouping them by option type. I doubt we can change > this now since it would break the command-line. In my private opinion, our command line could really use a thorough breaking. [...]
diff --git a/backends/rng.c b/backends/rng.c index 0f2fc11..26faf44 100644 --- a/backends/rng.c +++ b/backends/rng.c @@ -41,9 +41,9 @@ static bool rng_backend_prop_get_opened(Object *obj, Error **errp) return s->opened; } -static void rng_backend_complete(UserCreatable *uc, Error **errp) +void rng_backend_open(RngBackend *s, Error **errp) { - object_property_set_bool(OBJECT(uc), true, "opened", errp); + object_property_set_bool(OBJECT(s), true, "opened", errp); } static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp) @@ -80,20 +80,12 @@ static void rng_backend_init(Object *obj) NULL); } -static void rng_backend_class_init(ObjectClass *oc, void *data) -{ - UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); - - ucc->complete = rng_backend_complete; -} - static const TypeInfo rng_backend_info = { .name = TYPE_RNG_BACKEND, .parent = TYPE_OBJECT, .instance_size = sizeof(RngBackend), .instance_init = rng_backend_init, .class_size = sizeof(RngBackendClass), - .class_init = rng_backend_class_init, .abstract = true, .interfaces = (InterfaceInfo[]) { { TYPE_USER_CREATABLE }, diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index 03fd04a..9323cbe 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -15,7 +15,6 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-rng.h" #include "sysemu/rng.h" -#include "qom/object_interfaces.h" static bool is_guest_ready(VirtIORNG *vrng) { @@ -157,14 +156,6 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) if (vrng->conf.rng == NULL) { vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); - user_creatable_complete(OBJECT(vrng->conf.default_backend), - &local_err); - if (local_err) { - error_propagate(errp, local_err); - object_unref(OBJECT(vrng->conf.default_backend)); - return; - } - object_property_add_child(OBJECT(dev), "default-backend", OBJECT(vrng->conf.default_backend), @@ -186,6 +177,12 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) virtio_init(vdev, "virtio-rng", VIRTIO_ID_RNG, 0); + rng_backend_open(vrng->rng, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + vrng->vq = virtio_add_queue(vdev, 8, handle_input); vrng->quota_remaining = vrng->conf.max_bytes; diff --git a/include/sysemu/rng.h b/include/sysemu/rng.h index 0a27c9b..7637fac 100644 --- a/include/sysemu/rng.h +++ b/include/sysemu/rng.h @@ -79,4 +79,15 @@ void rng_backend_request_entropy(RngBackend *s, size_t size, * to stop tracking any request. */ void rng_backend_cancel_requests(RngBackend *s); + +/** + * rng_backend_open: + * @s: the backend to open + * @errp: a pointer to return the #Error object if an error occurs. + * + * This function will open the backend if it is not already open. Calling this + * function on an already opened backend will not result in an error. + */ +void rng_backend_open(RngBackend *s, Error **errp); + #endif
This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. The commit introduced a regression bug, the initialization order of virtio-rng backend was changed. # x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \ -chardev socket,host=localhost,port=1024,id=chr0 \ -object rng-egd,chardev=chr0,id=rng0 qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not found Chardev 'chr0' isn't initialized when we try to open rng backend, Signed-off-by: Amos Kong <akong@redhat.com> Conflicts: hw/virtio/virtio-rng.c --- backends/rng.c | 12 ++---------- hw/virtio/virtio-rng.c | 15 ++++++--------- include/sysemu/rng.h | 11 +++++++++++ 3 files changed, 19 insertions(+), 19 deletions(-)