On 2013年02月12日 01:44, Daniel P. Berrange wrote:
On Mon, Feb 11, 2013 at 07:37:14PM +0800, Osier Yang wrote:
On 2013年02月11日 18:59, Daniel P. Berrange wrote:
On Fri, Feb 08, 2013 at 09:08:01PM +0800, Osier Yang wrote:
qemuProcessStart invokes qemuProcessStop when fails, to avoid
removing hash entry which belongs to other domain(s), this introduces
a new virBitmapPtr argument for qemuProcessStop. And qemuProcessStart
sets the bit for the disk only if it's successfully added into the
hash table. Thus if the argument is provided for qemuProcessStop, it
can't remove the hash entry belongs to other domain(s).
I find this approach rather dubious - IMHO it is a sign that you're
not recording enough information in the shared disk hash. eg perhaps
the hash ought to be recording the UUID of each VM that is holding
a reference. That way you're protected from qemuProcessStop() trying
todo something wrong.
I'm doubting about if it's really deserved to maintain a bunch of
arrays in the hash entry. As we only need the recording for
qemuProcessStart, it's much simpler to only use a bitmap to record
the added disks for a VM in qemuProcessStart from my p.o.v.
IMHO it is a more robust design to record which VM is owning the
disk, since it prevents us introducing errors elsewhere in the
code which can lead to the same kind of problem later.
Okay, I have no more reason to not go this way.
Incidentally, how does the shared disks hash get populated when
libvirtd starts up& reconnects to existing running VMs ? AFAICT,
nothing is being done in that codepath.
Indeed, will add.
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list