Re: [Qemu-devel] [PATCH v1 4/4] virtio_rng: replace custom backend API with UserCreatable.complete() callback

[Amos Kong <akong@redhat.com>]

On Thu, Jan 16, 2014 at 05:34:39PM +0100, Igor Mammedov wrote:
> in addition fix default backend leak by releasing it if its
> initialization failed.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Hi Igor,

This patch introduced a regression.

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.

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1128095

> ---
>  backends/rng.c         |   12 ++++++++++--
>  hw/virtio/virtio-rng.c |   15 +++++++++------
>  include/sysemu/rng.h   |   11 -----------
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/backends/rng.c b/backends/rng.c
> index 7bab806..8b8d5a4 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;
>  }
>  
> -void rng_backend_open(RngBackend *s, Error **errp)
> +static void rng_backend_complete(UserCreatable *uc, Error **errp)
>  {
> -    object_property_set_bool(OBJECT(s), true, "opened", errp);
> +    object_property_set_bool(OBJECT(uc), true, "opened", errp);
>  }
>  
>  static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp)
> @@ -77,12 +77,20 @@ 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 755fdee..a16e3bc 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -15,6 +15,7 @@
>  #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)
>  {
> @@ -148,6 +149,14 @@ 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),
> @@ -166,12 +175,6 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> -    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);
>  
>      assert(vrng->conf.max_bytes <= INT64_MAX);
> diff --git a/include/sysemu/rng.h b/include/sysemu/rng.h
> index 7637fac..0a27c9b 100644
> --- a/include/sysemu/rng.h
> +++ b/include/sysemu/rng.h
> @@ -79,15 +79,4 @@ 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
> -- 
> 1.7.1
> 

-- 
			Amos.