Re: [Qemu-devel] [PATCH v3 2/2] virtio-serial-port: Convert to QOM realize/unrealize

[Amit Shah]

Hi,

I haven't really followed the whole discussion, so can't say much
about it -- overall looks alright.  A couple of minor nits:

>      max_nr_ports = tswap32(port->vser->config.max_nr_ports);
>      if (port->id >= max_nr_ports) {
> -        error_report("virtio-serial-bus: Out-of-range port id specified, 
> max. allowed: %u",
> -                     max_nr_ports - 1);
> -        return -1;
> +        error_setg(errp, "virtio-serial-bus: Out-of-range port id specified, 
> "
> +                         "max. allowed: %u",
> +                   max_nr_ports - 1);

indentation looks off.  Just put this last line on the line above?

> diff --git a/include/hw/virtio/virtio-serial.h 
> b/include/hw/virtio/virtio-serial.h
> index 1d2040b..b1bc9e5 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -77,19 +77,19 @@ typedef struct VirtIOSerialPort VirtIOSerialPort;
>  typedef struct VirtIOSerialPortClass {
>      DeviceClass parent_class;
>  
> -    /* Is this a device that binds with hvc in the guest? */
> -    bool is_console;
> -
>      /*
> -     * The per-port (or per-app) init function that's called when a
> +     * The per-port (or per-app) realize function that's called when a
>       * new device is found on the bus.
>       */
> -    int (*init)(VirtIOSerialPort *port);
> +    DeviceRealize realize;
>      /*
> -     * Per-port exit function that's called when a port gets
> +     * Per-port unrealize function that's called when a port gets
>       * hot-unplugged or removed.
>       */
> -    int (*exit)(VirtIOSerialPort *port);
> +    DeviceUnrealize unrealize;
> +
> +    /* Is this a device that binds with hvc in the guest? */
> +    bool is_console;

is_console is moved unnecessarily?


                Amit