| Message ID | e821aa9bccb49bf68c94e3d49b105c420dde9981.1571905346.git.jag.raman@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Initial support of multi-process qemu | expand |
On Thu, Oct 24, 2019 at 05:08:43AM -0400, Jagannathan Raman wrote: > qemu_thread_cancel() added to destroy a given running thread. > This will be needed in the following patches. > > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > --- > include/qemu/thread.h | 1 + > util/qemu-thread-posix.c | 10 ++++++++++ > 2 files changed, 11 insertions(+) Is this still needed? I thought previous discussion concluded that thread cancellation is hard to get right and it's not actually used by this series? Stefan
On 11/13/2019 10:30 AM, Stefan Hajnoczi wrote: > On Thu, Oct 24, 2019 at 05:08:43AM -0400, Jagannathan Raman wrote: >> qemu_thread_cancel() added to destroy a given running thread. >> This will be needed in the following patches. >> >> Signed-off-by: John G Johnson <john.g.johnson@oracle.com> >> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> >> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> >> --- >> include/qemu/thread.h | 1 + >> util/qemu-thread-posix.c | 10 ++++++++++ >> 2 files changed, 11 insertions(+) > > Is this still needed? I thought previous discussion concluded that > thread cancellation is hard to get right and it's not actually used by > this series? Hi Stefan, This is used in PATCH 41/49. Thank you very much! -- Jag > > Stefan >
On Wed, Nov 13, 2019 at 10:38:06AM -0500, Jag Raman wrote: > > > On 11/13/2019 10:30 AM, Stefan Hajnoczi wrote: > > On Thu, Oct 24, 2019 at 05:08:43AM -0400, Jagannathan Raman wrote: > > > qemu_thread_cancel() added to destroy a given running thread. > > > This will be needed in the following patches. > > > > > > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > > > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > > > --- > > > include/qemu/thread.h | 1 + > > > util/qemu-thread-posix.c | 10 ++++++++++ > > > 2 files changed, 11 insertions(+) > > > > Is this still needed? I thought previous discussion concluded that > > thread cancellation is hard to get right and it's not actually used by > > this series? > > Hi Stefan, > > This is used in PATCH 41/49. I don't believe the cancellation usage in that patch is safe :-) Regards, Daniel
On 11/13/2019 10:51 AM, Daniel P. Berrangé wrote: > On Wed, Nov 13, 2019 at 10:38:06AM -0500, Jag Raman wrote: >> >> >> On 11/13/2019 10:30 AM, Stefan Hajnoczi wrote: >>> On Thu, Oct 24, 2019 at 05:08:43AM -0400, Jagannathan Raman wrote: >>>> qemu_thread_cancel() added to destroy a given running thread. >>>> This will be needed in the following patches. >>>> >>>> Signed-off-by: John G Johnson <john.g.johnson@oracle.com> >>>> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> >>>> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> >>>> --- >>>> include/qemu/thread.h | 1 + >>>> util/qemu-thread-posix.c | 10 ++++++++++ >>>> 2 files changed, 11 insertions(+) >>> >>> Is this still needed? I thought previous discussion concluded that >>> thread cancellation is hard to get right and it's not actually used by >>> this series? >> >> Hi Stefan, >> >> This is used in PATCH 41/49. > > I don't believe the cancellation usage in that patch is safe :-) Thanks for the feedback, we will address that. May I please ask why it is not safe? Any clarification will help us to find a better alternative. Thank you very much! -- Jag > > Regards, > Daniel >
On Wed, Nov 13, 2019 at 11:04:58AM -0500, Jag Raman wrote: > > > On 11/13/2019 10:51 AM, Daniel P. Berrangé wrote: > > On Wed, Nov 13, 2019 at 10:38:06AM -0500, Jag Raman wrote: > > > > > > > > > On 11/13/2019 10:30 AM, Stefan Hajnoczi wrote: > > > > On Thu, Oct 24, 2019 at 05:08:43AM -0400, Jagannathan Raman wrote: > > > > > qemu_thread_cancel() added to destroy a given running thread. > > > > > This will be needed in the following patches. > > > > > > > > > > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > > > > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > > > > > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > > > > > --- > > > > > include/qemu/thread.h | 1 + > > > > > util/qemu-thread-posix.c | 10 ++++++++++ > > > > > 2 files changed, 11 insertions(+) > > > > > > > > Is this still needed? I thought previous discussion concluded that > > > > thread cancellation is hard to get right and it's not actually used by > > > > this series? > > > > > > Hi Stefan, > > > > > > This is used in PATCH 41/49. > > > > I don't believe the cancellation usage in that patch is safe :-) > > Thanks for the feedback, we will address that. > > May I please ask why it is not safe? Any clarification will help us to > find a better alternative. I put some comments inline in the patch 41 explaining my thoughts. Regards, Daniel
diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 047db03..fe7fa5a 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -175,6 +175,7 @@ void qemu_thread_create(QemuThread *thread, const char *name, void *(*start_routine)(void *), void *arg, int mode); void *qemu_thread_join(QemuThread *thread); +void qemu_thread_cancel(QemuThread *thread); void qemu_thread_get_self(QemuThread *thread); bool qemu_thread_is_self(QemuThread *thread); void qemu_thread_exit(void *retval); diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 838980a..2fd85ed 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -590,3 +590,13 @@ void *qemu_thread_join(QemuThread *thread) } return ret; } + +void qemu_thread_cancel(QemuThread *thread) +{ + int err; + + err = pthread_cancel(thread->thread); + if (err) { + error_exit(err, __func__); + } +}