Re: [libvirt] [PATCH] Add RESUME event listener to qemu monitor.

On Tue, Jan 08, 2013 at 01:28:10PM -0500, Andres Lagar-Cavilla wrote: > On Jan 7, 2013, at 5:44 PM, "Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: > >> On Mon, Jan 07, 2013 at 04:25:01PM -0500, Andres Lagar-Cavilla wrote: >>> Perform all the appropriate plumbing. >>> >>> When qemu/KVM VMs are paused manually through a monitor not-owned by libvirt, >>> libvirt will think of them as "paused" event after they are resumed and >>> effectively running. With this patch the discrepancy goes away. >>> >>> This is meant to address bug 892791. >>> >>> Signed-off-by: Andres Lagar-Cavilla <andres(a)lagarcavilla.org&gt; >> >> Although we don't support people issuing monitor commands behind >> libvirt's back, I see no harm in handling the resume event here, >> as long as we don't end up emitting multiple resume events for >> a single operation (eg if libvirt triggers the resume it already >> emits an event independently) > > Hi Daniel, > > In most cases, qemuProcessStartCPUs will be the libvirt API issuing qemu 'cont' commands. Due to the locking rules, we can be confident that it will complete processing while holding the VM lock, and in the process update the state to VIR_DOMAIN_RUNNING. The callback handler for RESUME introduced here will be serialized against the VM lock after StartCPUs, and result in effectively a no-op (because it only does work if the vm state is PAUSED). > > Create/Load Snapshot call StopCPUs before doing any work, so qemu itself will not attempt to resume the VM, and thus no event will be generated. In strict terms this is unnecessary (although relatively little) work, but here is the rationale in qemu_driver.c: > /* savevm monitor command pauses the domain emitting an event which > * confuses libvirt since it's not notified when qemu resumes the > * domain. Thus we stop and start CPUs ourselves. > */ > > Relevant to the issue at hand. If this patch is merged then the library > can get rid of this StopCPUs invocation, but additional book-keeping is > necessary. IMHO this is neither pressing nor necessary. Actally we can't get rid of it, because events only work in QMP mode. If talking to an old QEMU via HMP we need to deal with it as described in the comment. ACK to your patch anyway.

Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|