Re: [PATCH 1/9] hw/i386/sgx: Do not open-code qdev_realize_and_unref()

[Philippe Mathieu-Daudé]

On 3/2/23 13:32, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/i386/sgx.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> > index db004d17a6..5ddc5d7ea2 100644
> > --- a/hw/i386/sgx.c
> > +++ b/hw/i386/sgx.c
> > @@ -299,7 +299,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> >                                   &sgx_epc->mr);
> >   
> >       for (list = x86ms->sgx_epc_list; list; list = list->next) {
> > -        obj = object_new("sgx-epc");
> > +        obj = object_new(TYPE_SGX_EPC);
>
> I wonder why this doesn't use qdev_new().

OK.

> >           /* set the memdev link with memory backend */
> >           object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
> > @@ -307,8 +307,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> >           /* set the numa node property for sgx epc object */
> >           object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, 
> > list->value->node,
> >                                &error_fatal);
> > -        object_property_set_bool(obj, "realized", true, &error_fatal);
> > -        object_unref(obj);
> > +        qdev_realize_and_unref(DEVICE(obj), NULL, &error_fatal);
>
> Yes, please!  Must have crept in after I converted all realizations.
> I can see two more:
>
> hw/pci/pcie_sriov.c:        object_property_set_bool(OBJECT(vf), "realized", false, 
> &local_err);
> linux-user/syscall.c:            object_property_set_bool(OBJECT(cpu), 
> "realized", false, NULL);

No: these would be qdev_UNrealize_and_unref(). Do we want it?
Maybe to avoid open-coding it, yes?

> >       }
> >   
> >       if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) {
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks!