Re: [libvirt] [PATCH 00/26] Resolve more Coverity issues

2014-09-11 Thread Peter Krempa
On 09/10/14 23:56, John Ferlan wrote: > > > Would it be easier if I made batches of 5 patches? or perhaps just > batches of patches of the same error? It might help to overcome the psychological barrier of taking responsibility to review everything in the given series :) Anyhow, I'll have a loo

[libvirt] [PATCH] selinux: Properly check TAP FD label

2014-09-11 Thread Michal Privoznik
After a4431931 the TAP FDs ale labeled with image label instead of the process label. On the other hand, the commit was incomplete as a few lines above, there's still old check for the process label presence while it should be check for the image label instead. Signed-off-by: Michal Privoznik ---

Re: [libvirt] [PATCH 01/26] qemu_driver: Resolve Coverity COPY_PASTE_ERROR

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > In qemuDomainSetBlkioParameters(), Coverity points out that the calls > to qemuDomainParseBlkioDeviceStr() are slightly different and points > out there may be a cut-n-paste error. > > In the first call (AFFECT_LIVE), the second parameter is "param->field";

Re: [libvirt] [PATCH 02/26] remote_driver: Resolve Coverity RESOURCE_LEAK

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Since 98b9acf5aa02551dd37d0209339aba2e22e4004a > > This ends up being a false positive for two reasons... > > expected to be already allocated and thus is passed by value; whereas, > the call into remoteDomainGetJobStats() 'params' is passed by reference. >

Re: [libvirt] [PATCH 03/26] storage: Resolve Coverity UNUSED_VALUE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Since cd4d547576a4f0371d1d4d4e0ca6db124c5ba257 > > Coverity notes that setting 'ret = -3' prior to the unconditional > setting of 'ret = 0' will cause the value to be UNUSED. > > Since the comment indicates that it is expect to allow the code > to continue,

Re: [libvirt] [PATCH 05/26] qemu: Resolve Coverity REVERSE_INULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity complains that checking for !domlist after setting doms = domlist > and making a deref of doms just above > > It seems the call in question was intended to me made in the case that > 'doms' was passed in and not when the virDomainObjListExport() cal

Re: [libvirt] [PATCH 04/26] vbox: Resolve Coverity UNUSED_VALUE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Handle a few places where Coverity complains about the value being > unused. For two of them (Close cases) - the comments above the close > indicate there is no harm to ignore the error - so added an ignore_value. > For the other condition, added an rc check

Re: [libvirt] [PATCH 06/26] storage: Resolve Coverity OVERFLOW_BEFORE_WIDEN

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity complains that when multiplying to 32 bit values that eventually > will be stored in a 64 bit value that it's possible the math could > overflow unless one of the values being multiplied is type cast to > the proper size. > > Signed-off-by: John Fer

Re: [libvirt] [PATCH 14/26] lxc: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If we jump to cleanup before allocating 'result', then the call to > virBlkioDeviceArrayClear() could dereference result > > Signed-off-by: John Ferlan > --- > src/lxc/lxc_driver.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > ACK s

