2008/12/16 Daniel P. Berrange <[email protected]> > On Tue, Dec 16, 2008 at 03:05:30PM +0300, Anton Protopopov wrote: > > 2008/12/13 Ivan Vovk <[email protected]> > > > > > 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 [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
