[Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Thomas Huth]

Each device that is instantiatable by the users should be marked with
a category. Since the generic-loader does not fit anywhere else, put
it into the MISC category.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/core/generic-loader.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index fde32cb..be29ae1 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -204,6 +204,7 @@ static void generic_loader_class_init(ObjectClass *klass, void *data)
     dc->unrealize = generic_loader_unrealize;
     dc->props = generic_loader_props;
     dc->desc = "Generic Loader";
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }
 
 static TypeInfo generic_loader_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Cornelia Huck]

On Fri,  5 Oct 2018 11:42:26 +0200
Thomas Huth <thuth@redhat.com> wrote:

> Each device that is instantiatable by the users should be marked with
> a category. Since the generic-loader does not fit anywhere else, put
> it into the MISC category.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/core/generic-loader.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index fde32cb..be29ae1 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -204,6 +204,7 @@ static void generic_loader_class_init(ObjectClass *klass, void *data)
>      dc->unrealize = generic_loader_unrealize;
>      dc->props = generic_loader_props;
>      dc->desc = "Generic Loader";
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>  }
>  
>  static TypeInfo generic_loader_info = {

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Alistair Francis]

On Fri, Oct 5, 2018 at 2:52 AM Thomas Huth <thuth@redhat.com> wrote:
>
> Each device that is instantiatable by the users should be marked with
> a category. Since the generic-loader does not fit anywhere else, put
> it into the MISC category.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/generic-loader.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index fde32cb..be29ae1 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -204,6 +204,7 @@ static void generic_loader_class_init(ObjectClass *klass, void *data)
>      dc->unrealize = generic_loader_unrealize;
>      dc->props = generic_loader_props;
>      dc->desc = "Generic Loader";
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>  }
>
>  static TypeInfo generic_loader_info = {
> --
> 1.8.3.1
>
>

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Peter Maydell]

On 5 October 2018 at 10:42, Thomas Huth <thuth@redhat.com> wrote:
> Each device that is instantiatable by the users should be marked with
> a category.

Presumably we could assert() this somewhere (at which
point we'd find that we have dozens of devices that
fail to set a category bit, I imagine) ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Thomas Huth]

On 2018-10-05 18:32, Peter Maydell wrote:
> On 5 October 2018 at 10:42, Thomas Huth <thuth@redhat.com> wrote:
>> Each device that is instantiatable by the users should be marked with
>> a category.
> 
> Presumably we could assert() this somewhere (at which
> point we'd find that we have dozens of devices that
> fail to set a category bit, I imagine) ?

You bet ... alone in qemu-system-aarch64, "-device help" shows more than
120 devices without a category...

So if we'd want to enforce this, there is a lot of clean-up work needed
first. Once we're in a good shape, I think a "make check" test would be
the right thing to enforce that future devices are always provided with
a category.

 Thomas

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device

[Peter Maydell]

On 8 October 2018 at 07:14, Thomas Huth <thuth@redhat.com> wrote:
> On 2018-10-05 18:32, Peter Maydell wrote:
>> On 5 October 2018 at 10:42, Thomas Huth <thuth@redhat.com> wrote:
>>> Each device that is instantiatable by the users should be marked with
>>> a category.
>>
>> Presumably we could assert() this somewhere (at which
>> point we'd find that we have dozens of devices that
>> fail to set a category bit, I imagine) ?
>
> You bet ... alone in qemu-system-aarch64, "-device help" shows more than
> 120 devices without a category...
>
> So if we'd want to enforce this, there is a lot of clean-up work needed
> first. Once we're in a good shape, I think a "make check" test would be
> the right thing to enforce that future devices are always provided with
> a category.

Asserting in something like the device base class realize seems
to me like a good approach -- that way it gets automatically
covered by the existing test that tries to create every
user creatable device (we do have one of those, right?), and
developers of new devices will spot their error immediately
even if they don't happen to run 'make check'.

thanks
-- PMM