On Sat, Nov 01, 2014 at 05:46:14PM +0100, Andreas Färber wrote:
> Hi,
>
> Am 01.11.2014 um 16:56 schrieb Eduardo Habkost:
> > Extract the DeviceClass lookup from qdev_device_add() to a separate
> > function.
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > ---
> > qdev-monitor.c | 70
> > +++++++++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 42 insertions(+), 28 deletions(-)
> >
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index fac7d17..982f3f4 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -180,6 +180,44 @@ static const char *find_typename_by_alias(const char
> > *alias)
> > return NULL;
> > }
> >
> > +static DeviceClass *qdev_get_device_class(const char **driver, Error
> > **errp)
>
> Since this does nothing qdev-specific, what about
> device_get_class_by_name()? The only that's not generically suitable for
> hw/core/ is the "driver" naming in the error handling; otherwise it
> looks very similar to the CPUClass hooks I added a while back.
Well, I don't know where's the line that divides "qdev-specific" from
"DeviceState-specific". The distinction seems arbitrary to me, and I
won't see any problem if somebody moves all qdev-*.c files inside
hw/core. :)
I assume that at least qdev_alias_table (which is used by both
qdev_device_add() and qdev_device_help()) can be considered
qdev-specific (but just because the table is currently inside
qdev-monitor.c).
Anyway, the only point of this function is to reduce code duplication
between qdev_device_add() and qdev_device_help() while fixing the
"-device ,help" crashes. If somebody wants to move any of the existing
qdev-*.c logic to hw/core later, I won't complain (but I wouldn't want
to do that after hard freeze).
>
> > +{
> > + ObjectClass *oc;
> > + DeviceClass *dc;
> > +
> > + oc = object_class_by_name(*driver);
> > + if (!oc) {
> > + const char *typename = find_typename_by_alias(*driver);
> > +
> > + if (typename) {
> > + *driver = typename;
> > + oc = object_class_by_name(*driver);
> > + }
> > + }
> > +
> > + if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {
> > + error_setg(errp, "'%s' is not a valid device model name", *driver);
> > + return NULL;
> > + }
> > +
> > + if (object_class_is_abstract(oc)) {
> > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
> > + "non-abstract device type");
> > + return NULL;
> > + }
> > +
>
> See 3/3 for whether we may want to return here.
The point of this function is to allow reuse of checks that are
necessary on both qdev_device_add() and qdev_device_help(), and both
functions need to reject abstract and
cannot_instantiate_with_device_add_yet classes.
--
Eduardo
