| Message ID | 157017473006.331610.2983143972519884544.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | xive: Make some device types not user creatable | expand |
On 04/10/2019 09:38, Greg Kurz wrote: > Some device types of the XIVE model are exposed to the QEMU command > line: > > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive > name "xive-end-source", desc "XIVE END Source" > name "xive-source", desc "XIVE Interrupt Source" > name "xive-tctx", desc "XIVE Interrupt Thread Context" > > These are internal devices that shouldn't be instantiable by the > user. By the way, they can't be because their respective realize > functions expect link properties that can't be set from the command > line: > > qemu-system-ppc64: -device xive-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: > Property '.cpu' not found > > Hide them by setting dc->user_creatable to false in their respective > class init functions. > > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > hw/intc/xive.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 29df06df1136..6c54a35fd4bb 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) > dc->realize = xive_tctx_realize; > dc->unrealize = xive_tctx_unrealize; > dc->vmsd = &vmstate_xive_tctx; > + dc->user_creatable = false; > } > > static const TypeInfo xive_tctx_info = { > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) > dc->props = xive_source_properties; > dc->realize = xive_source_realize; > dc->vmsd = &vmstate_xive_source; > + dc->user_creatable = false; > } > > static const TypeInfo xive_source_info = { > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) > dc->desc = "XIVE END Source"; > dc->props = xive_end_source_properties; > dc->realize = xive_end_source_realize; > + dc->user_creatable = false; > } > > static const TypeInfo xive_end_source_info = { >
Hi Greg, On 10/4/19 9:38 AM, Greg Kurz wrote: > Some device types of the XIVE model are exposed to the QEMU command > line: > > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive > name "xive-end-source", desc "XIVE END Source" > name "xive-source", desc "XIVE Interrupt Source" > name "xive-tctx", desc "XIVE Interrupt Thread Context" > > These are internal devices that shouldn't be instantiable by the > user. By the way, they can't be because their respective realize > functions expect link properties that can't be set from the command > line: > > qemu-system-ppc64: -device xive-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: > Property '.cpu' not found Why do you have to test that manually, isn't it what tests/device-introspect-test.c::test_one_device does? > Hide them by setting dc->user_creatable to false in their respective > class init functions. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/intc/xive.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 29df06df1136..6c54a35fd4bb 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) > dc->realize = xive_tctx_realize; > dc->unrealize = xive_tctx_unrealize; > dc->vmsd = &vmstate_xive_tctx; > + dc->user_creatable = false; > } > > static const TypeInfo xive_tctx_info = { > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) > dc->props = xive_source_properties; > dc->realize = xive_source_realize; > dc->vmsd = &vmstate_xive_source; > + dc->user_creatable = false; > } > > static const TypeInfo xive_source_info = { > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) > dc->desc = "XIVE END Source"; > dc->props = xive_end_source_properties; > dc->realize = xive_end_source_realize; > + dc->user_creatable = false; > } > > static const TypeInfo xive_end_source_info = { > >
On Fri, 4 Oct 2019 11:17:30 +0200 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Greg, > > On 10/4/19 9:38 AM, Greg Kurz wrote: > > Some device types of the XIVE model are exposed to the QEMU command > > line: > > > > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive > > name "xive-end-source", desc "XIVE END Source" > > name "xive-source", desc "XIVE Interrupt Source" > > name "xive-tctx", desc "XIVE Interrupt Thread Context" > > > > These are internal devices that shouldn't be instantiable by the > > user. By the way, they can't be because their respective realize > > functions expect link properties that can't be set from the command > > line: > > > > qemu-system-ppc64: -device xive-source: required link 'xive' not found: > > Property '.xive' not found > > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: > > Property '.xive' not found > > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: > > Property '.cpu' not found > > Why do you have to test that manually, isn't it what > tests/device-introspect-test.c::test_one_device does? > Heh probably because I wasn't aware of it :) And BTW, test_one_device() can't help here since it only cares about 'device_add foo,help' not crashing QEMU: help = qtest_hmp(qts, "device_add \"%s,help\"", type); as explained in a comment at the beginning of the file. /* * Covers QMP device-list-properties and HMP device_add help. We * currently don't check that their output makes sense, only that QEMU * survives. Useful since we've had an astounding number of crash * bugs around here. */ > > Hide them by setting dc->user_creatable to false in their respective > > class init functions. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/intc/xive.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > > index 29df06df1136..6c54a35fd4bb 100644 > > --- a/hw/intc/xive.c > > +++ b/hw/intc/xive.c > > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) > > dc->realize = xive_tctx_realize; > > dc->unrealize = xive_tctx_unrealize; > > dc->vmsd = &vmstate_xive_tctx; > > + dc->user_creatable = false; > > } > > > > static const TypeInfo xive_tctx_info = { > > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) > > dc->props = xive_source_properties; > > dc->realize = xive_source_realize; > > dc->vmsd = &vmstate_xive_source; > > + dc->user_creatable = false; > > } > > > > static const TypeInfo xive_source_info = { > > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) > > dc->desc = "XIVE END Source"; > > dc->props = xive_end_source_properties; > > dc->realize = xive_end_source_realize; > > + dc->user_creatable = false; > > } > > > > static const TypeInfo xive_end_source_info = { > > > >
On Fri, Oct 04, 2019 at 09:38:50AM +0200, Greg Kurz wrote: 65;5603;1c> Some device types of the XIVE model are exposed to the QEMU command > line: > > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive > name "xive-end-source", desc "XIVE END Source" > name "xive-source", desc "XIVE Interrupt Source" > name "xive-tctx", desc "XIVE Interrupt Thread Context" > > These are internal devices that shouldn't be instantiable by the > user. By the way, they can't be because their respective realize > functions expect link properties that can't be set from the command > line: > > qemu-system-ppc64: -device xive-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: > Property '.cpu' not found > > Hide them by setting dc->user_creatable to false in their respective > class init functions. > > Signed-off-by: Greg Kurz <groug@kaod.org> Applied to ppc-for-4.2. > --- > hw/intc/xive.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 29df06df1136..6c54a35fd4bb 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) > dc->realize = xive_tctx_realize; > dc->unrealize = xive_tctx_unrealize; > dc->vmsd = &vmstate_xive_tctx; > + dc->user_creatable = false; > } > > static const TypeInfo xive_tctx_info = { > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) > dc->props = xive_source_properties; > dc->realize = xive_source_realize; > dc->vmsd = &vmstate_xive_source; > + dc->user_creatable = false; > } > > static const TypeInfo xive_source_info = { > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) > dc->desc = "XIVE END Source"; > dc->props = xive_end_source_properties; > dc->realize = xive_end_source_realize; > + dc->user_creatable = false; > } > > static const TypeInfo xive_end_source_info = { >
Greg Kurz <groug@kaod.org> writes: > Some device types of the XIVE model are exposed to the QEMU command > line: > > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive > name "xive-end-source", desc "XIVE END Source" > name "xive-source", desc "XIVE Interrupt Source" > name "xive-tctx", desc "XIVE Interrupt Thread Context" > > These are internal devices that shouldn't be instantiable by the > user. By the way, they can't be because their respective realize > functions expect link properties that can't be set from the command > line: > > qemu-system-ppc64: -device xive-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: > Property '.xive' not found > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: > Property '.cpu' not found > > Hide them by setting dc->user_creatable to false in their respective > class init functions. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/intc/xive.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 29df06df1136..6c54a35fd4bb 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) > dc->realize = xive_tctx_realize; > dc->unrealize = xive_tctx_unrealize; > dc->vmsd = &vmstate_xive_tctx; > + dc->user_creatable = false; > } > > static const TypeInfo xive_tctx_info = { > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) > dc->props = xive_source_properties; > dc->realize = xive_source_realize; > dc->vmsd = &vmstate_xive_source; > + dc->user_creatable = false; > } > > static const TypeInfo xive_source_info = { > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) > dc->desc = "XIVE END Source"; > dc->props = xive_end_source_properties; > dc->realize = xive_end_source_realize; > + dc->user_creatable = false; > } > > static const TypeInfo xive_end_source_info = { These all need a comment like the existing ->user_creatable = false have. Your commit message mentions link properties. Based on that: /* * Reason: link property 'NAME-OF-PROP' needs to be wired up. */ Rather minimal, though. Several existing similar cases are a bit more specific, which is nice: /* * Reason: part of WHATEVER, needs to be wired up by FUNCTION(). */ or if there is or could be more than one FUNCTION(): /* * Reason: part of WHATEVER, needs to be wired up, e.g. by FUNCTION(). */ David queued your patch already. If it goes into master without such comments, please post them as a follow-up patch.
diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 29df06df1136..6c54a35fd4bb 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) dc->realize = xive_tctx_realize; dc->unrealize = xive_tctx_unrealize; dc->vmsd = &vmstate_xive_tctx; + dc->user_creatable = false; } static const TypeInfo xive_tctx_info = { @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data) dc->props = xive_source_properties; dc->realize = xive_source_realize; dc->vmsd = &vmstate_xive_source; + dc->user_creatable = false; } static const TypeInfo xive_source_info = { @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) dc->desc = "XIVE END Source"; dc->props = xive_end_source_properties; dc->realize = xive_end_source_realize; + dc->user_creatable = false; } static const TypeInfo xive_end_source_info = {
Some device types of the XIVE model are exposed to the QEMU command line: $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive name "xive-end-source", desc "XIVE END Source" name "xive-source", desc "XIVE Interrupt Source" name "xive-tctx", desc "XIVE Interrupt Thread Context" These are internal devices that shouldn't be instantiable by the user. By the way, they can't be because their respective realize functions expect link properties that can't be set from the command line: qemu-system-ppc64: -device xive-source: required link 'xive' not found: Property '.xive' not found qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: Property '.xive' not found qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: Property '.cpu' not found Hide them by setting dc->user_creatable to false in their respective class init functions. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/intc/xive.c | 3 +++ 1 file changed, 3 insertions(+)