Re: [PATCH 5/6] remove qemuDomainObjEnterMonitorWithDriver and qemuDomainObjExitMonitorWithDriver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/06/2011 01:20 AM, Hu Tao wrote:
> Bodies of qemuDomainObjEnterMonitorWithDriver/qemuDomainObjExitMonitorWithDriver
> are the same as qemuDomainObjEnterMonitor/qemuDomainObjExitMonitor, so
> remove them.
> ---
>  src/qemu/qemu_domain.c    |   32 ----------------
>  src/qemu/qemu_domain.h    |    4 --
>  src/qemu/qemu_driver.c    |   20 +++++-----
>  src/qemu/qemu_hotplug.c   |   90 ++++++++++++++++++++++----------------------
>  src/qemu/qemu_migration.c |   46 +++++++++++-----------
>  src/qemu/qemu_process.c   |   36 +++++++++---------
>  6 files changed, 96 insertions(+), 132 deletions(-)

Aha - I should have previewed the patch series first, before raising the
same point in my review of patch 3.  I'm still not convinced that we can
get away with this, if any of the callers had previously been calling
with driver locked, because driver lock must be dropped before waiting
for the condition variable.  But if we can, then it's a nice concept.

I've run out of review time for today, though, so I haven't reviewed
this one closely (nor 4 or 6), and probably won't look at them until we
resolve the bigger issues I raised earlier on patches 1 and 3 when you
post a v2 of the series.

Even documenting in the commit message of patch 3 that you are
intentionally changing semantics of no longer calling with driver
locked, but saving the cleanup for a later patch, would be helpful.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]