This will make the type name constant consistent with the name of
the type checking macro.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org
---
 include/hw/timer/i8254.h | 4 ++--
 hw/timer/i8254.c         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index 1a522a2457..ddd925074f 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
 DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
                      PIT_COMMON, TYPE_PIT_COMMON)
 
-#define TYPE_I8254 "isa-pit"
+#define TYPE_PIT "isa-pit"
 #define TYPE_KVM_I8254 "kvm-pit"
 
 static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
@@ -54,7 +54,7 @@ static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
     DeviceState *dev;
     ISADevice *d;
 
-    d = isa_new(TYPE_I8254);
+    d = isa_new(TYPE_PIT);
     dev = DEVICE(d);
     qdev_prop_set_uint32(dev, "iobase", base);
     isa_realize_and_unref(d, bus, &error_fatal);
diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
index c01ee2c72a..86f455f67e 100644
--- a/hw/timer/i8254.c
+++ b/hw/timer/i8254.c
@@ -39,7 +39,7 @@
 
 typedef struct PITClass PITClass;
 DECLARE_CLASS_CHECKERS(PITClass, PIT,
-                       TYPE_I8254)
+                       TYPE_PIT)
 
 struct PITClass {
     PITCommonClass parent_class;
@@ -370,7 +370,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pit_info = {
-    .name          = TYPE_I8254,
+    .name          = TYPE_PIT,
     .parent        = TYPE_PIT_COMMON,
     .instance_size = sizeof(PITCommonState),
     .class_init    = pit_class_initfn,
-- 
2.26.2

On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> This will make the type name constant consistent with the name of
> the type checking macro.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/timer/i8254.h | 4 ++--
>  hw/timer/i8254.c         | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> index 1a522a2457..ddd925074f 100644
> --- a/include/hw/timer/i8254.h
> +++ b/include/hw/timer/i8254.h
> @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
>  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
>                       PIT_COMMON, TYPE_PIT_COMMON)
>  
> -#define TYPE_I8254 "isa-pit"
> +#define TYPE_PIT "isa-pit"

I disagree with this patch, as we have various PIT and only one I8254.

>  #define TYPE_KVM_I8254 "kvm-pit"
>  
>  static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
> @@ -54,7 +54,7 @@ static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
>      DeviceState *dev;
>      ISADevice *d;
>  
> -    d = isa_new(TYPE_I8254);
> +    d = isa_new(TYPE_PIT);
>      dev = DEVICE(d);
>      qdev_prop_set_uint32(dev, "iobase", base);
>      isa_realize_and_unref(d, bus, &error_fatal);
> diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
> index c01ee2c72a..86f455f67e 100644
> --- a/hw/timer/i8254.c
> +++ b/hw/timer/i8254.c
> @@ -39,7 +39,7 @@
>  
>  typedef struct PITClass PITClass;
>  DECLARE_CLASS_CHECKERS(PITClass, PIT,
> -                       TYPE_I8254)
> +                       TYPE_PIT)
>  
>  struct PITClass {
>      PITCommonClass parent_class;
> @@ -370,7 +370,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo pit_info = {
> -    .name          = TYPE_I8254,
> +    .name          = TYPE_PIT,
>      .parent        = TYPE_PIT_COMMON,
>      .instance_size = sizeof(PITCommonState),
>      .class_init    = pit_class_initfn,
> 

On Thu, Sep 03, 2020 at 02:47:03PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > This will make the type name constant consistent with the name of
> > the type checking macro.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> >  include/hw/timer/i8254.h | 4 ++--
> >  hw/timer/i8254.c         | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> > index 1a522a2457..ddd925074f 100644
> > --- a/include/hw/timer/i8254.h
> > +++ b/include/hw/timer/i8254.h
> > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
> >  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
> >                       PIT_COMMON, TYPE_PIT_COMMON)
> >  
> > -#define TYPE_I8254 "isa-pit"
> > +#define TYPE_PIT "isa-pit"
> 
> I disagree with this patch, as we have various PIT and only one I8254.

I was unsure about this, and I agree with your point.  I will
suggest renaming the PIT macro to I8254 instead.

-- 
Eduardo

On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote:
> On Thu, Sep 03, 2020 at 02:47:03PM +0200, Philippe Mathieu-Daudé wrote:
> > On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > > This will make the type name constant consistent with the name of
> > > the type checking macro.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: qemu-devel@nongnu.org
> > > ---
> > >  include/hw/timer/i8254.h | 4 ++--
> > >  hw/timer/i8254.c         | 4 ++--
> > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> > > index 1a522a2457..ddd925074f 100644
> > > --- a/include/hw/timer/i8254.h
> > > +++ b/include/hw/timer/i8254.h
> > > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
> > >  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
> > >                       PIT_COMMON, TYPE_PIT_COMMON)
> > >  
> > > -#define TYPE_I8254 "isa-pit"
> > > +#define TYPE_PIT "isa-pit"
> > 
> > I disagree with this patch, as we have various PIT and only one I8254.
> 
> I was unsure about this, and I agree with your point.  I will
> suggest renaming the PIT macro to I8254 instead.

IMHO the macro name should be directly related to the object name
string with non-alnum characters replaced by underscore.

