Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support.

[Paolo Bonzini]

On 27/11/2015 12:26, Peter Xu wrote:
> On Fri, Nov 27, 2015 at 11:31:00AM +0100, Paolo Bonzini wrote:
>>
> 
> [snip]
> 
>>> +
>>> +static GlobalDumpState *dump_state_get_global(void)
>>> +{
>>> +    static bool init = false;
>>> +    static GlobalDumpState global_dump_state;
>>> +    if (unlikely(!init)) {
>>> +        qemu_mutex_init(&global_dump_state.gds_mutex);
>>
>> The mutex is not necessary.  The structure is always created in the main
>> thread and released by the dump thread (of which there is just one).
> 
> [1]
> 
>>
>> You can then just make a global DumpState (not a pointer!), and separate
>> the fields between:
>>
>> - those that are protected by a mutex (most likely the DumpResult and
>> written_size, possibly others)
> 
> Hi, Paolo,
> 
> So mutex is still necessary, right? (refer to [1]) Since
> "dump-query" will read several fields of it, while the dump thread
> might be modifying them as well?

Right, initially I meant a mutex around the allocation.  But then I read
the other patches where you add more fields, and came back to add more
content.

Strictly speaking it's likely that you can do everything without a
mutex, but it's easier to use one.

> Ok, I think I can remove the global state and make a static
> DumpState. When I was drafting the patch, I just tried to keep the
> old logic (malloc/free) and avoid introducing bugs. Maybe I was
> wrong. I should better not introduce new struct if not necessary. 

It's okay.  The only confusing bit was that you had to add more state to
GlobalDumpState, and in the end it was split between DumpState and
GlobalDumpState.  As far as this patch is concerned, using malloc/free
would have been okay, but then the subsequent changes suggest a
different approach.

Thanks!

Paolo

> Will try to follow this example in v3.
> 
> Thanks.
> Peter
> 
>>
>> Paolo
>>
> 
>