1. Patchew
  2. QEMU
  3. [Qemu-devel] [RFC PATCH 0/6] qom: introduce TypeInfo name aliases
  4. View patch

[Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias

Philippe Mathieu-Daudé posted 6 patches 7 years, 9 months ago
[Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Posted by Philippe Mathieu-Daudé 7 years, 9 months ago 
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/e1000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 05a00cba31..2280f7fdf9 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1648,6 +1648,7 @@ typedef struct E1000Info {
     uint16_t   device_id;
     uint8_t    revision;
     uint16_t   phy_id2;
+    const char **aliases;
 } E1000Info;
 
 static void e1000_class_init(ObjectClass *klass, void *data)
@@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
 
 static const E1000Info e1000_devices[] = {
     {
-        .name      = "e1000",
+        .name      = "e1000-82540em",
         .device_id = E1000_DEV_ID_82540EM,
         .revision  = 0x03,
         .phy_id2   = E1000_PHY_ID2_8254xx_DEFAULT,
+        .aliases   = (const char * []) {"e1000", NULL},
     },
     {
         .name      = "e1000-82544gc",
@@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
 
         type_info.name = info->name;
         type_info.parent = TYPE_E1000_BASE;
+        type_info.aliases = info->aliases;
         type_info.class_data = (void *)info;
         type_info.class_init = e1000_class_init;
         type_info.instance_init = e1000_instance_init;
-- 
2.15.1
Re: [Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Posted by Dr. David Alan Gilbert 7 years, 9 months ago 
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/e1000.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 05a00cba31..2280f7fdf9 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1648,6 +1648,7 @@ typedef struct E1000Info {
>      uint16_t   device_id;
>      uint8_t    revision;
>      uint16_t   phy_id2;
> +    const char **aliases;
>  } E1000Info;
>  
>  static void e1000_class_init(ObjectClass *klass, void *data)
> @@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
>  
>  static const E1000Info e1000_devices[] = {
>      {
> -        .name      = "e1000",
> +        .name      = "e1000-82540em",
>          .device_id = E1000_DEV_ID_82540EM,
>          .revision  = 0x03,
>          .phy_id2   = E1000_PHY_ID2_8254xx_DEFAULT,
> +        .aliases   = (const char * []) {"e1000", NULL},
>      },
>      {
>          .name      = "e1000-82544gc",
> @@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
>  
>          type_info.name = info->name;
>          type_info.parent = TYPE_E1000_BASE;
> +        type_info.aliases = info->aliases;
>          type_info.class_data = (void *)info;
>          type_info.class_init = e1000_class_init;
>          type_info.instance_init = e1000_instance_init;

Can you just check that this doesn't break migration compatibility
either way please;  I think there's some alias code somewhere but I
can't remember the details.

One thing I do remember that broke when it previously changed was
entries in PC_COMPAT_* tables like the one in hw/i386/pc_piix.c's
PC_COMPAT_1_3 - so check with an 'info qtree' that the property is OK.

Dave

> -- 
> 2.15.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Posted by Philippe Mathieu-Daudé 7 years, 9 months ago 
Hi David,

On 01/04/2018 01:24 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/net/e1000.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 05a00cba31..2280f7fdf9 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -1648,6 +1648,7 @@ typedef struct E1000Info {
>>      uint16_t   device_id;
>>      uint8_t    revision;
>>      uint16_t   phy_id2;
>> +    const char **aliases;
>>  } E1000Info;
>>  
>>  static void e1000_class_init(ObjectClass *klass, void *data)
>> @@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
>>  
>>  static const E1000Info e1000_devices[] = {
>>      {
>> -        .name      = "e1000",
>> +        .name      = "e1000-82540em",
>>          .device_id = E1000_DEV_ID_82540EM,
>>          .revision  = 0x03,
>>          .phy_id2   = E1000_PHY_ID2_8254xx_DEFAULT,
>> +        .aliases   = (const char * []) {"e1000", NULL},
>>      },
>>      {
>>          .name      = "e1000-82544gc",
>> @@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
>>  
>>          type_info.name = info->name;
>>          type_info.parent = TYPE_E1000_BASE;
>> +        type_info.aliases = info->aliases;
>>          type_info.class_data = (void *)info;
>>          type_info.class_init = e1000_class_init;
>>          type_info.instance_init = e1000_instance_init;
> 
> Can you just check that this doesn't break migration compatibility
> either way please;  I think there's some alias code somewhere but I
> can't remember the details.

Good point.

Something related to qdev-monitor.c?

> One thing I do remember that broke when it previously changed was
> entries in PC_COMPAT_* tables like the one in hw/i386/pc_piix.c's
> PC_COMPAT_1_3 - so check with an 'info qtree' that the property is OK.

Ok, I'll verify and think about a qtest for that.

I took this device because I wanted to test another arch than ARM (which
strongly use FDT).
If there is an issue migrating this particular device we can drop this
patch for this only device, and the series still stands for the ARM/FDT.

Regards,

Phil.
Re: [Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Posted by Dr. David Alan Gilbert 7 years, 9 months ago 
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Hi David,
> 
> On 01/04/2018 01:24 PM, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >>  hw/net/e1000.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> >> index 05a00cba31..2280f7fdf9 100644
> >> --- a/hw/net/e1000.c
> >> +++ b/hw/net/e1000.c
> >> @@ -1648,6 +1648,7 @@ typedef struct E1000Info {
> >>      uint16_t   device_id;
> >>      uint8_t    revision;
> >>      uint16_t   phy_id2;
> >> +    const char **aliases;
> >>  } E1000Info;
> >>  
> >>  static void e1000_class_init(ObjectClass *klass, void *data)
> >> @@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
> >>  
> >>  static const E1000Info e1000_devices[] = {
> >>      {
> >> -        .name      = "e1000",
> >> +        .name      = "e1000-82540em",
> >>          .device_id = E1000_DEV_ID_82540EM,
> >>          .revision  = 0x03,
> >>          .phy_id2   = E1000_PHY_ID2_8254xx_DEFAULT,
> >> +        .aliases   = (const char * []) {"e1000", NULL},
> >>      },
> >>      {
> >>          .name      = "e1000-82544gc",
> >> @@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
> >>  
> >>          type_info.name = info->name;
> >>          type_info.parent = TYPE_E1000_BASE;
> >> +        type_info.aliases = info->aliases;
> >>          type_info.class_data = (void *)info;
> >>          type_info.class_init = e1000_class_init;
> >>          type_info.instance_init = e1000_instance_init;
> > 
> > Can you just check that this doesn't break migration compatibility
> > either way please;  I think there's some alias code somewhere but I
> > can't remember the details.
> 
> Good point.
> 
> Something related to qdev-monitor.c?

The thing I was worrying about was when names end up in the migration
stream;  that can be a bit subtle so the only way is to just check it
doesn't break.

> > One thing I do remember that broke when it previously changed was
> > entries in PC_COMPAT_* tables like the one in hw/i386/pc_piix.c's
> > PC_COMPAT_1_3 - so check with an 'info qtree' that the property is OK.
> 
> Ok, I'll verify and think about a qtest for that.
> 
> I took this device because I wanted to test another arch than ARM (which
> strongly use FDT).
> If there is an issue migrating this particular device we can drop this
> patch for this only device, and the series still stands for the ARM/FDT.

Yes, although my concern was more general, I just had a previous case
where I had to fix e1000 downstream because of renaming; but just keep
in mind other places where names turn up, like the compatibility lists.

Dave

> Regards,
> 
> Phil.
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.