ie since the object type is "isa-pit", then the macro should be
TYPE_ISA_PIT

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Thu, Sep 03, 2020 at 05:44:29PM +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote:
> > On Thu, Sep 03, 2020 at 02:47:03PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > > > This will make the type name constant consistent with the name of
> > > > the type checking macro.
> > > > 
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Cc: qemu-devel@nongnu.org
> > > > ---
> > > >  include/hw/timer/i8254.h | 4 ++--
> > > >  hw/timer/i8254.c         | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> > > > index 1a522a2457..ddd925074f 100644
> > > > --- a/include/hw/timer/i8254.h
> > > > +++ b/include/hw/timer/i8254.h
> > > > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
> > > >  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
> > > >                       PIT_COMMON, TYPE_PIT_COMMON)
> > > >  
> > > > -#define TYPE_I8254 "isa-pit"
> > > > +#define TYPE_PIT "isa-pit"
> > > 
> > > I disagree with this patch, as we have various PIT and only one I8254.
> > 
> > I was unsure about this, and I agree with your point.  I will
> > suggest renaming the PIT macro to I8254 instead.
> 
> IMHO the macro name should be directly related to the object name
> string with non-alnum characters replaced by underscore.
> 
> ie since the object type is "isa-pit", then the macro should be
> TYPE_ISA_PIT

I think that's a good idea in this specific case because it's a
short name (I will do it).  But I don't think we'll be able to
always follow that rule, as the QOM type name is user-visible.

-- 
Eduardo

On 9/3/20 6:55 PM, Eduardo Habkost wrote:
> On Thu, Sep 03, 2020 at 05:44:29PM +0100, Daniel P. Berrangé wrote:
>> On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote:
>>> On Thu, Sep 03, 2020 at 02:47:03PM +0200, Philippe Mathieu-Daudé wrote:
>>>> On 9/3/20 12:42 AM, Eduardo Habkost wrote:
>>>>> This will make the type name constant consistent with the name of
>>>>> the type checking macro.
>>>>>
>>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>> ---
>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>>> Cc: qemu-devel@nongnu.org
>>>>> ---
>>>>>  include/hw/timer/i8254.h | 4 ++--
>>>>>  hw/timer/i8254.c         | 4 ++--
>>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
>>>>> index 1a522a2457..ddd925074f 100644
>>>>> --- a/include/hw/timer/i8254.h
>>>>> +++ b/include/hw/timer/i8254.h
>>>>> @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
>>>>>  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
>>>>>                       PIT_COMMON, TYPE_PIT_COMMON)
>>>>>  
>>>>> -#define TYPE_I8254 "isa-pit"
>>>>> +#define TYPE_PIT "isa-pit"
>>>>
>>>> I disagree with this patch, as we have various PIT and only one I8254.
>>>
>>> I was unsure about this, and I agree with your point.  I will
>>> suggest renaming the PIT macro to I8254 instead.
>>
>> IMHO the macro name should be directly related to the object name
>> string with non-alnum characters replaced by underscore.
>>
>> ie since the object type is "isa-pit", then the macro should be
>> TYPE_ISA_PIT
> 
> I think that's a good idea in this specific case because it's a
> short name (I will do it).  But I don't think we'll be able to
> always follow that rule, as the QOM type name is user-visible.

Only user-visible if user-creatable, right?

On Thu, Sep 03, 2020 at 09:26:16PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/3/20 6:55 PM, Eduardo Habkost wrote:
> > On Thu, Sep 03, 2020 at 05:44:29PM +0100, Daniel P. Berrangé wrote:
> >> On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote:
> >>> On Thu, Sep 03, 2020 at 02:47:03PM +0200, Philippe Mathieu-Daudé wrote:
> >>>> On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> >>>>> This will make the type name constant consistent with the name of
> >>>>> the type checking macro.
> >>>>>
> >>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>>> ---
> >>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>>>> Cc: qemu-devel@nongnu.org
> >>>>> ---
> >>>>>  include/hw/timer/i8254.h | 4 ++--
> >>>>>  hw/timer/i8254.c         | 4 ++--
> >>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> >>>>> index 1a522a2457..ddd925074f 100644
> >>>>> --- a/include/hw/timer/i8254.h
> >>>>> +++ b/include/hw/timer/i8254.h
> >>>>> @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass;
> >>>>>  DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass,
> >>>>>                       PIT_COMMON, TYPE_PIT_COMMON)
> >>>>>  
> >>>>> -#define TYPE_I8254 "isa-pit"
> >>>>> +#define TYPE_PIT "isa-pit"
> >>>>
> >>>> I disagree with this patch, as we have various PIT and only one I8254.
> >>>
> >>> I was unsure about this, and I agree with your point.  I will
> >>> suggest renaming the PIT macro to I8254 instead.
> >>
> >> IMHO the macro name should be directly related to the object name
> >> string with non-alnum characters replaced by underscore.
> >>
> >> ie since the object type is "isa-pit", then the macro should be
> >> TYPE_ISA_PIT
> > 
> > I think that's a good idea in this specific case because it's a
> > short name (I will do it).  But I don't think we'll be able to
> > always follow that rule, as the QOM type name is user-visible.
> 
> Only user-visible if user-creatable, right?

All of them are user-visible.  All devices have their properties
configurable using -global, and are visible in the QOM and qdev
trees.

It doesn't mean we can't change them, but it means that changing
them is more than just code refactoring.

-- 
Eduardo