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

["Andreas Färber" <afaerber@suse.de>]

Hi Amit,

Am 12.03.2014 13:41, schrieb 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?

Done. (Was intentional to align error string, but no need to keep on
separate lines after breaking.)

> 
>> 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?

Fixed. (Previously fields parent_realize were added right after parent_obj.)

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg