qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ide: Get rid of IDEDrive struct


From: John Snow
Subject: Re: [PATCH] ide: Get rid of IDEDrive struct
Date: Fri, 7 Aug 2020 20:01:43 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

On 8/5/20 4:41 PM, Peter Maydell wrote:
On Wed, 5 Aug 2020 at 20:49, Eduardo Habkost <ehabkost@redhat.com> wrote:

The struct had a single field (IDEDevice dev), and is only used
in the QOM type declarations and property lists.  We can simply
use the IDEDevice struct directly instead.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
@@ -327,7 +323,6 @@ static void ide_hd_class_init(ObjectClass *klass, void 
*data)
  static const TypeInfo ide_hd_info = {
      .name          = "ide-hd",
      .parent        = TYPE_IDE_DEVICE,
-    .instance_size = sizeof(IDEDrive),
      .class_init    = ide_hd_class_init,
  };

This is one of those areas where this change works and reduces
amount of code, but on the other hand it means the QOM type
doesn't follow the common pattern for a leaf type of:
  * it has a struct
  * it has cast macros that cast to that struct
  * the typeinfo instance_size is the size of that struct
(it wasn't exactly following this pattern before, of course).

We define in https://wiki.qemu.org/Documentation/QOMConventions
(in the 'When to create class types and macros' bit at the bottom)
what we expect for whether to provide class cast macros/a
class struct/class_size in the TypeInfo, essentially recommending
that types follow one of two patterns (simple leaf class with no
methods or class members, vs everything else) even if in a
particular case you could take a short-cut and not define
everything. We haven't really defined similar "this is the
standard pattern, provide it all even if you don't strictly
need it" rules for the instance struct/macros. Maybe we should?

Just a thought, not a nak; I know we have quite a number
of types that take this kind of "we don't really need to
provide all the standard QOM macros/structs/etc" approach
(some of which I wrote!).


I'll defer to your judgment here. The IDE stuff is very confusing, but I don't know the best way to wrangle it to make it less confusing.

I assume at some point migration compatibility gets in the way of any REAL refactoring that might start to make this code make more sense.

Open to suggestions.

--js




reply via email to

[Prev in Thread] Current Thread [Next in Thread]