[libvirt] [PATCH v2 4/5] In storageVolumeCreateXML, spawn a new thread for volbuilding, in storageVolumeDelete, generate the signal

[Guannan Ren]

On 07/18/2011 06:57 PM, Daniel P. Berrange wrote:
> On Sun, Jul 17, 2011 at 06:45:00PM +0800, Guannan Ren wrote:
>> ---
>>   src/storage/storage_backend.c |    9 ++++
>>   src/storage/storage_driver.c  |   83 ++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 82 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>> index f632edd..bc10933 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>> @@ -1632,3 +1632,12 @@ virStorageBackendRunProgNul(virConnectPtr conn,
>>       return -1;
>>   }
>>   #endif /* WIN32 */
>> +
>> +void virStorageBackendVoluCleanup(void *arg)
>> +{
>> +
>> +    volBuildThreadPtr data = arg;
>> +
>> +    data->buildret = 0;
>> +    data->threadEnd = 1;
>> +}
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index 997b876..d8ac648 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -48,6 +48,7 @@
>>   #include "files.h"
>>   #include "fdstream.h"
>>   #include "configmake.h"
>> +#include "threads.h"
>>
>>   #define VIR_FROM_THIS VIR_FROM_STORAGE
>>
>> @@ -1276,6 +1277,29 @@ cleanup:
>>
>>   static int storageVolumeDelete(virStorageVolPtr obj, unsigned int flags);
>>
>> +static void virStorageBuildVol(void *arg)
>> +{
>> +    int ret = -1;
>> +    volBuildThreadPtr data = arg;
>> +    virStoragePoolObjPtr pool = data->pool;
>> +    virStorageVolDefPtr vol = data->vol;
>> +
>> +    pthread_cleanup_push(virStorageBackendVoluCleanup, data);
>> +
>> +    ret = data->buildvol(data->obj->conn, pool, vol);
>> +
>> +    pthread_cleanup_pop(0);
> NACK, all use of pthread specific APIs must be in src/util/threads-pthread.c
>
> Independantly of this, IMHO pthread cancellation handlers are a recipe
> for trouble because it is incredibly hard to make sure you correctly
> cleanup all resources in the thread, even with use of cleanup handlers.
>
> IMHO, threads should be made to monitor some external "quit" boolean
> variable (eg see  threadpool.c thread termination).
>
> Daniel
     yep  but the push and pop function couldn't be in different 
function, according to man page as follows.
     It is dangerous to use thread handler, I agree with you.
     Thanks.

     "These functions may be implemented as macros. The application 
shall ensure that they appear as statements, and  in  pairs  within  the
        same  lexical  scope (that is, the pthread_cleanup_push() macro 
may be thought to expand to a token list whose first token is ’{’ with
        pthread_cleanup_pop() expanding to a token list whose last token 
is the corresponding ’}’ ).
     "