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

Thomas Huth posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1538732546-2284-1-git-send-email-thuth@redhat.com
Test docker-clang@ubuntu failed
Test checkpatch passed
hw/core/generic-loader.c | 1 +
1 file changed, 1 insertion(+)
[Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device
Posted by Thomas Huth 7 years, 1 month ago
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
Posted by Alistair Francis 7 years, 1 month ago
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
Posted by Peter Maydell 7 years, 1 month ago
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
Posted by Thomas Huth 7 years ago
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
Posted by Peter Maydell 7 years ago
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

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Set a category for the generic-loader device
Posted by Cornelia Huck 7 years, 1 month ago
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>