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 ebl...@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
