2008/12/16 Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>></span><br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="Wj3C7c">On Tue, Dec 16, 2008 at 03:05:30PM +0300, Anton Protopopov wrote:<br>
> 2008/12/13 Ivan Vovk <<a href="mailto:ivovk@intermedia.net">ivovk@intermedia.net</a>><br>
><br>
> > Hello,<br>
> ><br>
> ><br>
> ><br>
> > after updating to the last official release 0.5.1 my application stopped<br>
> > working. No errors to the log file though i raised exceptions where it was<br>
> > possible.<br>
> ><br>
> > i checked with virsh xml description used for creating OpenVZ container and<br>
> > got the following:<br>
> ><br>
> ><br>
> ><br>
> > virsh # create ovz.xml<br>
> > Segmentation fault<br>
> ><br>
> > Attached patch, fixes that.<br>
</div></div></blockquote><div><br> - NACK. This patch is a guarenteed deadlock.</div><div>+ this patch helps us to see a deadlock, which previously was invisible because of segfault ;)<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
You can't have one public<br>
driver API method, calling into another directly. openvzDomainSetVcpus()<br>
will attempt to lock the driver mutex, and the VM mutex, and then deadlock<br>
because openvzDomainCreateXML/openvzDomainDefineXML already hold the<br>
VM mutex.<br>
<br>
The way to address this is to move the backend logic out of the<br>
openvzDomainSetVcpus method, into a helper that is passed a pre-locked<br>
virDomainObjPtr instance. This helper can be called by public driver<br>
APIs openvzDomainSetVcpus and openvzDomainCreateXML and<br>
openvzDomainDefineXML.<br>
<br>
eg, the helper would look like<br>
<br>
static int openvzDomainSetVcpusInternal(virConnectPtr conn,<br>
virDomainObjPtr vm)<br>
char str_vcpus[32];<br>
const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINAL,<br>
"--cpus", str_vcpus, "--save", NULL };<br>
pcpus = openvzGetNodeCPUs();<br>
if (pcpus > 0 && pcpus < nvcpus)<br>
nvcpus = pcpus;<br>
<br>
snprintf(str_vcpus, 31, "%d", nvcpus);<br>
str_vcpus[31] = '\0';<br>
<br>
openvzSetProgramSentinal(prog, vm->def->name);<br>
if (virRun(dom->conn, prog, NULL) < 0) {<br>
openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR,<br>
_("Could not exec %s"), VZCTL);<br>
retur n-1;<br>
}<br>
<br>
vm->def->vcpus = nvcpus;<br>
return 0;<br>
}<br>
<br>
The public API would look like<br>
<br>
<br>
static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) {<br>
struct openvz_driver *driver = dom->conn->privateData;<br>
virDomainObjPtr vm;<br>
unsigned int pcpus;<br>
int ret = -1;<br>
<br>
openvzDriverLock(driver);<br>
vm = virDomainFindByUUID(&driver->domains, dom->uuid);<br>
openvzDriverUnlock(driver);<br>
if (!vm) {<br>
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,<br>
"%s", _("no domain with matching uuid"));<br>
goto cleanup;<br>
}<br>
<br>
if (nvcpus <= 0) {<br>
openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR,<br>
"%s", _("VCPUs should be >= 1"));<br>
goto cleanup;<br>
}<br>
<br>
openvzDomainSetVcpusInternal(vm, nvcpus);<br>
<br>
ret = 0;<br>
<br>
cleanup:<br>
if (vm)<br>
virDomainObjUnlock(vm);<br>
return ret;<br>
}<br>
<br>
<br>
Similarly the Create/DefineXML methods calling openvzDomainSetVcpusInternal<br>
</blockquote><div>Thanks, this seems to work (see attached patch).<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
Regards,<br>
Daniel<br>
<font color="#888888">--<br>
|: Red Hat, Engineering, London -o- <a href="http://people.redhat.com/berrange/" target="_blank">http://people.redhat.com/berrange/</a> :|<br>
|: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a> -o- <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> -o- <a href="http://ovirt.org" target="_blank">http://ovirt.org</a> :|<br>
|: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a> -o- <a href="http://search.cpan.org/%7Edanberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|<br>
</font></blockquote></div><br>