Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2015-08-08 02:25, Ouyang, Changchun wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jan Kiszka
>> Sent: Saturday, August 8, 2015 1:21 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
> 
> Vchost should be vhost in the title

Oops. Unless I need to resend for some other reason, I guess the commit
can fix this up.

> 
>>
>> On VHOST_*_RESET_OWNER, we reinitialize the device but without telling
>> the application. That will cause crashes when it continues to invoke vhost
>> services on the device. Fix it by calling the destruction hook if the device is
>> still in use.
> What's your qemu version?

git head, see my other reply for details.

> Any validation work on this patch?

What do you mean with this? Test cases? Or steps to reproduce? For the
latter, just fire up a recent qemu, let the guest enable the virtio
device, then reboot or simply terminate qemu.

>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> This is the surprisingly simple answer to my questions in
>> http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
>>
>>  lib/librte_vhost/virtio-net.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c index
>> b520ec5..3c5b5b2 100644
>> --- a/lib/librte_vhost/virtio-net.c
>> +++ b/lib/librte_vhost/virtio-net.c
>> @@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
>>
>>  	ll_dev = get_config_ll_entry(ctx);
>>
>> +	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
>> +		notify_ops->destroy_device(&ll_dev->dev);
>> +
> 
> I am not sure whether destroy_device here will affect the second time init_device(below) and new_device(after the reset) or not.
> Need validation.

Cannot follow, what do you mean with "second time"? If the callback
could invoke something that causes cleanup_device to be called as well?
That's at least not the case with vhost-switch, but I'm far from being
familiar with the API to asses if that is possible in general.

Jan

> 
>>  	cleanup_device(&ll_dev->dev);
>>  	init_device(&ll_dev->dev);
>>
>> --
>> 2.1.4


-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux