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

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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