[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" su
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support. |
Date: |
Fri, 27 Nov 2015 19:26:45 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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?
>
> - those that are only written before the thread is started, and thus do
> not need to be protected by a mutex
>
> - those that are only accessed by the thread, and thus do not need to be
> protected by a mutex either.
>
> See for example this extract from migration/block.c:
>
> typedef struct BlkMigState {
> /* Written during setup phase. Can be read without a lock. */
> int blk_enable;
> int shared_base;
> QSIMPLEQ_HEAD(bmds_list, BlkMigDevState) bmds_list;
> int64_t total_sector_sum;
> bool zero_blocks;
>
> /* Protected by lock. */
> QSIMPLEQ_HEAD(blk_list, BlkMigBlock) blk_list;
> int submitted;
> int read_done;
>
> /* Only used by migration thread. Does not need a lock. */
> int transferred;
> int prev_progress;
> int bulk_completed;
>
> /* The lock. */
> QemuMutex lock;
> } BlkMigState;
>
> static BlkMigState block_mig_state;
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.
Will try to follow this example in v3.
Thanks.
Peter
>
> Paolo
>
- Re: [Qemu-devel] [PATCH v2 2/8] dump-guest-memory: add "detach" flag for QMP/HMP interfaces., (continued)
Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support., Paolo Bonzini, 2015/11/27
[Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED, Peter Xu, 2015/11/26
[Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status, Peter Xu, 2015/11/26