- Devel - libvirt List Archives

[libvirt] [PATCHv2 0/2] Couple of NVRAM fixes
by Michal Privoznik 12 Sep '14

12 Sep '14

diff to v1: -patch 1/2 (2/2 in previous version) updated according to Laszlo's review Michal Privoznik (2): virDomainUndefineFlags: Allow NVRAM unlinking formatdomain: Update <loader/> example to match the rest docs/formatdomain.html.in | 2 +- include/libvirt/libvirt.h.in | 2 ++ src/qemu/qemu_driver.c | 20 +++++++++++++++++++- tools/virsh-domain.c | 20 ++++++++++++++++---- tools/virsh.pod | 6 +++++- 5 files changed, 43 insertions(+), 7 deletions(-) -- 1.8.5.5

2 5

[libvirt] [PATCH 0/9] More Coverity fixes
by John Ferlan 12 Sep '14

12 Sep '14

There are two repeats from the last series (1 & 2). For patch 1, I went with my suggestion - I'm open to others For patch 2, Coverity was complaining more about the way nparams would be overwritten - fix that by adding a new variable New patches 3 & 4 -> eblake helped out with these - especially the mgetgroups oddity 5 -> Fallout from fixing 4 6 -> virTimeFieldsThen() and the "offset = 0". I'd be OK with deleting the code, but it just feels like someone had it on a todo list to come back to some day 7 & 8 -> Fairly straightforward 9 -> This was an interesting case - it seems from what was being done that I have the right "answer". I did go all the way back to the initial submission of the code and it did the same thing, except it was using an unsigned long instead of int and well thus wouldn't ever hit the condition since we're grabbing the big endian int value This gets me down to 5 issues John Ferlan (9): remote_driver: Resolve Coverity RESOURCE_LEAK virsh: Resolve Coverity NEGATIVE_RETURNS daemon: Resolve Coverity RESOURCE_LEAK virutil: Resolve Coverity RESOURCE_LEAK virfile: Resolve Coverity RESOURCE_LEAK virtime: Resolve Coverity DEADCODE qemu: Resolve Coverity FORWARD_NULL libxl: Resolve Coverity CHECKED_RETURN virstoragefile: Resolve Coverity DEADCODE daemon/libvirtd.c | 8 ++++---- src/libxl/libxl_driver.c | 3 ++- src/qemu/qemu_capabilities.c | 2 +- src/remote/remote_driver.c | 20 +++++++++++++++++++- src/util/virfile.c | 7 +++++-- src/util/virstoragefile.c | 2 +- src/util/virtime.c | 2 ++ src/util/virutil.c | 1 + tools/virsh-domain.c | 7 ++++--- 9 files changed, 39 insertions(+), 13 deletions(-) -- 1.9.3

4 20

[libvirt] [PATCH v2 0/2] parallels: use parallels SDK instead of prlctl tool
by Dmitry Guryanov 12 Sep '14

12 Sep '14

This patchset begins reworking of parallels driver. We have published Opensource version of parallels SDK (under LGPL license), so libvirt can link with it. Dmitry Guryanov (2): parallels: build with parallels SDK parallels: login to parallels SDK configure.ac | 24 ++-- po/POTFILES.in | 1 + src/Makefile.am | 8 +- src/parallels/parallels_driver.c | 40 ++++++- src/parallels/parallels_sdk.c | 241 +++++++++++++++++++++++++++++++++++++++ src/parallels/parallels_sdk.h | 30 +++++ src/parallels/parallels_utils.h | 4 + 7 files changed, 334 insertions(+), 14 deletions(-) create mode 100644 src/parallels/parallels_sdk.c create mode 100644 src/parallels/parallels_sdk.h -- 1.9.3

2 7

[libvirt] [libvirt-python][PATCH] generator: Free strings after libvirt_charPtrWrap
by Michal Privoznik 12 Sep '14

12 Sep '14

