Created attachment 497317 [details] opevz_conf.c patch Description of problem: There are some bugs when parsing the openvz configuration files. Version-Release number of selected component (if applicable): 0.9.1 How reproducible: Always Steps to Reproduce: 1. Execute virsh connecting to openvz Actual results: Crashes with errors like: error: internal error Could not read config for container 100 error: failed to connect to the hypervisor Expected results: Stablish a connection Additional info: There are several bugs I've found: 1. The error reported above should be more verbose 2. There is a loop that crashes always in openvz_conf.c 3. When parsing the NETIF paramter, the tokens are not properly parsed. 4. There sould be some tolerance when some OpenVZ parameters do not exist on config files. Right now, the lack of some parameters will lead to an error. I submit a patch that solves the 1, 2 and 3 troubles. Anyway setting all needed paramaters on config files and applying this patch leads to a generic error in libvirt. I'm not a C coder but I thought that this could be useful for somebody.
I noticed this report after I created a similar patch, and I have more issues to report: 5. There's a possible memory leak in openvzReadVPSConfigParam if buffer given in argument was allocated prior to function call _AND_ given parameter was not found (but currently, it is only a possibility). Reading the comment, it is supposed to VIR_FREE(*value) it, instead of re-initializing with "value[0] = 0;" assignment (which will leak region that was in *value). 6. As reported, it fails even when optional OpenVZ configuration (like CPUS=) is missing. This is because commit f0443765 (which made a switch to getline(3) from openvz_readline) broke semantics on EOF handling. Both openvzReadConfigParam and openvzReadVPSConfigParam should return -1 on error, 0 if parameter was missing, and 1 if found. Because getline(3) returns -1 on EOF, current code returns -1 when parameter was not found. Patch provided by Diego prevents error if all parameters are there, but still fails is optional one is missing. I have attached updated patch which fixes #[1456], but lacks fix on #[23] as I haven't verified that one yet.
Created attachment 499581 [details] Patch to fix openvzReadConfigParam issue which makde openvz driver to fail always. Patch for the report I made on 2011-05-18 07:45:02 EDT.
Thanks for the report and the patch. However, libvirt devs generally don't review patches in bugzilla. Can you please send your patch to libvirt-list, and ensure that it applied against latest upstream, and that make check and make syntax-check both pass? When the patch is applied, we can close this bug. Thanks!
Pushed upstream as: commit 3aab7f2d6b068f0547d10f35588ec7f1ab6882a8 Author: Taisuke Yamada <tai> AuthorDate: 2011-05-26 19:28:54 +0200 Commit: Matthias Bolte <matthias.bolte> CommitDate: 2011-05-26 19:49:18 +0200 openvz: Fix regression in config file parsing As reported by Diego Blanco in https://bugzilla.redhat.com/show_bug.cgi?id=702602 commit f0443765 which replaced openvz_readline to getline(3) broke OpenVZ driver as it changed semantics of EOF-handling when parsing OpenVZ configuration. There're several other issues reported with current OpenVZ driver: #1: unclear error message when parsing "CPUS=" line #2: openvz driver goes into crashing loop #3: "NETIF=" line in configuration is not parsed correctly #4: aborts even when optional parameter is missing #5: there's a potential memory leak This updated patch to fix #[145]. This patch does not fix #[23] as I haven't verified these yet, but this at least got me to run OpenVZ on libvirt once again. git describe: v0.9.1-269-g3aab7f2 contains: v0.9.2~120