Re: [Qemu-devel] [PATCH V4 0/7] CAN bus support for QEMU (SJA1000 PCI so far)

[Pavel Pisa]

Hello Paolo,

On Friday 26 of January 2018 12:12:32 Paolo Bonzini wrote:
> Coincidentially, I have some time on a flight next Monday. :)  I
> obviously cannot really _test_ anything, but I can at least do the
> changes below and set up all the QOM boilerplate.

thanks much for offer to help.

Deniz Eren or Oleksij Rempel can test your changes as well.

I have prepared branch "can-pci-qom" in GitLab repository

  https://gitlab.fel.cvut.cz/canbus/qemu-canbus

  https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom

and I have updated all PCI devices to use realize() instead of init(),
eliminated all uses of exit() and changed error reporting to use
error_setg() and error_setg_errno(). So I think that there is
no fatal blocker in these files. Problematic is setup
of connect_to_host variant

 can_bus_set_connect_to_host(can_bus_connect_to_host_socketcan);

in "hw/can/can_socketcan.c" and location of this file
in HW. I keep it there to have all CAN support *.c
files in single place for now.

I have studied "tests/check-qom-proplist.c" for while
but I expect that you will be much more successfull
and efficient to define mainline acceptable object model.

Thanks again,

Pavel

> >> The best way would be a QOM object.  That is, you would do
> >>
> >>    -object can-bus,id=canbus0
> >>    -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0
> >>    -device kvaser_pci,canbus=canbus0
> >
> > I would prefer to allow CAN emulation to be used without
> > explicit can-bus object creation specified on the command line.
> > The bus object could be created when requested by
> > can-host-socketcan or device (kvaser_pci) automatically.
>
> It could be fine to allow "-device kvaser_pci" to automatically create a
> private bus.  However, IMO it's better to be explicit in the case where
> multiple objects (e.g. can-host-socketcan and the kvaser_pci device, or
> two controllers) want to connect to the same object.
>
> >> Separating the QOM objects is a bit more work, but it makes the
> >> semantics clearer.  The classes would be:
> >>
> >> - can-bus and an abstract class can-host, which would inherit directly
> >> from TYPE_OBJECT and implement TYPE_USER_CREATABLE
> >>
> >> - can-host-socketcan, which would inherit from can-host (and take the
> >> TYPE_USER_CREATABLE implementation from there)
> >>
> >> while CanBusClientState and CanBusClientInfo need not be QOMified.
> >
> > May it be, can-host-socketcan can be based directly on TYPE_OBJECT,
> > because only QEMU CAN bus specific part is CanBusClientState which
> > allows it to connect to the CAN bus same way as other CAN controllers
> > connect to the bus.
>
> The abstract class "can-host" is useful to keep can-host-socketcan free
> of things that are not specific to SocketCAN, but it's just a small detail.