Re: [libvirt] [PATCH 15/26] qemu: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If we jump to cleanup before allocating the 'result', then the call > to virBlkioDeviceArrayClear will deref result causing a problem. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_driver.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(

Re: [libvirt] [PATCH 16/26] network: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If the VIR_STRDUP(exptime,...) fails, then we will jump to cleanup, > no need to check if exptime is set which causes Coverity to issue > a complaint in the virStrToLong_ll call because there wasn't a check > for a NULL value while there was one for the refer

Re: [libvirt] [PATCH 17/26] virstring: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Perhaps a false positive, but since Coverity doesn't understand the > relationship between the 'count' and the 'strings', rather than leave > the chance the on input 'strings' is NULL and causes a deref - just > check for it and return > > Signed-off-by: Joh

Re: [libvirt] [PATCH 18/26] qemu: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If the qemuMigrationEatCookie() fails to set mig, we jump to cleanup: > which will call qemuMigrationCancelDriveMirror() without first checking > if mig == NULL > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_migration.c | 3 ++- > 1 file changed, 2 i

Re: [libvirt] [PATCH 19/26] network_conf: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > The code compares def->forwarders when deciding to return 0 at a > couple of points, then uses "def->nfwds" as a way to index into > the def->forwarders array. That reference results in Coverity > complaining that def->forwarders being NULL was checked as pa

Re: [libvirt] [PATCH 20/26] qemu: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > In qemuProcessInitPCIAddresses() if qemuMonitorGetAllPCIAddresses() > returns a negative (or zero) value, then no need to call the > qemuProcessDetectPCIAddresses(). > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_process.c | 5 +++-- > 1 file changed

Re: [libvirt] [PATCH 21/26] nodeinfo: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If the virNumaGetNodeCPUs() call fails with -1, then jumping to cleanup > with 'cpus == NULL' and calling virCapabilitiesClearHostNUMACellCPUTopology > will cause issues. > > Signed-off-by: John Ferlan > --- > src/nodeinfo.c | 2 +- > 1 file changed, 1 ins

Re: [libvirt] [PATCH 22/26] virsh: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity notes that if virDomainGetCPUStats returns a negative value > into 'nparams' then when we end up at cleanup, the call to virTypedParams > will have issues > > Signed-off-by: John Ferlan > --- > tools/virsh-domain.c | 1 + > 1 file changed, 1 inser

Re: [libvirt] [PATCH 23/26] xen: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity notes that if the call to virBitmapParse() returns a negative > value, then when we jump to the error label, the call to > virCapabilitiesClearHostNUMACellCPUTopology() will have issues > with the negative nb_cpus > > Signed-off-by: John Ferlan > -

Re: [libvirt] [PATCH 24/26] qemu: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity notes that if qemuMonitorGetMachines() returns a negative > nmachines value, then the code at the cleanup label will have issues. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_capabilities.c | 2 +- > 1 file changed, 1 insertion(+), 1 delet

Re: [libvirt] [PATCH 25/26] qemu: Resolve Coverity NEGATIVE_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity notes that if the virConnectListAllDomains returns a negative > value then the loop at the cleanup label that ends on numDomains will > have issues. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_driver.c | 8 +--- > 1 file changed, 5 in

Re: [libvirt] [PATCH 26/26] libxl: Resolve Coverity NULL_RETURNS

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > With all the changes in my previous foray into this code, I forgot to > remove the libxlDomainEventQueue(driver, event); call inside the > dom == NULL condition. > > Signed-off-by: John Ferlan > --- > src/libxl/libxl_migration.c | 1 - > 1 file changed, 1

Re: [libvirt] [PATCH 13/26] qemu: Resolve Coverity FORWARD_NULL

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > If the virJSONValueNewObject() fails, then rather than going to error > and getting a Coverity false positive since it doesn't seem to understand > the relationship between nkeywords, keywords, and values and seems to > believe calling qemuFreeKeywords will c

Re: [libvirt] [PATCH 12/26] virsh: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity points out that if 'dom' isn't returned from virDomainQemuAttach, > then the code already jumps to cleanup, so there was no need for the > subsequent if (dom != NULL) check. > > I moved the error message about failure into the goto cleanup on failur

Re: [libvirt] [PATCH 11/26] tests: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity complains that the various checks for autoincrement and changed > variables are DEADCODE - seems to me to be a false positive - so mark it. They are not false positive. Currently the code doesn't allow that to happen. The tests are designed to catch

Re: [libvirt] [PATCH 10/26] qemu: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Add another 'dead_code_begin' - victims of our own coding practices > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_command.c | 1 + > 1 file changed, 1 insertion(+) > ACK signature.asc Description: OpenPGP digital signature -- libvir-list maili

Re: [libvirt] [PATCH 09/26] virsh: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Coverity points out that by using EMPTYSTR(type) we are guarding against > the possibility that it could be NULL; however, based on how 'type' was > initialized to NULL, then either "ipv4", "ipv6", or "" - there is no way > it could be NULL. Since "-" is sup

Re: [libvirt] [PATCH 08/26] virfile: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
Well, the bug here is a bit more serious than mere DEADCODE ... On 09/05/14 00:26, John Ferlan wrote: > Adjust the parentheses in/for the waitpid loops; otherwise, Coverity > points out: > > (1) Event assignment: Assigning: "waitret" = "waitpid(pid, &status, 0) == > -1" > (2) Event between:

Re: [libvirt] [PATCH V3 1/5] util: Introduce virJSONStringCompare for JSON doc comparisons

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 03, 2014 at 09:07:35PM -0600, Jim Fehlig wrote: > From: "Daniel P. Berrange" > > Comparing JSON docs using strcmp is simple, but is not flexible > as it is sensitive to whitespace used in the doc generation. > When comparing objects it may also be desirable to treat the > existance of

Re: [libvirt] [PATCH V3 3/5] tests: Add more test suite mock helpers

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 03, 2014 at 09:07:37PM -0600, Jim Fehlig wrote: > From: "Daniel P. Berrange" > > Rename the VIR_MOCK_IMPL* macros to VIR_MOCK_WRAP* > and add new VIR_MOCK_IMPL macros which let you directly > implement overrides in the preloaded source. > > Signed-off-by: Daniel P. Berrange > Signed

Re: [libvirt] [PATCH V3 2/5] util: Allow port allocator to skip bind() check

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 03, 2014 at 09:07:36PM -0600, Jim Fehlig wrote: > From: "Daniel P. Berrange" > > Test suites using the port allocator don't want to have different > behaviour depending on whether a port is in use on the host. Add > a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use > to s

Re: [libvirt] [PATCH V3 4/5] libxl: fix mapping of lifecycle actions

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 03, 2014 at 09:07:38PM -0600, Jim Fehlig wrote: > The libxl driver was blindly assigning libvirt's > virDomainLifecycleAction to libxl's libxl_action_on_shutdown, when > in fact the various actions take on different values in these enums. > > Introduce helpers to properly map the enum

Re: [libvirt] [PATCH V3 5/5] libxl: Add a test suite for libxl option generator

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 03, 2014 at 09:07:39PM -0600, Jim Fehlig wrote: > From: "Daniel P. Berrange" > > The libxl library allows a libxl_domain_config object to be > serialized to a JSON string. Use this to allow testing of > the XML -> libxl_domain_config conversion process > > Signed-off-by: Daniel P. Be

Re: [libvirt] [PATCH v2 1/2] parallels: build with parallels SDK

2014-09-11 Thread Daniel P. Berrange
On Sat, Sep 06, 2014 at 08:28:09PM +0400, Dmitry Guryanov wrote: > Executing prlctl command is not an optimal way to interact with > Parallels Cloud Server (PCS), it's better to use parallels SDK, > which is a remote API to paralles dispatcher service. > > We prepared opensource version of this SD

Re: [libvirt] [PATCH v2 2/2] parallels: login to parallels SDK

2014-09-11 Thread Daniel P. Berrange
On Sat, Sep 06, 2014 at 08:28:10PM +0400, Dmitry Guryanov wrote: > Add files parallels_sdk.c and parallels_sdk.h for code > which works with SDK, so libvirt's code will not mix with > dealing with parallels SDK. > > To use Parallels SDK you must first call PrlApi_InitEx function, > and then you wi

Re: [libvirt] [PATCH v1 00/10] Keep original security label

2014-09-11 Thread Daniel P. Berrange
On Wed, Sep 10, 2014 at 03:26:06PM +0200, Michal Privoznik wrote: > I know I've sent several versions like ages ago, so this should > not start with v1, but hey, this is completely new approach, so > I'm gonna start from 1. > > Here, the virtlockd is misused to hold the original seclabels > (altho

Re: [libvirt] [PATCH v1 00/10] Keep original security label

2014-09-11 Thread Michal Privoznik
On 11.09.2014 13:13, Daniel P. Berrange wrote: On Wed, Sep 10, 2014 at 03:26:06PM +0200, Michal Privoznik wrote: I know I've sent several versions like ages ago, so this should not start with v1, but hey, this is completely new approach, so I'm gonna start from 1. Here, the virtlockd is misused

[libvirt] [PATCH 3/5] conf: remove redundant local variable

2014-09-11 Thread Ján Tomko
Use just one int variable for all the FromString calls. --- src/conf/domain_conf.c | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fcf7fb6..0fdfa6f 100644 --- a/src/conf/domain_conf.c +++ b/src/

[libvirt] [PATCH 2/5] Add virSwitch to data types

2014-09-11 Thread Ján Tomko
Just to make this series work until Martin pushes his more complete cleanup: https://www.redhat.com/archives/libvir-list/2014-September/msg00391.html --- docs/schemas/basictypes.rng | 6 ++ 1 file changed, 6 insertions(+) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng

[libvirt] [PATCH 5/5] qemu: wire up virtio-net segment offloading options

2014-09-11 Thread Ján Tomko
--- src/qemu/qemu_command.c | 20 .../qemuxml2argv-net-virtio-disable-offloads.args| 8 tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 30 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxm

[libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-11 Thread Ján Tomko
Add the following attributes: csum, gso, guest_tso4, guest_tso6, guest_ecn to the element of network interface which control the virtio-net device properties of the same names. --- docs/formatdomain.html.in | 27 docs/schemas/domaincommon.rng

[libvirt] [PATCH 1/5] conf: split out virtio net driver formatting

2014-09-11 Thread Ján Tomko
Instead of checking upfront if the element will be needed in a big condition, just format all the attributes into a string and output the element if the string is not empty. --- src/conf/domain_conf.c | 68 -- 1 file changed, 44 insertions(+), 24 d

[libvirt] [PATCH 0/5] Add support for turning off offloading for virtio-net

2014-09-11 Thread Ján Tomko
Ján Tomko (5): conf: split out virtio net driver formatting Add virSwitch to data types conf: remove redundant local variable conf: add options for disabling segment offloading qemu: wire up virtio-net segment offloading options docs/formatdomain.html.in | 27 +

Re: [libvirt] [PATCH 07/26] virsh: Resolve Coverity DEADCODE

2014-09-11 Thread Peter Krempa
On 09/05/14 00:26, John Ferlan wrote: > Since 0766783abbe8bbc9ea686c2c3149f4c0ac139e19 > > Coverity complains that the EDIT_FREE definition results in DEADCODE. > > As it turns out with the change to use the EDIT_FREE macro the call to > vir*Free() wouldn't be necessary nor would it happen... >

[libvirt] [PATCH 0/2] Couple of NVRAM fixes

2014-09-11 Thread Michal Privoznik
*** 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 -

[libvirt] [PATCH 1/2] nvram: Fix permissions

2014-09-11 Thread Michal Privoznik
I've noticed two problem with the automatically created NVRAM varstore file. The first, even though I run qemu as root:root for some reason I get Permission denied when trying to open the _VARS.fd file. The problem is, the upper directory misses execute permissions, which in combination with us dro

[libvirt] [PATCH 2/2] virDomainUndefineFlags: Allow NVRAM unlinking

2014-09-11 Thread Michal Privoznik
When a domain is undefined, there are options to remove it's managed save state or snapshots. However, there's another file that libvirt creates per domain: the NVRAM variable store file. Make sure that the file is not left behind if the domain is undefined. Signed-off-by: Michal Privoznik --- i

Re: [libvirt] [PATCH 00/26] Resolve more Coverity issues

2014-09-11 Thread John Ferlan
On 09/04/2014 06:26 PM, John Ferlan wrote: > Sorry for the large dump, but before I got too involved in other things > I figured I'd go through the list of the remaining 68 Coverity issues > from the new version in order to reduce the pile. Many are benign, some > seemingly false positives, and I

[libvirt] [PATCH] util/virprocess.c: fix MinGW build

2014-09-11 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina --- src/util/virprocess.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 15d8309..3dae1bd 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -28,7 +28,6 @@ #include

Re: [libvirt] [PATCH] util/virprocess.c: fix MinGW build

2014-09-11 Thread Peter Krempa
Subject doesn't describe what caused the build to fail. On 09/11/14 14:57, Pavel Hrdina wrote: Neither the rest of the commit message. > Signed-off-by: Pavel Hrdina > --- > src/util/virprocess.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/src/util/vir

Re: [libvirt] [PATCH] util/virprocess.c: fix MinGW build

2014-09-11 Thread Pavel Hrdina
On 09/11/2014 03:03 PM, Peter Krempa wrote: Subject doesn't describe what caused the build to fail. On 09/11/14 14:57, Pavel Hrdina wrote: Neither the rest of the commit message. The build failed because of missing "sys/syscall.h". Will update the commit message, thanks. Signed-off-by: P

Re: [libvirt] [PATCH] util/virprocess.c: fix MinGW build

2014-09-11 Thread Peter Krempa
On 09/11/14 15:12, Pavel Hrdina wrote: > On 09/11/2014 03:03 PM, Peter Krempa wrote: >> Subject doesn't describe what caused the build to fail. >> >> On 09/11/14 14:57, Pavel Hrdina wrote: >> >> Neither the rest of the commit message. > > The build failed because of missing "sys/syscall.h". > > W

Re: [libvirt] [PATCH] util/virprocess.c: fix MinGW build

2014-09-11 Thread Pavel Hrdina
On 09/11/2014 03:13 PM, Peter Krempa wrote: On 09/11/14 15:12, Pavel Hrdina wrote: On 09/11/2014 03:03 PM, Peter Krempa wrote: Subject doesn't describe what caused the build to fail. On 09/11/14 14:57, Pavel Hrdina wrote: Neither the rest of the commit message. The build failed because of m

Re: [libvirt] [PATCH 1/3] schemas: finish virTristate{Bool, Switch} transition

2014-09-11 Thread Laine Stump
On 09/08/2014 07:40 AM, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > docs/schemas/basictypes.rng | 19 -- > docs/schemas/capability.rng | 10 +-- > docs/schemas/domaincaps.rng | 5 +- > docs/schemas/domaincommon.rng | 155 > +--

Re: [libvirt] [PATCH 1/2] nvram: Fix permissions

2014-09-11 Thread Laszlo Ersek
On 09/11/14 14:09, Michal Privoznik wrote: > I've noticed two problem with the automatically created NVRAM varstore > file. The first, even though I run qemu as root:root for some reason I > get Permission denied when trying to open the _VARS.fd file. The > problem is, the upper directory misses ex

Re: [libvirt] [PATCH 1/2] nvram: Fix permissions

2014-09-11 Thread Michal Privoznik
On 11.09.2014 16:07, Laszlo Ersek wrote: On 09/11/14 14:09, Michal Privoznik wrote: I've noticed two problem with the automatically created NVRAM varstore file. The first, even though I run qemu as root:root for some reason I get Permission denied when trying to open the _VARS.fd file. The probl

Re: [libvirt] [PATCH v2 2/2] parallels: login to parallels SDK

2014-09-11 Thread Dmitry Guryanov
On Thursday 11 September 2014 12:09:20 Daniel P. Berrange wrote: > On Sat, Sep 06, 2014 at 08:28:10PM +0400, Dmitry Guryanov wrote: > > Add files parallels_sdk.c and parallels_sdk.h for code > > which works with SDK, so libvirt's code will not mix with > > dealing with parallels SDK. > > > > To us

Re: [libvirt] [PATCH 1/2] nvram: Fix permissions

2014-09-11 Thread Ján Tomko
On 09/11/2014 04:25 PM, Michal Privoznik wrote: > On 11.09.2014 16:07, Laszlo Ersek wrote: >> On 09/11/14 14:09, Michal Privoznik wrote: >>> I've noticed two problem with the automatically created NVRAM varstore >>> file. The first, even though I run qemu as root:root for some reason I >>> get Perm

Re: [libvirt] [PATCH 2/2] virDomainUndefineFlags: Allow NVRAM unlinking

2014-09-11 Thread Laszlo Ersek
some comments below On 09/11/14 14:09, Michal Privoznik wrote: > When a domain is undefined, there are options to remove it's > managed save state or snapshots. However, there's another file > that libvirt creates per domain: the NVRAM variable store file. > Make sure that the file is not left beh

Re: [libvirt] [PATCH v3] leaseshelper: improvements to support all events

2014-09-11 Thread Peter Krempa
On 08/04/14 09:59, Nehal J Wani wrote: > This patch enables the helper program to detect event(s) triggered when there > is a change in lease length or expiry and client-id. This transfers complete > control of leases database to libvirt and obsoletes use of the lease database > file (.leases). Tha

[libvirt] [PATCH 0/2] Allow using custom tap and vhost devices

2014-09-11 Thread Ján Tomko
This allows usage of alternative tun/tap/vhost backends implemented in userspace via cuse. Ján Tomko (2): conf: add backend element to interfaces Wire up the interface backend options docs/formatdomain.html.in | 20 + docs/schemas/domaincommon.rng

[libvirt] [PATCH 1/2] conf: add backend element to interfaces

2014-09-11 Thread Ján Tomko
For tuning the network, alternative devices for creating tap and vhost devices can be specified via: --- docs/formatdomain.html.in | 20 + docs/schemas/domaincommon.rng | 10 + src/conf/domain_conf.c| 11 + sr

[libvirt] [PATCH 2/2] Wire up the interface backend options

2014-09-11 Thread Ján Tomko
Pass the user-specified tun path down when creating tap device when called from the qemu driver. Also honor the vhost device path specified by user. --- src/bhyve/bhyve_command.c | 2 +- src/bhyve/bhyve_process.c | 2 +- src/network/bridge_driver.c | 6 +++--- src/qemu/qemu_command.c |

[libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

2014-09-11 Thread Peter Krempa
If a floppy drive isn't selected for snapshot explicitly and is empty don't try to snapshot it. For external snapshots this would fail as we can't generate a name for the snapshot from an empty drive. Reported-by: Pavel Hrdina --- src/conf/snapshot_conf.c | 9 - 1 file changed, 8 inserti

Re: [libvirt] [PATCH 02/26] remote_driver: Resolve Coverity RESOURCE_LEAK

2014-09-11 Thread Eric Blake
On 09/11/2014 02:38 AM, Peter Krempa wrote: > On 09/05/14 00:26, John Ferlan wrote: >> Since 98b9acf5aa02551dd37d0209339aba2e22e4004a >> >> This ends up being a false positive for two reasons... >> >> expected to be already allocated and thus is passed by value; whereas, >> the call into remoteDoma

Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group

2014-09-11 Thread Francesco Romani
- Original Message - > From: "Peter Krempa" > To: "Francesco Romani" , libvir-list@redhat.com > Sent: Tuesday, September 9, 2014 1:56:09 PM > Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group > > + * VIR_DOMAIN_STATS_VCPU: Return virtual CPU statistics. > > + * D

Re: [libvirt] [PATCH 02/26] remote_driver: Resolve Coverity RESOURCE_LEAK

2014-09-11 Thread John Ferlan
On 09/11/2014 04:38 AM, Peter Krempa wrote: > On 09/05/14 00:26, John Ferlan wrote: >> Since 98b9acf5aa02551dd37d0209339aba2e22e4004a >> >> This ends up being a false positive for two reasons... >> >> expected to be already allocated and thus is passed by value; whereas, >> the call into remoteDo

Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group

2014-09-11 Thread Peter Krempa
On 09/11/14 17:54, Francesco Romani wrote: > - Original Message - >> From: "Peter Krempa" >> To: "Francesco Romani" , libvir-list@redhat.com >> Sent: Tuesday, September 9, 2014 1:56:09 PM >> Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group > >>> + * VIR_DOMAIN_S

Re: [libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

2014-09-11 Thread Eric Blake
On 09/11/2014 09:47 AM, Peter Krempa wrote: > If a floppy drive isn't selected for snapshot explicitly and is empty > don't try to snapshot it. For external snapshots this would fail as we > can't generate a name for the snapshot from an empty drive. Do we need the same for cdrom drives? > > Rep

Re: [libvirt] [PATCH v3] leaseshelper: improvements to support all events

2014-09-11 Thread Nehal J Wani
On Thu, Sep 11, 2014 at 8:38 PM, Peter Krempa wrote: > On 08/04/14 09:59, Nehal J Wani wrote: >> This patch enables the helper program to detect event(s) triggered when there >> is a change in lease length or expiry and client-id. This transfers complete >> control of leases database to libvirt an

[libvirt] [PATCH v4 0/2] parallels: use parallels SDK instead of prlctl tool

2014-09-11 Thread Dmitry Guryanov
This patchset begins reworking of parallels driver. We have published Opensource version of parallels SDK (under LGPL license), so libvirt can link with it. Changes in v4: * Remove reference counting for PrlApi_InitEx and PrlApi_Deinit * remove Parallels SDK prefix in log messages

[libvirt] [PATCH v4 1/2] parallels: build with parallels SDK

2014-09-11 Thread Dmitry Guryanov
Executing prlctl command is not an optimal way to interact with Parallels Cloud Server (PCS), it's better to use parallels SDK, which is a remote API to paralles dispatcher service. We prepared opensource version of this SDK and published it on github, it's distributed under LGPL license. Here is

[libvirt] [PATCH v4 2/2] parallels: login to parallels SDK

2014-09-11 Thread Dmitry Guryanov
Add files parallels_sdk.c and parallels_sdk.h for code which works with SDK, so libvirt's code will not mix with dealing with parallels SDK. To use Parallels SDK you must first call PrlApi_InitEx function, and then you will be able to connect to a server with PrlSrv_LoginLocalEx function. When you

Re: [libvirt] [PATCH 02/26] remote_driver: Resolve Coverity RESOURCE_LEAK

2014-09-11 Thread John Ferlan
On 09/11/2014 11:55 AM, Eric Blake wrote: > On 09/11/2014 02:38 AM, Peter Krempa wrote: >> On 09/05/14 00:26, John Ferlan wrote: >>> Since 98b9acf5aa02551dd37d0209339aba2e22e4004a >>> >>> This ends up being a false positive for two reasons... >>> >>> expected to be already allocated and thus is p

Re: [libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

2014-09-11 Thread Peter Krempa
On 09/11/14 18:16, Eric Blake wrote: > On 09/11/2014 09:47 AM, Peter Krempa wrote: >> If a floppy drive isn't selected for snapshot explicitly and is empty >> don't try to snapshot it. For external snapshots this would fail as we >> can't generate a name for the snapshot from an empty drive. > > D

Re: [libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

2014-09-11 Thread Peter Krempa
On 09/11/14 18:25, Peter Krempa wrote: > On 09/11/14 18:16, Eric Blake wrote: >> On 09/11/2014 09:47 AM, Peter Krempa wrote: >>> If a floppy drive isn't selected for snapshot explicitly and is empty >>> don't try to snapshot it. For external snapshots this would fail as we >>> can't generate a name

Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group

2014-09-11 Thread Francesco Romani
- Original Message - > From: "Peter Krempa" > To: "Francesco Romani" , libvir-list@redhat.com > Sent: Thursday, September 11, 2014 6:07:48 PM > Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group > > I like this, but I'd also need to do a cross-check on my our code

Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group

2014-09-11 Thread Francesco Romani
- Original Message - > From: "Peter Krempa" > To: "Francesco Romani" , libvir-list@redhat.com > Sent: Tuesday, September 9, 2014 1:42:15 PM > Subject: Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface > group > > + * VIR_DOMAIN_STATS_INTERFACE: Return network interface st

Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group

2014-09-11 Thread Peter Krempa
On 09/11/14 19:11, Francesco Romani wrote: > - Original Message - >> From: "Peter Krempa" >> To: "Francesco Romani" , libvir-list@redhat.com >> Sent: Tuesday, September 9, 2014 1:42:15 PM >> Subject: Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface >> group > >>> + * VIR

[libvirt] [PATCH 3/6] util: storage: Allow metadata crawler to report useful errors

2014-09-11 Thread Peter Krempa
Add a new parameter to virStorageFileGetMetadata that will break the backing chain detection process and report useful error message rather than having to use virStorageFileChainGetBroken. This patch just introduces the option, usage will be provided separately. --- src/qemu/qemu_domain.c

[libvirt] [PATCH 6/6] qemu: Improve check for local storage

2014-09-11 Thread Peter Krempa
Now that we have a simple function to check locality of storage, reuse it in qemuDomainCheckDiskPresence(). Also reuse check for empty storage source. --- src/qemu/qemu_domain.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_doma

[libvirt] [PATCH 0/6] Improve backing store error reporting

2014-09-11 Thread Peter Krempa
Peter Krempa (6): util: Add function to check if a virStorageSource is "empty" qemu: Drop unused formatting of uuid util: storage: Allow metadata crawler to report useful errors qemu: Report better errors from broken backing chains storage: Improve error message when traversing backing c

[libvirt] [PATCH 2/6] qemu: Drop unused formatting of uuid

2014-09-11 Thread Peter Krempa
The formatted UUID isn't used anywhere else in qemuDomainCheckDiskStartupPolicy. Drop it. --- src/qemu/qemu_domain.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c0306d7..bd7d511 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_

[libvirt] [PATCH 4/6] qemu: Report better errors from broken backing chains

2014-09-11 Thread Peter Krempa
Request erroring out from the backing chain traveller and drop qemu's internal backing chain integrity tester. The backin chain traveller reports errors by itself with possibly more detail than qemuDiskChainCheckBroken ever could. We also need to make sure that we reconnect to existing qemu insta

[libvirt] [PATCH 1/6] util: Add function to check if a virStorageSource is "empty"

2014-09-11 Thread Peter Krempa
To express empty drive we historically use storage source with empty path. Unfortunately NBD disks may be declared without a path. Add a helper to wrap this logic. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 20 src/util/virstoragefile.h | 1 + 3 files

[libvirt] [PATCH 5/6] storage: Improve error message when traversing backing chains

2014-09-11 Thread Peter Krempa
Report also the name of the parent file and uid/gid used to access it to help debugging broken storage configurations. --- src/storage/storage_driver.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_drive

[libvirt] [PATCHv2 0/2] conf: snapshot: Don't default-snapshot empty drives

2014-09-11 Thread Peter Krempa
Series contains a duplicate submission of patch 1/2 with my ohter series. Peter Krempa (2): util: Add function to check if a virStorageSource is "empty" conf: snapshot: Don't default-snapshot empty drives src/conf/snapshot_conf.c | 8 +++- src/libvirt_private.syms | 1 + src/util/v

[libvirt] [PATCHv2 1/2] DUPLICATE: util: Add function to check if a virStorageSource is "empty"

2014-09-11 Thread Peter Krempa
To express empty drive we historically use storage source with empty path. Unfortunately NBD disks may be declared without a path. Add a helper to wrap this logic. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 20 src/util/virstoragefile.h | 1 + 3 files

[libvirt] [PATCHv2 2/2] conf: snapshot: Don't default-snapshot empty drives

2014-09-11 Thread Peter Krempa
If a (floppy) drive isn't selected for snapshot explicitly and is empty don't try to snapshot it. For external snapshots this would fail as we can't generate a name for the snapshot from an empty drive. Reported-by: Pavel Hrdina --- Version 2 now doesn't discriminate according to drive type sr

Re: [libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

2014-09-11 Thread Eric Blake
On 09/11/2014 10:28 AM, Peter Krempa wrote: > On 09/11/14 18:25, Peter Krempa wrote: >> On 09/11/14 18:16, Eric Blake wrote: >>> On 09/11/2014 09:47 AM, Peter Krempa wrote: If a floppy drive isn't selected for snapshot explicitly and is empty don't try to snapshot it. For external snapsho

Re: [libvirt] [PATCH] add migration support for OpenVZ driver

2014-09-11 Thread Guido Günther
On Thu, Sep 04, 2014 at 10:27:12PM -0400, Hongbin Lu wrote: > Thanks Guido, > > Your comment is addressed: > https://www.redhat.com/archives/libvir-list/2014-September/msg00284.html. Great. The migration code looks good to me from a OpenVZ view but I don't feel qualified enough to ACK this from a

Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group

2014-09-11 Thread Eric Blake
On 09/11/2014 11:16 AM, Peter Krempa wrote: >> struct _virDomainInterfaceStats { >> long long rx_bytes; >> long long rx_packets; >> long long rx_errs; >> long long rx_drop; >> long long tx_bytes; >> long long tx_packets; >> long long tx_errs; >> long long tx_drop; >

Re: [libvirt] [PATCHv2 1/2] DUPLICATE: util: Add function to check if a virStorageSource is "empty"

2014-09-11 Thread Eric Blake
On 09/11/2014 11:54 AM, Peter Krempa wrote: > To express empty drive we historically use storage source with empty > path. Unfortunately NBD disks may be declared without a path. > > Add a helper to wrap this logic. > --- > src/libvirt_private.syms | 1 + > src/util/virstoragefile.c | 20 ++

Re: [libvirt] [PATCHv2 2/2] conf: snapshot: Don't default-snapshot empty drives

2014-09-11 Thread Eric Blake
On 09/11/2014 11:54 AM, Peter Krempa wrote: > If a (floppy) drive isn't selected for snapshot explicitly and is empty > don't try to snapshot it. For external snapshots this would fail as we > can't generate a name for the snapshot from an empty drive. > > Reported-by: Pavel Hrdina > --- ACK. -

Re: [libvirt] [PATCH 0/6] Improve backing store error reporting

2014-09-11 Thread Richard W.M. Jones
On Thu, Sep 11, 2014 at 07:47:46PM +0200, Peter Krempa wrote: > Peter Krempa (6): > util: Add function to check if a virStorageSource is "empty" > qemu: Drop unused formatting of uuid > util: storage: Allow metadata crawler to report useful errors > qemu: Report better errors from broken b

Re: [libvirt] [PATCH RFC] network: try to eliminate default network conflict during package install

2014-09-11 Thread Cole Robinson
On 09/10/2014 01:54 PM, Laine Stump wrote: > Sometimes libvirt is installed on a host that is already using the > network 192.168.122.0/24. If the libvirt-daemon-config-network package > is installed, this creates a conflict, since that package has been > hard-coded to create a virtual network that

Re: [libvirt] [PATCH RFC] network: try to eliminate default network conflict during package install

2014-09-11 Thread Eric Blake
On 09/10/2014 11:54 AM, Laine Stump wrote: > Sometimes libvirt is installed on a host that is already using the > network 192.168.122.0/24. If the libvirt-daemon-config-network package > is installed, this creates a conflict, since that package has been > hard-coded to create a virtual network that

Re: [libvirt] [PATCH V3 4/5] libxl: fix mapping of lifecycle actions

2014-09-11 Thread Jim Fehlig
Daniel P. Berrange wrote: > On Wed, Sep 03, 2014 at 09:07:38PM -0600, Jim Fehlig wrote: > >> The libxl driver was blindly assigning libvirt's >> virDomainLifecycleAction to libxl's libxl_action_on_shutdown, when >> in fact the various actions take on different values in these enums. >> >> Introd

Re: [libvirt] [PATCH V3 1/5] util: Introduce virJSONStringCompare for JSON doc comparisons

2014-09-11 Thread Jim Fehlig
Daniel P. Berrange wrote: > On Wed, Sep 03, 2014 at 09:07:35PM -0600, Jim Fehlig wrote: > >> From: "Daniel P. Berrange" >> >> Comparing JSON docs using strcmp is simple, but is not flexible >> as it is sensitive to whitespace used in the doc generation. >> When comparing objects it may also be

Re: [libvirt] [PATCH V3 4/5] libxl: fix mapping of lifecycle actions

2014-09-11 Thread Jim Fehlig
Jim Fehlig wrote: > The libxl driver was blindly assigning libvirt's > virDomainLifecycleAction to libxl's libxl_action_on_shutdown, when > in fact the various actions take on different values in these enums. > > Introduce helpers to properly map the enum values. > BTW, forgot to mention that t

Re: [libvirt] [PATCH V3 5/5] libxl: Add a test suite for libxl option generator

2014-09-11 Thread Jim Fehlig
Daniel P. Berrange wrote: > On Wed, Sep 03, 2014 at 09:07:39PM -0600, Jim Fehlig wrote: > >> From: "Daniel P. Berrange" >> >> The libxl library allows a libxl_domain_config object to be >> serialized to a JSON string. Use this to allow testing of >> the XML -> libxl_domain_config conversion pro

  1   2   >