[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-serial: add enable_backend callback
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-serial: add enable_backend callback |
Date: |
Tue, 11 Jul 2017 11:31:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 10/07/2017 18:39, Michael S. Tsirkin wrote:
> On Mon, Jul 10, 2017 at 04:13:54PM +0200, Laurent Vivier wrote:
>>> static void virtconsole_realize(DeviceState *dev, Error **errp)
>>> {
>>> VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
>>> @@ -233,6 +253,7 @@ static void virtserialport_class_init(ObjectClass
>>> *klass, void *data)
>>> k->unrealize = virtconsole_unrealize;
>>> k->have_data = flush_buf;
>>> k->set_guest_connected = set_guest_connected;
>>> + k->enable_backend = virtconsole_enable_backend;
>>
>> Why don't you register a vm_state change handler to change the state of
>> the virtconsole according to the state of the machine instead of adding
>> a new function in the VirtIOSerialPortClass?
>>
>> See a23a6d1 ("virtio-rng: stop virtqueue while the CPU is stopped")
>>
>> Thanks,
>> Laurent
>
>
> In fact that commit does it the wrong way IMHO.
>
> The order of this call wrt other virtio calls is not
> guaranteed.
>
> IMHO the right way is to set a vm state change handler in VirtioBusClass
> or status change handler in VirtioDeviceClass.
>
OK, so I guess Pavel's solution is the good one as it calls his handler
from virtio-serial-bus set_status() handler which is called by
virtio_vmstate_change() function, which is registered with
qemu_add_vm_change_state_handler() by virtio_init()...
Thanks,
Laurent