Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-19 Thread Dr. David Alan Gilbert
* Dima Stepanov (dimas...@yandex-team.ru) wrote: > On Mon, May 18, 2020 at 10:53:59AM +0100, Dr. David Alan Gilbert wrote: > > * Dima Stepanov (dimas...@yandex-team.ru) wrote: > > > On Mon, May 18, 2020 at 10:50:39AM +0800, Jason Wang wrote: > > > > > > > > On 2020/5/16 上午12:54, Dima Stepanov

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-19 Thread Michael S. Tsirkin
On Fri, May 15, 2020 at 07:54:57PM +0300, Dima Stepanov wrote: > On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: > > > > On 2020/5/13 下午5:47, Dima Stepanov wrote: > > >>> case CHR_EVENT_CLOSED: > > >>> /* a close event may happen during a read/write, but vhost > > >>>

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-19 Thread Dima Stepanov
On Wed, May 13, 2020 at 01:56:18PM +0800, Jason Wang wrote: > > On 2020/5/13 下午12:15, Michael S. Tsirkin wrote: > >On Tue, May 12, 2020 at 12:35:30PM +0300, Dima Stepanov wrote: > >>On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: > >>>On 2020/5/11 下午5:25, Dima Stepanov wrote: > On

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-19 Thread Dima Stepanov
On Mon, May 18, 2020 at 10:53:59AM +0100, Dr. David Alan Gilbert wrote: > * Dima Stepanov (dimas...@yandex-team.ru) wrote: > > On Mon, May 18, 2020 at 10:50:39AM +0800, Jason Wang wrote: > > > > > > On 2020/5/16 上午12:54, Dima Stepanov wrote: > > > >On Thu, May 14, 2020 at 03:34:24PM +0800, Jason

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-18 Thread Dr. David Alan Gilbert
* Dima Stepanov (dimas...@yandex-team.ru) wrote: > On Mon, May 18, 2020 at 10:50:39AM +0800, Jason Wang wrote: > > > > On 2020/5/16 上午12:54, Dima Stepanov wrote: > > >On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: > > >>On 2020/5/13 下午5:47, Dima Stepanov wrote: > > > case

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-18 Thread Dima Stepanov
On Mon, May 18, 2020 at 10:50:39AM +0800, Jason Wang wrote: > > On 2020/5/16 上午12:54, Dima Stepanov wrote: > >On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: > >>On 2020/5/13 下午5:47, Dima Stepanov wrote: > > case CHR_EVENT_CLOSED: > > /* a close event may happen

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-18 Thread Dima Stepanov
On Mon, May 18, 2020 at 10:52:08AM +0800, Jason Wang wrote: > > On 2020/5/16 上午11:20, Li Feng wrote: > >Hi, Dima. > >This abort is what I have mentioned in my previous email. > >I have triggered this crash without any fix a week ago. > >And I have written a test patch to let

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-18 Thread Dima Stepanov
On Sat, May 16, 2020 at 11:20:03AM +0800, Li Feng wrote: > Hi, Dima. > This abort is what I have mentioned in my previous email. Yes, i understood it and this abort() message was fixed by the previous patch. But since we try new postphone approach this patch isn't working and we need to get the

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-17 Thread Jason Wang
On 2020/5/16 上午11:20, Li Feng wrote: Hi, Dima. This abort is what I have mentioned in my previous email. I have triggered this crash without any fix a week ago. And I have written a test patch to let vhost_log_global_start return int and propagate the error to up layer. However, my change is a

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-17 Thread Jason Wang
On 2020/5/16 上午12:54, Dima Stepanov wrote: On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: On 2020/5/13 下午5:47, Dima Stepanov wrote: case CHR_EVENT_CLOSED: /* a close event may happen during a read/write, but vhost * code assumes the vhost_dev remains

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-15 Thread Li Feng
Hi, Dima. This abort is what I have mentioned in my previous email. I have triggered this crash without any fix a week ago. And I have written a test patch to let vhost_log_global_start return int and propagate the error to up layer. However, my change is a little large, because the origin

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-15 Thread Dima Stepanov
On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: > > On 2020/5/13 下午5:47, Dima Stepanov wrote: > >>> case CHR_EVENT_CLOSED: > >>> /* a close event may happen during a read/write, but vhost > >>> * code assumes the vhost_dev remains setup, so delay the > >>>

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-14 Thread Jason Wang
On 2020/5/13 下午5:47, Dima Stepanov wrote: case CHR_EVENT_CLOSED: /* a close event may happen during a read/write, but vhost * code assumes the vhost_dev remains setup, so delay the * stop & clear to idle. * FIXME: better handle failure in vhost code,

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-13 Thread Dima Stepanov
On Wed, May 13, 2020 at 01:56:18PM +0800, Jason Wang wrote: > > On 2020/5/13 下午12:15, Michael S. Tsirkin wrote: > >On Tue, May 12, 2020 at 12:35:30PM +0300, Dima Stepanov wrote: > >>On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: > >>>On 2020/5/11 下午5:25, Dima Stepanov wrote: > On

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-13 Thread Dima Stepanov
On Wed, May 13, 2020 at 11:20:50AM +0800, Jason Wang wrote: > > On 2020/5/12 下午5:35, Dima Stepanov wrote: > >On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: > >>On 2020/5/11 下午5:25, Dima Stepanov wrote: > >>>On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: > On

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-12 Thread Jason Wang
On 2020/5/13 下午12:15, Michael S. Tsirkin wrote: On Tue, May 12, 2020 at 12:35:30PM +0300, Dima Stepanov wrote: On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: On 2020/5/11 下午5:25, Dima Stepanov wrote: On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: On 2020/4/30

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-12 Thread Michael S. Tsirkin
On Tue, May 12, 2020 at 12:35:30PM +0300, Dima Stepanov wrote: > On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: > > > > On 2020/5/11 下午5:25, Dima Stepanov wrote: > > >On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: > > >>On 2020/4/30 下午9:36, Dima Stepanov wrote: > > >>>If

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-12 Thread Jason Wang
On 2020/5/12 下午5:35, Dima Stepanov wrote: On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: On 2020/5/11 下午5:25, Dima Stepanov wrote: On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: On 2020/4/30 下午9:36, Dima Stepanov wrote: If vhost-user daemon is used as a backend

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-12 Thread Dima Stepanov
On Tue, May 12, 2020 at 11:32:50AM +0800, Jason Wang wrote: > > On 2020/5/11 下午5:25, Dima Stepanov wrote: > >On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: > >>On 2020/4/30 下午9:36, Dima Stepanov wrote: > >>>If vhost-user daemon is used as a backend for the vhost device, then we >

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-12 Thread Dima Stepanov
and we can continue migration. As a result no error is returned and assert() isn't hit. Also there is a question from Raphael to Michael about it you can find it in this thread, by i will add it also: > Subject: Re: [PATCH v2 5/5] vhost: add device started check in > migration set log

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-11 Thread Li Feng
Hi, Dima. If vhost_migration_log return < 0, then vhost_log_global_start will trigger a crash. Does your patch have process this abort? If a disconnect happens in the migration stage, the correct operation is to stop the migration, right? 841 static void vhost_log_global_start(MemoryListener

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-11 Thread Jason Wang
On 2020/5/11 下午5:25, Dima Stepanov wrote: On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: On 2020/4/30 下午9:36, Dima Stepanov wrote: If vhost-user daemon is used as a backend for the vhost device, then we should consider a possibility of disconnect at any moment. If such

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-11 Thread Dima Stepanov
On Sun, May 10, 2020 at 08:03:39PM -0400, Raphael Norwitz wrote: > On Thu, May 7, 2020 at 11:35 AM Dima Stepanov wrote: > > > > What do you think? > > > > Apologies - I tripped over the if (dev->started && r < 0) check. > Never-mind my point with race conditions and failing migrations. > >

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-11 Thread Dima Stepanov
On Mon, May 11, 2020 at 11:15:53AM +0800, Jason Wang wrote: > > On 2020/4/30 下午9:36, Dima Stepanov wrote: > >If vhost-user daemon is used as a backend for the vhost device, then we > >should consider a possibility of disconnect at any moment. If such > >disconnect happened in the

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-10 Thread Jason Wang
On 2020/4/30 下午9:36, Dima Stepanov wrote: If vhost-user daemon is used as a backend for the vhost device, then we should consider a possibility of disconnect at any moment. If such disconnect happened in the vhost_migration_log() routine the vhost device structure will be clean up. At the

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-10 Thread Raphael Norwitz
On Thu, May 7, 2020 at 11:35 AM Dima Stepanov wrote: > > What do you think? > Apologies - I tripped over the if (dev->started && r < 0) check. Never-mind my point with race conditions and failing migrations. Rather than modifying vhost_dev_set_log(), it may be clearer to put a check after

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-07 Thread Dima Stepanov
On Wed, May 06, 2020 at 06:08:34PM -0400, Raphael Norwitz wrote: > As you correctly point out, this code needs to be looked at more > carefully so that > if the device does disconnect in the background we can handle the migration > path > gracefully. In particular, we need to decide whether a

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-07 Thread Michael S. Tsirkin
On Wed, May 06, 2020 at 06:08:34PM -0400, Raphael Norwitz wrote: > As you correctly point out, this code needs to be looked at more > carefully so that > if the device does disconnect in the background we can handle the migration > path > gracefully. In particular, we need to decide whether a

Re: [PATCH v2 5/5] vhost: add device started check in migration set log

2020-05-06 Thread Raphael Norwitz
As you correctly point out, this code needs to be looked at more carefully so that if the device does disconnect in the background we can handle the migration path gracefully. In particular, we need to decide whether a migration should be allowed to continue if a device disconnects durning the