On Mon, Nov 27, 2017 at 10:44:26AM +0000, Dr. David Alan Gilbert wrote: [...]
> > > > > > But then I added a break on pthread_mutex_lock, and I've got > > > this set caused by qemu_start_incoming_migration sending a > > > MIGRATION_STATUS_SETUP event: > > > > > > #1 0x0000555555ba6eba in qemu_mutex_lock > > > (mutex=mutex@entry=0x5555563f9ba0 <monitor_lock>) > > > at /home/dgilbert/git/qemu/util/qemu-thread-posix.c:65 > > > #2 0x00005555557f01c1 in monitor_qapi_event_queue > > > (event=QAPI_EVENT_MIGRATION, qdict=0x555557d93c00, errp=<optimized out>) > > > at /home/dgilbert/git/qemu/monitor.c:442 > > > > > > 440 trace_monitor_protocol_event_queue(event, qdict, > > > evconf->rate); > > > 441 > > > 442 qemu_mutex_lock(&monitor_lock); > > > 443 > > > > > > #3 0x0000555555b92722 in qapi_event_send_migration > > > (status=status@entry=MIGRATION_STATUS_SETUP, errp=0x555556859320 > > > <error_abort>) at qapi-event.c:661 > > > #4 0x0000555555a6159f in qemu_start_incoming_migration > > > (uri=0x555557693f30 "tcp:0:4444", errp=0x7fffffffc700) > > > at /home/dgilbert/git/qemu/migration/migration.c:253 > > > #5 0x0000555555a641c5 in qmp_migrate_incoming (uri=0x555557693f30 > > > "tcp:0:4444", errp=0x7fffffffc730) > > > at /home/dgilbert/git/qemu/migration/migration.c:1321 > > > > > > is there anything which protects us there? > > > > IIUC you mean what if we e.g. page fault during taking the > > monitor_lock? IMHO it just can't happen - monitor_lock is really used > > in limited places and during those critical sections there is no guest > > memory access at all (which only protects the monitor logic itself > > AFAICT). > > OK, so we should document somewhere which locks it's OK to take in an > OOB command, and then make sure that for each of those locks we add > a note saying that anyone taking them must be careful. > We should also add a note above teh qmp_migrate_incoming that it's an > OOB command and to take care. Makes sense. I think it will be hard to document what locks can be taken during OOB command handing since there can be too many locks there... But at least I can add a comment to qmp_migrate_incoming() along with current patch to note that this function should be OOB friendly from now on. > > However, since you've convinced me it's OK: > > > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> Thanks! -- Peter Xu
