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