It will be used in multiple threads in follow-up patches.  Let it start
to have refcounts.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/io/task.h |  3 +++
 io/task.c         | 23 ++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/io/task.h b/include/io/task.h
index 9dbe3758d7..c6acd6489c 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -322,4 +322,7 @@ gpointer qio_task_get_result_pointer(QIOTask *task);
  */
 Object *qio_task_get_source(QIOTask *task);
 
+void qio_task_ref(QIOTask *task);
+void qio_task_unref(QIOTask *task);
+
 #endif /* QIO_TASK_H */
diff --git a/io/task.c b/io/task.c
index 204c0be286..00d3a5096a 100644
--- a/io/task.c
+++ b/io/task.c
@@ -32,6 +32,7 @@ struct QIOTask {
     Error *err;
     gpointer result;
     GDestroyNotify destroyResult;
+    uint32_t refcount;
 
     /* Threaded QIO task specific fields */
     GSource *idle_source;  /* The idle task to run complete routine */
@@ -57,6 +58,8 @@ QIOTask *qio_task_new(Object *source,
 
     trace_qio_task_new(task, source, func, opaque);
 
+    qio_task_ref(task);
+
     return task;
 }
 
@@ -165,7 +168,7 @@ void qio_task_complete(QIOTask *task)
 {
     task->func(task, task->opaque);
     trace_qio_task_complete(task);
-    qio_task_free(task);
+    qio_task_unref(task);
 }
 
 
@@ -208,3 +211,21 @@ Object *qio_task_get_source(QIOTask *task)
 {
     return task->source;
 }
+
+void qio_task_ref(QIOTask *task)
+{
+    if (!task) {
+        return;
+    }
+    atomic_inc(&task->refcount);
+}
+
+void qio_task_unref(QIOTask *task)
+{
+    if (!task) {
+        return;
+    }
+    if (atomic_fetch_dec(&task->refcount) == 1) {
+        qio_task_free(task);
+    }
+}
-- 
2.14.3

On Wed, Feb 28, 2018 at 01:06:29PM +0800, Peter Xu wrote:
> It will be used in multiple threads in follow-up patches.  Let it start
> to have refcounts.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/io/task.h |  3 +++
>  io/task.c         | 23 ++++++++++++++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/include/io/task.h b/include/io/task.h
> index 9dbe3758d7..c6acd6489c 100644
> --- a/include/io/task.h
> +++ b/include/io/task.h
> @@ -322,4 +322,7 @@ gpointer qio_task_get_result_pointer(QIOTask *task);
>   */
>  Object *qio_task_get_source(QIOTask *task);
>  
> +void qio_task_ref(QIOTask *task);
> +void qio_task_unref(QIOTask *task);

It should just be turned back into a QObject as it was originally,
so we get refcounting for free.

> diff --git a/io/task.c b/io/task.c
> index 204c0be286..00d3a5096a 100644
> --- a/io/task.c
> +++ b/io/task.c
> @@ -32,6 +32,7 @@ struct QIOTask {
>      Error *err;
>      gpointer result;
>      GDestroyNotify destroyResult;
> +    uint32_t refcount;
>  
>      /* Threaded QIO task specific fields */
>      GSource *idle_source;  /* The idle task to run complete routine */
> @@ -57,6 +58,8 @@ QIOTask *qio_task_new(Object *source,
>  
>      trace_qio_task_new(task, source, func, opaque);
>  
> +    qio_task_ref(task);
> +
>      return task;
>  }
>  
> @@ -165,7 +168,7 @@ void qio_task_complete(QIOTask *task)
>  {
>      task->func(task, task->opaque);
>      trace_qio_task_complete(task);
> -    qio_task_free(task);
> +    qio_task_unref(task);
>  }
>  
>  
> @@ -208,3 +211,21 @@ Object *qio_task_get_source(QIOTask *task)
>  {
>      return task->source;
>  }
> +
> +void qio_task_ref(QIOTask *task)
> +{
> +    if (!task) {
> +        return;
> +    }
> +    atomic_inc(&task->refcount);
> +}
> +
> +void qio_task_unref(QIOTask *task)
> +{
> +    if (!task) {
> +        return;
> +    }
> +    if (atomic_fetch_dec(&task->refcount) == 1) {
> +        qio_task_free(task);
> +    }
> +}
> -- 
> 2.14.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, Feb 28, 2018 at 09:16:59AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 28, 2018 at 01:06:29PM +0800, Peter Xu wrote:
> > It will be used in multiple threads in follow-up patches.  Let it start
> > to have refcounts.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  include/io/task.h |  3 +++
> >  io/task.c         | 23 ++++++++++++++++++++++-
> >  2 files changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/io/task.h b/include/io/task.h
> > index 9dbe3758d7..c6acd6489c 100644
> > --- a/include/io/task.h
> > +++ b/include/io/task.h
> > @@ -322,4 +322,7 @@ gpointer qio_task_get_result_pointer(QIOTask *task);
> >   */
> >  Object *qio_task_get_source(QIOTask *task);
> >  
> > +void qio_task_ref(QIOTask *task);
> > +void qio_task_unref(QIOTask *task);
> 
> It should just be turned back into a QObject as it was originally,
> so we get refcounting for free.

Could you point me the commit that has the original code?  I would be
glad to revert to that, or yeah I can switch to QObject too.  Thanks,

-- 
Peter Xu

On Wed, Feb 28, 2018 at 08:54:00PM +0800, Peter Xu wrote:
> On Wed, Feb 28, 2018 at 09:16:59AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Feb 28, 2018 at 01:06:29PM +0800, Peter Xu wrote:
> > > It will be used in multiple threads in follow-up patches.  Let it start
> > > to have refcounts.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  include/io/task.h |  3 +++
> > >  io/task.c         | 23 ++++++++++++++++++++++-
> > >  2 files changed, 25 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/io/task.h b/include/io/task.h
> > > index 9dbe3758d7..c6acd6489c 100644
> > > --- a/include/io/task.h
> > > +++ b/include/io/task.h
> > > @@ -322,4 +322,7 @@ gpointer qio_task_get_result_pointer(QIOTask *task);
> > >   */
> > >  Object *qio_task_get_source(QIOTask *task);
> > >  
> > > +void qio_task_ref(QIOTask *task);
> > > +void qio_task_unref(QIOTask *task);
> > 
> > It should just be turned back into a QObject as it was originally,
> > so we get refcounting for free.
> 
> Could you point me the commit that has the original code?  I would be
> glad to revert to that, or yeah I can switch to QObject too.  Thanks,

It was never actually committed - it was just that way during initial
review but was suggested to be simplified as ref counting wasn't needed.

That all said, having now seen the later parts of this patch series, I
don't think we would need todo this at all, because we should not be
exposing the GTask to callers and thus the refcounting question goes away

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, Feb 28, 2018 at 01:07:44PM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 28, 2018 at 08:54:00PM +0800, Peter Xu wrote:
> > On Wed, Feb 28, 2018 at 09:16:59AM +0000, Daniel P. Berrangé wrote:
> > > On Wed, Feb 28, 2018 at 01:06:29PM +0800, Peter Xu wrote:
> > > > It will be used in multiple threads in follow-up patches.  Let it start
> > > > to have refcounts.
> > > > 
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > ---
> > > >  include/io/task.h |  3 +++
> > > >  io/task.c         | 23 ++++++++++++++++++++++-
> > > >  2 files changed, 25 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/include/io/task.h b/include/io/task.h
> > > > index 9dbe3758d7..c6acd6489c 100644
> > > > --- a/include/io/task.h
> > > > +++ b/include/io/task.h
> > > > @@ -322,4 +322,7 @@ gpointer qio_task_get_result_pointer(QIOTask *task);
> > > >   */
> > > >  Object *qio_task_get_source(QIOTask *task);
> > > >  
> > > > +void qio_task_ref(QIOTask *task);
> > > > +void qio_task_unref(QIOTask *task);
> > > 
> > > It should just be turned back into a QObject as it was originally,
> > > so we get refcounting for free.
> > 
> > Could you point me the commit that has the original code?  I would be
> > glad to revert to that, or yeah I can switch to QObject too.  Thanks,
> 
> It was never actually committed - it was just that way during initial
> review but was suggested to be simplified as ref counting wasn't needed.
> 
> That all said, having now seen the later parts of this patch series, I
> don't think we would need todo this at all, because we should not be
> exposing the GTask to callers and thus the refcounting question goes away

Indeed.  I suppose that means you like my proposal in the other
thread.  But I'll wait for your explicit acknowledgement in that too.

Thanks,

-- 
Peter Xu