| Message ID | 20180104144046.30793-3-f4bug@amsat.org |
|---|---|
| State | Rejected |
| Headers | show |
| Series | qom: introduce TypeInfo name aliases | expand |
* 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
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.
* 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
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;
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/net/e1000.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)