On 02/10/2011 06:02 AM, Jiri Denemark wrote:
> Certainly a tougher prospect. On the surface, it looks correct;
> however, I'm worried that there might be some potential for a data race
> bug - that is, if we ever have any command that can build a command line
> string while holding a read-only connection, is it right to be
> temporarily modifying the domain def? (That is, it seems like
> domxml-to-native should work on a read-only connection without trying to
> modify the guest definition.)
Eh, the code is not changing domain def anywhere. The only thing which is
changed is qemuCmdFlags, which is generated in every caller of
qemuBuildCommandLine() so cloning the bitmap seemed like an unneeded overkill
to me.
Then ignore my concerns :) For a while, there, I was thinking that
qemuCmdFlags was part of the domain definition (and in the future, it
needs to be - we have a FIXME about that, since there is the issue that
if qemu is upgraded then libvirt restarted, that existing qemu processes
still have the older command line syntax, and reprobing the new qemu
binary could give the wrong results). But since you are correct that
for now, all qemuCmdFlags are stack-allocated temporaries, and not part
of the domain definition, there is no race, and this code is fine.
Do you still think it's needed?
Nah, although you'll probably hit other rebasing issues when respinning
this series for post-0.8.8 release, so I won't be surprised if I see a v2.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org