Up till bb3301ba the wrapper was freeing the passed strings for us. However that changed after the commit. So now we don't free any strings which results in memory leaks as reported upstream [1]: ==14265== 2,407 bytes in 1 blocks are definitely lost in loss record 1,457 of 1,550 ==14265== at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==14265== by 0x5C46624: xdr_string (in /usr/lib64/libc-2.17.so) ==14265== by 0xCFD9FCD: xdr_remote_nonnull_string (remote_protocol.c:31) ==14265== by 0xCFDC2C8: xdr_remote_domain_get_xml_desc_ret (remote_protocol.c:1617) ==14265== by 0xCFF0811: virNetMessageDecodePayload (virnetmessage.c:407) ==14265== by 0xCFE68FB: virNetClientProgramCall (virnetclientprogram.c:379) ==14265== by 0xCFBE8B1: callFull.isra.2 (remote_driver.c:6578) ==14265== by 0xCFC7F04: remoteDomainGetXMLDesc (remote_driver.c:6600) ==14265== by 0xCF8167C: virDomainGetXMLDesc (libvirt.c:4380) ==14265== by 0xCC2C4DF: libvirt_virDomainGetXMLDesc (libvirt.c:1141) ==14265== by 0x4F12B93: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) ==14265== by 0x4F141AC: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) The python documentation clearly advise us to call free() [2]. From an example in their docs: PyObject *res; char *buf = (char *) malloc(BUFSIZ); /* for I/O */ if (buf == NULL) return PyErr_NoMemory(); ...Do some I/O operation involving buf... res = PyString_FromString(buf); free(buf); /* malloc'ed */ return res; Moreover, instead of using VIR_FREE() (which we are not exporting), I'll just go with bare free(). 1: https://www.redhat.com/archives/libvir-list/2014-September/msg00736.html 2: https://docs.python.org/2/c-api/memory.html Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- While the fix seems okay, for some reason I'm seeing another valgrind warning after the fix is applied: ==21247== Invalid read of size 4 ==21247== at 0x4EBFB93: PyObject_Free (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4EB613E: insertdict_by_entry (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4EB7BAF: dict_set_item_by_hash_or_entry (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4EBC4CB: _PyModule_Clear (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F2916A: PyImport_Cleanup (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F34B0D: Py_Finalize (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F460A5: Py_Main (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x54462BF: (below main) (in /lib64/libc-2.19.so) ==21247== Address 0x69be020 is 2,016 bytes inside a block of size 4,390 free'd ==21247== at 0x4C2B5AC: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==21247== by 0x7F1EC2D: libvirt_virDomainGetXMLDesc (in /home/zippy/work/libvirt/libvirt-python.git/build/lib.linux-x86_64-2.7/libvirtmod.so) ==21247== by 0x4F180BB: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F199FF: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F18AB4: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F199FF: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F19AF8: PyEval_EvalCode (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F32EBE: run_mod (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F340E1: PyRun_FileExFlags (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F352E6: PyRun_SimpleFileExFlags (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x4F4670E: Py_Main (in /usr/lib64/libpython2.7.so.1.0) ==21247== by 0x54462BF: (below main) (in /lib64/libc-2.19.so) This happens when using our examples/dominfo.py. generator.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/generator.py b/generator.py index a798274..8ee046a 100755 --- a/generator.py +++ b/generator.py @@ -708,12 +708,16 @@ def print_function_wrapper(module, name, output, export, include): else: c_call = "\n c_retval = %s(%s);\n" % (name, c_call) ret_convert = " py_retval = libvirt_%sWrap((%s) c_retval);\n" % (n,c) + if n == "charPtr": + ret_convert = ret_convert + " free(c_retval);\n" ret_convert = ret_convert + " return py_retval;\n" elif ret[0] in py_return_types: (f, t, n, c) = py_return_types[ret[0]] c_return = " %s c_retval;\n" % (ret[0]) c_call = "\n c_retval = %s(%s);\n" % (name, c_call) ret_convert = " py_retval = libvirt_%sWrap((%s) c_retval);\n" % (n,c) + if n == "charPtr": + ret_convert = ret_convert + " free(c_retval);\n" ret_convert = ret_convert + " return py_retval;\n" else: if ret[0] in skipped_types: -- 1.8.5.5

2 1

[libvirt] [PATCH] virsh: Move --completed from resume to domjobinfo
by Jiri Denemark 12 Sep '14

12 Sep '14

Because of similar contexts, git rebase I did just before pushing the series which added --completed option patched the wrong command. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- Pushed as trivial. tools/virsh-domain.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc1e554..30b3fa9 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5114,10 +5114,6 @@ static const vshCmdOptDef opts_resume[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, - {.name = "completed", - .type = VSH_OT_BOOL, - .help = N_("return statistics of a recently completed job") - }, {.name = NULL} }; @@ -5377,6 +5373,10 @@ static const vshCmdOptDef opts_domjobinfo[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, + {.name = "completed", + .type = VSH_OT_BOOL, + .help = N_("return statistics of a recently completed job") + }, {.name = NULL} }; -- 2.1.0

1 0

[libvirt] [PATCHv3 0/8] bulk stats: QEMU implementation
by Francesco Romani 12 Sep '14

12 Sep '14

This patchset enhances the QEMU support for the new bulk stats API to include equivalents of these APIs: virDomainBlockInfo virDomainGetInfo - for balloon stats virDomainGetCPUStats virDomainBlockStatsFlags virDomainInterfaceStats virDomainGetVcpusFlags virDomainGetVcpus This subset of API is the one oVirt relies on. Scale/stress test on an oVirt test environment is in progress. changes in v3: more polishing and fixes after first review - addressed Eric's comments. - squashed patches which extracts helpers with patches which use them. - changed gathering strategy: now code tries to reap as much information as possible instead to give up and bail out with error. Only critical errors cause the bulk stats to fail. - moved away from the transfer semantics. I find it error-prone and not flexible enough, I'd like to avoid as much as possible. - rearranged helpers to have one single QEMU query job with many monitor jobs nested inside. - fixed docs. - implemented missing virsh domstats bits. changes in v2: polishing and optimizations. - incorporated feedback from Li Wei (thanks). - added documentation. - optimized block group to gather all the information with just one call to QEMU monitor. - stripped to bare bones merged the 'block info' group into the 'block' group - oVirt actually needs just one stat from there. - reorganized the keys to be more consistent and shorter. The patchset is organized as follows: - the first patch enhance the internal stats gathering API to accomodate the needs of the groups which extract information using QEMU monitor jobs. - the next 6 patches implement the bulk stats groups, extracting helpers where do refactoring to extract internal helpers every time it is feasible and convenient. - the last patch enhance the virsh domstats command with options to use the new bulk stats. *** BLURB HERE *** Francesco Romani (8): qemu: bulk stats: extend internal collection API qemu: bulk stats: implement CPU stats group qemu: bulk stats: implement balloon group qemu: bulk stats: implement VCPU group qemu: bulk stats: implement interface group qemu: bulk stats: implement block group qemu: bulk stats: add block allocation information virsh: add options to query bulk stats group include/libvirt/libvirt.h.in | 5 + src/libvirt.c | 61 +++++ src/qemu/qemu_driver.c | 577 +++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 22 ++ src/qemu/qemu_monitor.h | 21 ++ src/qemu/qemu_monitor_json.c | 211 +++++++++++----- src/qemu/qemu_monitor_json.h | 4 + src/qemu/qemu_monitor_text.c | 13 + src/qemu/qemu_monitor_text.h | 4 + tools/virsh-domain-monitor.c | 35 +++ tools/virsh.pod | 4 +- 11 files changed, 823 insertions(+), 134 deletions(-) -- 1.9.3

4 36

[libvirt] libvirt-python: memory leak after GetXMLDesc?
by Junichi Nomura 12 Sep '14

12 Sep '14

Hello, I've observed memory leak in long-running python program and suspects a bug in libvirt-python. libvirt-python contains auto-generated code like this: libvirt_virDomainGetXMLDesc(...) { ... LIBVIRT_BEGIN_ALLOW_THREADS; c_retval = virDomainGetXMLDesc(domain, flags); LIBVIRT_END_ALLOW_THREADS; py_retval = libvirt_charPtrWrap((char *) c_retval); return py_retval; } virDomainGetXMLDesc() expects the caller to free c_retval. Though it used to be freed in libvirt_charPtrWrap(), commit bb3301ba ("Don't free passed in args in libvirt_charPtrWrap / libvirt_charPtrSizeWrap") has moved the responsibility to the outside. So, it seems either GetXMLDesc should not depend on auto-generation or the generator should be fixed. Any comments? -- Jun'ichi Nomura, NEC Corporation

3 3

[libvirt] [PATCH 0/2] Couple of NVRAM fixes
by Michal Privoznik 12 Sep '14

12 Sep '14

*** BLURB HERE *** Michal Privoznik (2): nvram: Fix permissions virDomainUndefineFlags: Allow NVRAM unlinking include/libvirt/libvirt.h.in | 2 ++ libvirt.spec.in | 2 +- src/qemu/qemu_driver.c | 19 ++++++++++++++++++- src/security/security_selinux.c | 5 ++++- tools/virsh-domain.c | 15 ++++++++++++--- tools/virsh.pod | 6 +++++- 6 files changed, 42 insertions(+), 7 deletions(-) -- 1.8.5.5

3 8

[libvirt] QEMU migration with non-shared storage
by Michael Chapman 12 Sep '14

12 Sep '14

Hello, I am trying to understand libvirt's logic for checking whether migration of a VM is safe, and how it determines which disks should be mirrored by QEMU. My particular use case involves VMs that may have disks backed onto LVM or onto Ceph RBD, or both. As far as I can tell, the qemuMigrationIsSafe check is there to ensure that all disks are readonly, or have cache=none, or their backends can guarantee cache coherence. As far as I can tell, however, QEMU flushes *all* block devices when it pauses a VM's CPUs (just before the final part of migration, for instance), so I'm wondering why this check is needed. Is there any possible situation for the source VM to be paused, for its block devices to be flushed, and yet the destination VM can't see all completed writes? Why is RBD is handled specially in this function? The current logic is that an RBD-backed disk is safe to be migrated even if it's got caching enabled, but I'm not sure how RBD is different from other backends in this regard. If VIR_MIGRATE_NON_SHARED_DISK or _INC is specified, should these safety checks be relaxed? It seems to me that if any non-shared disk is going to be *explicitly* copied from the source to the destination VM, then cache coherence in the backend is irrelevant. At the moment, the set of non-shared block devices copied by VIR_MIGRATE_NON_SHARED_* differs depending on whether NBD is being used in the migration: - If NBD can't be used (e.g. with a tunnelled migration), then QEMU will copy *all* non-readonly block devices; - If NBD is being used, then QEMU will only mirror "shareable", "readonly" or "sourceless" disks. A problem arises with RBD disks that have caching enabled. According to qemuMigrationIsSafe, these disks are "safe" to be migrated. However in either the NBD or the non-NBD case, the RBD disk will be copied. This is clearly not desirable. If RBD is a special case in qemuMigrationIsSafe, does it also need to be a special case when configuring the NBD server? Or, if an NBD server is not going to be used, should the migration be considered "unsafe" if an RBD disk is present? I'd very much appreciate some help in understanding all of this. At the moment, I think my only option is to run RBD without caching at all. However, not only does that result in very poor performance, it also doesn't seem to match the qemuMigrationIsSafe check. Regards, Michael

4 3

[libvirt] [PATCH V4 0/3] Testing libvirt XML -> libxl_domain_config conversion
by Jim Fehlig 11 Sep '14

11 Sep '14

This is version 4 of the work started by danpb: https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html This series tests the conversion of libvirt XML to libxl_domain_config objects by the libvirt libxl driver. Changed in V4: - V3 patches 2-4 have been pushed - Patch 1 is unchanged from V3 - Patch 2 is new and adds tests for virJSONStringCompare - Cleanup of ignored context paths definition in patch 3 (was #5 in V3) Daniel P. Berrange (2): util: Introduce virJSONStringCompare for JSON doc comparisons libxl: Add a test suite for libxl option generator Jim Fehlig (1): tests: add tests for virJSONStringCompare configure.ac | 2 + src/libvirt_private.syms | 1 + src/util/virjson.c | 242 +++++++++++++++++++++++++++++++++ src/util/virjson.h | 16 +++ tests/Makefile.am | 25 +++- tests/jsontest.c | 63 ++++++++- tests/libxlxml2jsondata/basic-hvm.json | 217 +++++++++++++++++++++++++++++ tests/libxlxml2jsondata/basic-hvm.xml | 36 +++++ tests/libxlxml2jsondata/basic-pv.json | 163 ++++++++++++++++++++++ tests/libxlxml2jsondata/basic-pv.xml | 28 ++++ tests/libxlxml2jsontest.c | 228 +++++++++++++++++++++++++++++++ tests/virmocklibxl.c | 87 ++++++++++++ 12 files changed, 1102 insertions(+), 6 deletions(-) create mode 100644 tests/libxlxml2jsondata/basic-hvm.json create mode 100644 tests/libxlxml2jsondata/basic-hvm.xml create mode 100644 tests/libxlxml2jsondata/basic-pv.json create mode 100644 tests/libxlxml2jsondata/basic-pv.xml create mode 100644 tests/libxlxml2jsontest.c create mode 100644 tests/virmocklibxl.c -- 1.8.4.5

1 3