Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Feng, Shaohe


 -Original Message-
 From: Qiao, Liyong
 Sent: Wednesday, June 3, 2015 7:49 AM
 To: Eric Blake; Feng, Shaohe; libvir-list@redhat.com
 Cc: Bhandaru, Malini K; Ding, Jian-feng; Chylinski, Arek; Koniszewski, Pawel; 
 Li,
 Liang Z; berra...@redhat.com; veill...@redhat.com
 Subject: Re: [RFC] libvirt support multi-thread compression for live migration
 
 
 
 On 2015年06月03日 01:02, Eric Blake wrote:
  On 06/02/2015 07:56 AM, Qiao, Liyong wrote:
  Hi Eric
  Thanks for replying the mail, replied in lines.
 
  +virdomainMigrateGetParameters(virDomainPtr domain,
  +  int *level,
  +  int *threads,
  +  int *dthreads,
  +  int flags)
  +
  I'd rather we used virTypedParameters, to make it easier to use the same
 API to pass ALL future possible tuning parameters, rather than just 
 hard-coding
 to only the parameters of this one feature.
 
  Okay ,that sound good, but if virTypedParameters can be passed to
 qemu_driver such as qemu_monitor_json.c qemu_monitor_text.c ?
  [Your quoting style makes it very hard to distinguish original text
  from added text.  Please consider changing your outgoing mail process
  to ensure that you add another level of quoting to all previous text
  so that your added text is the only unquoted text.  Also, configure
  your mailer to wrap long lines.]
 hi Eric, sorry about the inconvenient mail client, I switch outlook to 
 thunder bird,
 hopes it will be better.
  Use existing API for a guide - for example, virDomainSetBlockIoTune
  takes virTypedParamters, as well as defines a specific set of
  parameters that it will understand.  The set of parameters can be
  enlarged without needing a new API (good for backporting features
  without requiring a .so version bump), but for any given libvirt
  version, the set of features understood is finite and limited to what
  you can translate to the QMP call.  So qemu_driver.c would take the
  virTypedParameters, reject the ones it does not understand, and
  convert the ones it does understand into the 'int level, int threads,
  int dthreads' parameters used in qemu_monitor_json.c where you drive
  the actual QMP command with fixed parameters and types.
 ah, that helps, thanks for kindly supporting, we will think a bit more to how
 implement this API (taken virTypedParamters as parameter)
 
 
  If we think that it is worth always turning on both compression styles
 simultaneously, then reuse the bit.  Otherwise, we need a different bit, and
 users can choose which (or both) of the two compression styles as desired.
 
  +1 for reuse compressed, and we can set compress-level,

   Consider this scenario : one of the host/hypervisor are high cpu/memory 
  usage,  
   Cloud Tool, such as Openstack can make a strategy that do not compression
   by multi-thread, for high cpu usage, they just want to use xbzrle.
   
   Is this scenario reasonable? 

  +compress-threads, compress-dthreads by
  +virdomainMigrateSetParameters(maybe some new virsh command---
  +migrate-setparameter)
  But how can we passing these parameter when we using `virsh migrate `, is
 there any parameter we can use in 'virsh migrate' command ?
  Can you point me out ?
  The underlying API already has a form that takes virTypedParameters
  (see virDomainMigrate3()), so your first task is to figure out how to
  extend the API to expose new typed parameters for your new migration
 tunings.
  Once that is done, then you can worry about how to tweak the 'virsh
  migrate' client to pass in those new parameters.
 
 --
 BR, Eli(Li Yong)Qiao


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Qiao,Liyong



On 2015年06月04日 20:09, Jiri Denemark wrote:

On Tue, Jun 02, 2015 at 11:02:27 -0600, Eric Blake wrote:

On 06/02/2015 07:56 AM, Qiao, Liyong wrote:

Hi Eric
Thanks for replying the mail, replied in lines.


+virdomainMigrateGetParameters(virDomainPtr domain,
+  int *level,
+  int *threads,
+  int *dthreads,
+  int flags)
+

I'd rather we used virTypedParameters, to make it easier to use the same API to 
pass ALL future possible tuning parameters, rather than just hard-coding to 
only the parameters of this one feature.

Okay ,that sound good, but if virTypedParameters can be passed to qemu_driver 
such as qemu_monitor_json.c qemu_monitor_text.c ?

[Your quoting style makes it very hard to distinguish original text from
added text.  Please consider changing your outgoing mail process to
ensure that you add another level of quoting to all previous text so
that your added text is the only unquoted text.  Also, configure your
mailer to wrap long lines.]

Use existing API for a guide - for example, virDomainSetBlockIoTune
takes virTypedParamters, as well as defines a specific set of parameters
that it will understand.

And do we actually need to changed these migration parameters on the fly
when migration is already running. I can imagine we would need to do so
for some parameters but multithreaded compression sounds like something
that needs to be configured when migration starts and there's nothing to
be tuned on the fly. If this is the case, I think we should just add
these new parameters to virDomainMigrate3 instead of requiring another
API to be called to actually configure multithreaded compression.

The separate API makes it a bit harder to repeat migration (which
previously failed) with a different parameters (e.g., without
multithreaded compression).

hi jirka
thanks for your advice, that make sense for me.

if I understanding you correctly,
then we need only do this

virsh migrate --live --compressed --compress-level 1--compress-threads 8 
--decompress-threads 2


and all compressed parameters (compress-level ,compress-threads, 
decompress-threads)  only
be specified when we do call virDomainMigrate3 instead of expose to user 
by 'virsh migrate-setparameters' ?


I think that's good for me, and make user easy to start LM.

thanks,
Eli.


Jirka


--
BR, Eli(Li Yong)Qiao

attachment: liyong_qiao.vcf--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Qiao,Liyong



On 2015年06月04日 20:01, Jiri Denemark wrote:

On Wed, Jun 03, 2015 at 15:13:17 +, Feng, Shaohe wrote:

Eric,
Thank you for reply.

Is it necessary to expose these two APIs to user application?
+ virdomainMigrateSetCapabilities
+ virdomainMigrateGetCapabilities

For qemu , the migration capabilities are xbzrle, rdma-pin-all,  
auto-converge,
  zero-blocks and compress.

No, these capabilities are either turned on/off based on flags passed to
virDomainMigrate3 API.
I think we need to call qemuMonitorGetMigrationCapability when 
qemuMigrationSetCompression
if the source/dest node doesn't support  'compress' capability, we need 
to stop migration if
flags contain VIR_MIGRATE_COMPRESSED (currently it stands for xbzrle, 
and Eric's previous mail

suggest we share this flag with 'compress')

-Eli.

Jirka


--
BR, Eli(Li Yong)Qiao

attachment: liyong_qiao.vcf--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] remote: fix odd comma operator

2015-06-04 Thread Eric Blake
Commit 1882c0bd accidentally used ',' instead of ';'; oddly
enough, the result was still syntactically valid (yes, C is
a fun language).  But it made me do a double take; it's better
to use idiomatic syntax.

* daemon/remote.c (remoteRelayDomainEventDeviceAdded): Fix
harmless typo.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the trivial rule.

 daemon/remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index e259a76..e9e2dca 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1068,7 +1068,7 @@ remoteRelayDomainEventDeviceAdded(virConnectPtr conn,
 return -1;

 make_nonnull_domain(data.dom, dom);
-data.callbackID = callback-callbackID,
+data.callbackID = callback-callbackID;

 remoteDispatchObjectEventSend(callback-client, remoteProgram,
   
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED,
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Eric Blake
On 06/04/2015 06:31 PM, Qiao,Liyong wrote:
 
 
 On 2015年06月04日 20:01, Jiri Denemark wrote:
 On Wed, Jun 03, 2015 at 15:13:17 +, Feng, Shaohe wrote:
 Eric,
 Thank you for reply.

 Is it necessary to expose these two APIs to user application?
 + virdomainMigrateSetCapabilities
 + virdomainMigrateGetCapabilities

 For qemu , the migration capabilities are xbzrle, rdma-pin-all, 
 auto-converge,
   zero-blocks and compress.
 No, these capabilities are either turned on/off based on flags passed to
 virDomainMigrate3 API.
 I think we need to call qemuMonitorGetMigrationCapability when

[It's hard to read replies that aren't visually distinct from the rest
of the text; you'll notice that most posters on this list leave a blank
line before and after any text they add, so that a scan of the left-most
column can easily spot blanks as a key for where new content begins]

 qemuMigrationSetCompression
 if the source/dest node doesn't support  'compress' capability, we need
 to stop migration if
 flags contain VIR_MIGRATE_COMPRESSED (currently it stands for xbzrle,
 and Eric's previous mail
 suggest we share this flag with 'compress')

No, I was asking whether libvirt's 'compress' flag should imply all
possible compression at once, or whether there are cases where xbzrle
and multi-thread are independently useful and should not both be on at
once.  If it is safe to always use both, then we don't need a new flag,
but I don't know if it is safe.

The migration handshake is already set up to negotiate capabilities
between source and destination, whether it takes one flag (turning on
both compressions, or gracefully falling back to xbzrle alone) or two
(one per compression type, and erroring out if a request is made for an
unsupported compression).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Li, Liang Z
 
  On 2015年06月04日 20:01, Jiri Denemark wrote:
  On Wed, Jun 03, 2015 at 15:13:17 +, Feng, Shaohe wrote:
  Eric,
  Thank you for reply.
 
  Is it necessary to expose these two APIs to user application?
  + virdomainMigrateSetCapabilities
  + virdomainMigrateGetCapabilities
 
  For qemu , the migration capabilities are xbzrle, rdma-pin-all,
  auto-converge,
zero-blocks and compress.
  No, these capabilities are either turned on/off based on flags passed
  to
  virDomainMigrate3 API.
  I think we need to call qemuMonitorGetMigrationCapability when
 
 [It's hard to read replies that aren't visually distinct from the rest of the 
 text;
 you'll notice that most posters on this list leave a blank line before and 
 after
 any text they add, so that a scan of the left-most column can easily spot
 blanks as a key for where new content begins]
 
  qemuMigrationSetCompression
  if the source/dest node doesn't support  'compress' capability, we
  need to stop migration if flags contain VIR_MIGRATE_COMPRESSED
  (currently it stands for xbzrle, and Eric's previous mail suggest we
  share this flag with 'compress')
 
 No, I was asking whether libvirt's 'compress' flag should imply all possible
 compression at once, or whether there are cases where xbzrle and multi-
 thread are independently useful and should not both be on at once.  If it is
 safe to always use both, then we don't need a new flag, but I don't know if it
 is safe.

Multiple thread compression is CPU-intensive, while xbzrle is RAM-intensive.
Although they can co-work correctly, there is no evidence show that turning both
on will achieve better performance than using xbzrle or multi-thread 
independently.

May be it's better to let the users decide which one to use depend on their 
specific 
use case.
   
 The migration handshake is already set up to negotiate capabilities between
 source and destination, whether it takes one flag (turning on both
 compressions, or gracefully falling back to xbzrle alone) or two (one per
 compression type, and erroring out if a request is made for an unsupported
 compression).
 
 --
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 3/4] qemu: Add capability for vhost-user multiqueue

2015-06-04 Thread Martin Kletzander

On Thu, Jun 04, 2015 at 07:04:15PM +0200, Martin Kletzander wrote:

The support for this was added in QEMU with commit
830d70db692e374b5f4407f96a1ceefdcc97.  Unfortunately we have to do
another ugly version-based capability check.  The other option would be
not to check for the capability at all and leave that to qemu as it's
doen with multiqueue tap devices.



s/doen/done/

Of course I haven't amended the commit properly before sending.  Sorry
for that.  I guess it's too late for my brain to function properly for
so long.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 4/5] rpc: Don't use unrelated value as privateData of client

2015-06-04 Thread Martin Kletzander
From: Daniel P. Berrange berra...@redhat.com

Append privateData of the client only if there are any, otherwise the
previous value (socket data) will get there again.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetserverclient.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 34c199445401..0e3a71f9b271 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -536,13 +536,14 @@ virJSONValuePtr 
virNetServerClientPreExecRestart(virNetServerClientPtr client)
 goto error;
 }

-if (client-privateData  client-privateDataPreExecRestart 
-!(child = client-privateDataPreExecRestart(client, 
client-privateData)))
-goto error;
+if (client-privateData  client-privateDataPreExecRestart) {
+if (!(child = client-privateDataPreExecRestart(client, 
client-privateData)))
+goto error;

-if (virJSONValueObjectAppend(object, privateData, child)  0) {
-virJSONValueFree(child);
-goto error;
+if (virJSONValueObjectAppend(object, privateData, child)  0) {
+virJSONValueFree(child);
+goto error;
+}
 }

 virObjectUnlock(client);
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] qemu: add multiqueue vhost-user support

2015-06-04 Thread Martin Kletzander

On Thu, Jun 04, 2015 at 05:54:17PM +0200, Maxime Leroy wrote:

On Thu, Jun 4, 2015 at 3:43 PM, Martin Kletzander mklet...@redhat.com wrote:

From: Maxime Leroy maxime.le...@6wind.com

This patch adds the support of queues attribute of the driver element
for vhost-user interface type. Example:

interface type='vhostuser'
  mac address='52:54:00:ee:96:6d'/
  source type='unix' path='/tmp/vhost2.sock' mode='client'/
  model type='virtio'/
  driver queues='4'/
/interface

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in | 11 +--
 src/qemu/qemu_command.c   | 15 ++-
 ...stuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
 ...hostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
 tests/qemuxml2argvtest.c  |  3 +++
 5 files changed, 37 insertions(+), 4 deletions(-)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
qemuxml2argv-net-vhostuser-multiq.args} (75%)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
qemuxml2argv-net-vhostuser-multiq.xml} (87%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 72ad54cee188..85238a16af8d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4260,7 +4260,8 @@ qemu-kvm -net nic,model=? /dev/null
 type='virtio'/gt;/code, multiple packet processing queues can be
 created; each queue will potentially be handled by a different
 processor, resulting in much higher throughput.
-span class=sinceSince 1.0.6 (QEMU and KVM only)/span
+span class=sinceSince 1.0.6 (QEMU and KVM only) and for vhost-user
+  since 1.2.17/span
   /dd
   dtcodehost/code offloading options/dt
   dd
@@ -4581,9 +4582,15 @@ qemu-kvm -net nic,model=? /dev/null
   lt;devicesgt;
 lt;interface type='vhostuser'gt;
   lt;mac address='52:54:00:3b:83:1a'/gt;
-  lt;source type='unix' path='/tmp/vhost.sock' mode='server'/gt;
+  lt;source type='unix' path='/tmp/vhost1.sock' mode='server'/gt;
   lt;model type='virtio'/gt;
 lt;/interfacegt;
+lt;interface type='vhostuser'gt;
+  lt;mac address='52:54:00:3b:83:1b'/gt;
+  lt;source type='unix' path='/tmp/vhost2.sock' mode='client'/gt;
+  lt;model type='virtio'/gt;
+  lt;driver queues='5'/gt;
+lt;/interfacegt;
   lt;/devicesgt;
   .../pre

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 61faa576e11b..f805f6700e71 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8089,6 +8089,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 {
 virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
 virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
+unsigned int queues = 1;


Why setting queues to 1 and not to net-driver.virtio.queues directly ?



Oh, you're right, I didn't write it from scratch, I just renamed it.
I'm amending the patch right now.


 char *nic = NULL;

 if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
@@ -8126,13 +8127,25 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 virBufferAsprintf(netdev_buf, type=vhost-user,id=host%s,chardev=char%s,
   net-info.alias, net-info.alias);

+queues = net-driver.virtio.queues;
+if (queues) {


I know it's never set to 1 thanks to your patch: conf: Ignore
multiqueue with one queue.

Anyway I think we should check if queues is superior to 1 for
improving code readability.



It seems more readable to me if you just check whether there is
anything to setup (treating it as a bool) and then just setting up
what needs to be done.  I'm OK with changing it back to (queues  1).


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 0/5] RPC JSON (de)serialization and fixes

2015-06-04 Thread Martin Kletzander
I took the liberty of changing Daniel's version and fix it up a bit.
I've split it into multiple patches, removed unneeded functions, and
fixed it for building without avahi.


First version here:

https://www.redhat.com/archives/libvir-list/2015-May/msg00812.html


Daniel P. Berrange (4):
  rpc: add testing of RPC JSON (de)serialization
  rpc: Make virNetServerAddClient function dynamic
  rpc: Don't use unrelated value as privateData of client
  rpc: Fix reference counting around virNetSocketAddIOCallback

Martin Kletzander (1):
  mdns: Set error when failing due to missing avahi

 src/libvirt_remote.syms|   1 +
 src/rpc/virnetserver.c |   4 +-
 src/rpc/virnetserver.h |   3 +
 src/rpc/virnetserverclient.c   |  13 +-
 src/rpc/virnetservermdns.c |   8 +-
 src/rpc/virnetserverservice.c  |   6 +-
 tests/Makefile.am  |   7 +
 tests/virnetserverdata/README  |  14 +
 .../virnetserverdata/input-data-anon-clients.json  |  62 +
 .../input-data-initial-nomdns.json |  61 +
 tests/virnetserverdata/input-data-initial.json |  62 +
 .../virnetserverdata/output-data-anon-clients.json |  62 +
 .../output-data-initial-nomdns.json|  62 +
 tests/virnetserverdata/output-data-initial.json|  63 +
 tests/virnetservertest.c   | 284 +
 15 files changed, 698 insertions(+), 14 deletions(-)
 create mode 100644 tests/virnetserverdata/README
 create mode 100644 tests/virnetserverdata/input-data-anon-clients.json
 create mode 100644 tests/virnetserverdata/input-data-initial-nomdns.json
 create mode 100644 tests/virnetserverdata/input-data-initial.json
 create mode 100644 tests/virnetserverdata/output-data-anon-clients.json
 create mode 100644 tests/virnetserverdata/output-data-initial-nomdns.json
 create mode 100644 tests/virnetserverdata/output-data-initial.json
 create mode 100644 tests/virnetservertest.c

--
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 3/5] rpc: Make virNetServerAddClient function dynamic

2015-06-04 Thread Martin Kletzander
From: Daniel P. Berrange berra...@redhat.com

As opposed to 'static'; by exporting it (privately).

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/libvirt_remote.syms | 1 +
 src/rpc/virnetserver.c  | 4 ++--
 src/rpc/virnetserver.h  | 3 +++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 6b520b5fa923..0d650b6b2737 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -77,6 +77,7 @@ xdr_virNetMessageError;


 # rpc/virnetserver.h
+virNetServerAddClient;
 virNetServerAddProgram;
 virNetServerAddService;
 virNetServerAddShutdownInhibition;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 42427dc5babb..091a1dc0bc8f 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -259,8 +259,8 @@ static int 
virNetServerDispatchNewMessage(virNetServerClientPtr client,
 }


-static int virNetServerAddClient(virNetServerPtr srv,
- virNetServerClientPtr client)
+int virNetServerAddClient(virNetServerPtr srv,
+  virNetServerClientPtr client)
 {
 virObjectLock(srv);

diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 8c5ae072db28..4b452be5a1ad 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -79,6 +79,9 @@ int virNetServerAddService(virNetServerPtr srv,
virNetServerServicePtr svc,
const char *mdnsEntryName);

+int virNetServerAddClient(virNetServerPtr srv,
+  virNetServerClientPtr client);
+
 int virNetServerAddProgram(virNetServerPtr srv,
virNetServerProgramPtr prog);

-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 4/4] qemu: add multiqueue vhost-user support

2015-06-04 Thread Martin Kletzander
From: Maxime Leroy maxime.le...@6wind.com

This patch adds the support of queues attribute of the driver element
for vhost-user interface type. Example:

interface type='vhostuser'
  mac address='52:54:00:ee:96:6d'/
  source type='unix' path='/tmp/vhost2.sock' mode='client'/
  model type='virtio'/
  driver queues='4'/
/interface

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207692

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in  | 11 +--
 src/qemu/qemu_command.c| 14 +-
 ...ostuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
 ...vhostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
 tests/qemuxml2argvtest.c   |  3 +++
 5 files changed, 36 insertions(+), 4 deletions(-)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
qemuxml2argv-net-vhostuser-multiq.args} (75%)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
qemuxml2argv-net-vhostuser-multiq.xml} (87%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 72ad54cee188..85238a16af8d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4260,7 +4260,8 @@ qemu-kvm -net nic,model=? /dev/null
 type='virtio'/gt;/code, multiple packet processing queues can be
 created; each queue will potentially be handled by a different
 processor, resulting in much higher throughput.
-span class=sinceSince 1.0.6 (QEMU and KVM only)/span
+span class=sinceSince 1.0.6 (QEMU and KVM only) and for vhost-user
+  since 1.2.17/span
   /dd
   dtcodehost/code offloading options/dt
   dd
@@ -4581,9 +4582,15 @@ qemu-kvm -net nic,model=? /dev/null
   lt;devicesgt;
 lt;interface type='vhostuser'gt;
   lt;mac address='52:54:00:3b:83:1a'/gt;
-  lt;source type='unix' path='/tmp/vhost.sock' mode='server'/gt;
+  lt;source type='unix' path='/tmp/vhost1.sock' mode='server'/gt;
   lt;model type='virtio'/gt;
 lt;/interfacegt;
+lt;interface type='vhostuser'gt;
+  lt;mac address='52:54:00:3b:83:1b'/gt;
+  lt;source type='unix' path='/tmp/vhost2.sock' mode='client'/gt;
+  lt;model type='virtio'/gt;
+  lt;driver queues='5'/gt;
+lt;/interfacegt;
   lt;/devicesgt;
   .../pre

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 61faa576e11b..862729f01352 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8089,6 +8089,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 {
 virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
 virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
+unsigned int queues = net-driver.virtio.queues;
 char *nic = NULL;

 if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
@@ -8126,13 +8127,24 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 virBufferAsprintf(netdev_buf, type=vhost-user,id=host%s,chardev=char%s,
   net-info.alias, net-info.alias);

+if (queues  1) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(multi-queue is not supported for vhost-user 
+ with this QEMU binary));
+goto error;
+}
+virBufferAsprintf(netdev_buf, ,queues=%u, queues);
+}
+
 virCommandAddArg(cmd, -chardev);
 virCommandAddArgBuffer(cmd, chardev_buf);

 virCommandAddArg(cmd, -netdev);
 virCommandAddArgBuffer(cmd, netdev_buf);

-if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, 0, qemuCaps))) {
+if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex,
+   queues, qemuCaps))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(Error generating NIC -device string));
 goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
similarity index 75%
copy from tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
copy to tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
index ac43630979ad..8affb53b3958 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
@@ -9,4 +9,8 @@ pc -m 214 -smp 1 -nographic -nodefaults -monitor 
unix:/tmp/test-monitor,server,n
 -netdev type=vhost-user,id=hostnet1,chardev=charnet1 \
 -device 
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,addr=0x4 
\
 -netdev socket,listen=:2015,id=hostnet2 \
--device 
rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,addr=0x5
+-device 
rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,addr=0x5 \
+-chardev socket,id=charnet3,path=/tmp/vhost2.sock \

[libvirt] [PATCH v2 0/4] Add support for vhost-user with multi-queue

2015-06-04 Thread Martin Kletzander
Also some tiny clean-up.

Martin Kletzander (2):
  conf: Ignore multiqueue with one queue.
  qemu: Add capability for vhost-user multiqueue

Maxime Leroy (2):
  docs: Clarify that attribute name is not used for vhostuser
  qemu: add multiqueue vhost-user support

 docs/formatdomain.html.in| 16 ++--
 src/conf/domain_conf.c   |  3 ++-
 src/qemu/qemu_capabilities.c |  6 ++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  | 14 +-
 ...tuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
 ...ostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
 .../qemuxml2argv-tap-vhost-incorrect.xml |  6 ++
 tests/qemuxml2argvtest.c |  3 +++
 .../qemuxml2xmlout-tap-vhost-incorrect.xml   |  6 ++
 10 files changed, 62 insertions(+), 5 deletions(-)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
qemuxml2argv-net-vhostuser-multiq.args} (75%)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
qemuxml2argv-net-vhostuser-multiq.xml} (87%)

--
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 2/5] rpc: add testing of RPC JSON (de)serialization

2015-06-04 Thread Martin Kletzander
From: Daniel P. Berrange berra...@redhat.com

The virNetServer class has the ability to serialize its state
to a JSON file, and then re-load that data after an in-place
execve() call to re-connect to active file handles. This data
format is critical ABI that must have compatibility across
releases, so it should be tested...

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tests/Makefile.am  |   7 +
 tests/virnetserverdata/README  |  14 +
 .../virnetserverdata/input-data-anon-clients.json  |  62 +
 .../input-data-initial-nomdns.json |  61 +
 tests/virnetserverdata/input-data-initial.json |  62 +
 .../virnetserverdata/output-data-anon-clients.json |  62 +
 .../output-data-initial-nomdns.json|  62 +
 tests/virnetserverdata/output-data-initial.json|  63 +
 tests/virnetservertest.c   | 284 +
 9 files changed, 677 insertions(+)
 create mode 100644 tests/virnetserverdata/README
 create mode 100644 tests/virnetserverdata/input-data-anon-clients.json
 create mode 100644 tests/virnetserverdata/input-data-initial-nomdns.json
 create mode 100644 tests/virnetserverdata/input-data-initial.json
 create mode 100644 tests/virnetserverdata/output-data-anon-clients.json
 create mode 100644 tests/virnetserverdata/output-data-initial-nomdns.json
 create mode 100644 tests/virnetserverdata/output-data-initial.json
 create mode 100644 tests/virnetservertest.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8e2dbec9af56..c9e2c8a0afa2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -188,6 +188,7 @@ if WITH_REMOTE
 test_programs += \
virnetmessagetest \
virnetsockettest \
+   virnetservertest \
virnetserverclienttest \
$(NULL)
 if WITH_GNUTLS
@@ -921,6 +922,12 @@ virnetsockettest_SOURCES = \
virnetsockettest.c testutils.h testutils.c
 virnetsockettest_LDADD = $(LDADDS)

+virnetservertest_SOURCES = \
+   virnetservertest.c \
+   testutils.h testutils.c
+virnetservertest_CFLAGS = $(XDR_CFLAGS) $(AM_CFLAGS)
+virnetservertest_LDADD = $(LDADDS)
+
 virnetserverclienttest_SOURCES = \
virnetserverclienttest.c \
testutils.h testutils.c
diff --git a/tests/virnetserverdata/README b/tests/virnetserverdata/README
new file mode 100644
index ..d6d79d27d5ea
--- /dev/null
+++ b/tests/virnetserverdata/README
@@ -0,0 +1,14 @@
+   virnetservertest data files
+   ===
+
+The various input-data-*.json files are a record of all the historical
+formats that libvirt has been able to produce data for. Everytime a
+new field is added to the JSON output, a *new* input data file should
+be created. We must not add new fields to existing input-data files,
+nor must we ever re-structure them if code changes, as we must check
+new code handles the legacy formats.
+
+The various output-data-*.json files are the record of what the *new*
+JSON output should look like for the correspondingly named input-data
+file. It is permissible to change the existing output-data-*.json
+files if the format we save in is updated.
diff --git a/tests/virnetserverdata/input-data-anon-clients.json 
b/tests/virnetserverdata/input-data-anon-clients.json
new file mode 100644
index ..8a51ff53d6cf
--- /dev/null
+++ b/tests/virnetserverdata/input-data-anon-clients.json
@@ -0,0 +1,62 @@
+{
+min_workers: 10,
+max_workers: 50,
+priority_workers: 5,
+max_clients: 100,
+max_anonymous_clients: 10,
+keepaliveInterval: 120,
+keepaliveCount: 5,
+keepaliveRequired: true,
+services: [
+{
+auth: 0,
+readonly: true,
+nrequests_client_max: 2,
+socks: [
+{
+fd: 100,
+errfd: -1,
+pid: 0,
+isClient: false
+}
+]
+},
+{
+auth: 2,
+readonly: false,
+nrequests_client_max: 5,
+socks: [
+{
+fd: 101,
+errfd: -1,
+pid: 0,
+isClient: false
+}
+]
+}
+],
+clients: [
+{
+auth: 1,
+readonly: true,
+nrequests_max: 15,
+sock: {
+fd: 102,
+errfd: -1,
+pid: -1,
+isClient: true
+}
+},
+{
+auth: 2,
+readonly: true,
+nrequests_max: 66,
+sock: {
+fd: 103,
+errfd: -1,
+pid: -1,
+isClient: true
+}
+}
+]
+}
diff --git a/tests/virnetserverdata/input-data-initial-nomdns.json 
b/tests/virnetserverdata/input-data-initial-nomdns.json
new file 

[libvirt] [PATCH v2 5/5] rpc: Fix reference counting around virNetSocketAddIOCallback

2015-06-04 Thread Martin Kletzander
From: Daniel P. Berrange berra...@redhat.com

Ref service passed as a parameter to the callback.  And don't unref the
socket that is part of the service being passed at another point in code.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetserverservice.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index d84b6defd4e5..9087473efd39 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -301,12 +301,15 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,

 /* IO callback is initially disabled, until we're ready
  * to deal with incoming clients */
+virObjectRef(svc);
 if (virNetSocketAddIOCallback(svc-socks[i],
   0,
   virNetServerServiceAccept,
   svc,
-  virObjectFreeCallback)  0)
+  virObjectFreeCallback)  0) {
+virObjectUnref(svc);
 goto error;
+}
 }


@@ -386,7 +389,6 @@ virNetServerServicePtr 
virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
   svc,
   virObjectFreeCallback)  0) {
 virObjectUnref(svc);
-virObjectUnref(sock);
 goto error;
 }
 }
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 1/5] mdns: Set error when failing due to missing avahi

2015-06-04 Thread Martin Kletzander
When building without avahi support, we used VIR_DEBUG() to note that to
the user.  However, functions that fail because of that (return NULL/-1)
did not set the error message.  This was the only file that forgot to do
such thing.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetservermdns.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/rpc/virnetservermdns.c b/src/rpc/virnetservermdns.c
index 131968061c5f..7f12a2947363 100644
--- a/src/rpc/virnetservermdns.c
+++ b/src/rpc/virnetservermdns.c
@@ -617,14 +617,14 @@ static const char *unsupported = N_(avahi not available 
at build time);
 virNetServerMDNS *
 virNetServerMDNSNew(void)
 {
-VIR_DEBUG(%s, _(unsupported));
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unsupported));
 return NULL;
 }

 int
 virNetServerMDNSStart(virNetServerMDNS *mdns ATTRIBUTE_UNUSED)
 {
-VIR_DEBUG(%s, _(unsupported));
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unsupported));
 return -1;
 }

@@ -632,7 +632,7 @@ virNetServerMDNSGroupPtr
 virNetServerMDNSAddGroup(virNetServerMDNS *mdns ATTRIBUTE_UNUSED,
  const char *name ATTRIBUTE_UNUSED)
 {
-VIR_DEBUG(%s, _(unsupported));
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unsupported));
 return NULL;
 }

@@ -648,7 +648,7 @@ virNetServerMDNSAddEntry(virNetServerMDNSGroupPtr group 
ATTRIBUTE_UNUSED,
  const char *type ATTRIBUTE_UNUSED,
  int port ATTRIBUTE_UNUSED)
 {
-VIR_DEBUG(%s, _(unsupported));
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unsupported));
 return NULL;
 }

-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib v2] storage-pool: API to get/set autostart flag

2015-06-04 Thread Christophe Fergeau
Looks good, ACK.

Christophe

On Wed, Jun 03, 2015 at 09:46:35PM +0100, Zeeshan Ali (Khattak) wrote:
 Add binding for virStoragePoolGetAutostart  virStoragePoolSetAutostart.
 ---
  libvirt-gobject/libvirt-gobject-storage-pool.c | 51 
 ++
  libvirt-gobject/libvirt-gobject-storage-pool.h |  5 +++
  libvirt-gobject/libvirt-gobject.sym|  6 +++
  3 files changed, 62 insertions(+)
 
 diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
 b/libvirt-gobject/libvirt-gobject-storage-pool.c
 index f3eac0d..7f26b1b 100644
 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
 +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
 @@ -1048,6 +1048,57 @@ gboolean gvir_storage_pool_delete (GVirStoragePool 
 *pool,
  return TRUE;
  }
  
 +/**
 + * gvir_storage_pool_get_autostart:
 + * @pool: the storage pool
 + * @err: return location for any #GError
 + *
 + * Return value: #True if autostart is enabled, #False otherwise.
 + */
 +gboolean gvir_storage_pool_get_autostart(GVirStoragePool *pool,
 + GError **err)
 +{
 +int ret;
 +
 +g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE);
 +g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
 +
 +if (virStoragePoolGetAutostart(pool-priv-handle, ret)) {
 +gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
 +   0,
 +   Failed to get autostart flag from storage 
 pool);
 +}
 +
 +return !!ret;
 +}
 +
 +/**
 + * gvir_storage_pool_set_autostart:
 + * @pool: the storage pool
 + * @autostart: Whether or not to autostart
 + * @err: return location for any #GError
 + *
 + * Sets whether or not storage pool @pool is started automatically on boot.
 + *
 + * Return value: #TRUE on success, #FALSE otherwise.
 + */
 +gboolean gvir_storage_pool_set_autostart(GVirStoragePool *pool,
 + gboolean autostart,
 + GError **err)
 +{
 +g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE);
 +g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
 +
 +if (virStoragePoolSetAutostart(pool-priv-handle, autostart)) {
 +gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
 +   0,
 +   Failed to set autostart flag on storage 
 pool);
 +return FALSE;
 +}
 +
 +return TRUE;
 +}
 +
  static void
  gvir_storage_pool_delete_helper(GSimpleAsyncResult *res,
  GObject *object,
 diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.h 
 b/libvirt-gobject/libvirt-gobject-storage-pool.h
 index f8529f0..f7f879c 100644
 --- a/libvirt-gobject/libvirt-gobject-storage-pool.h
 +++ b/libvirt-gobject/libvirt-gobject-storage-pool.h
 @@ -166,6 +166,11 @@ void gvir_storage_pool_delete_async (GVirStoragePool 
 *pool,
  gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool,
   GAsyncResult *result,
   GError **err);
 +gboolean gvir_storage_pool_get_autostart(GVirStoragePool *pool,
 + GError **err);
 +gboolean gvir_storage_pool_set_autostart(GVirStoragePool *pool,
 + gboolean autostart,
 + GError **err);
  
  G_END_DECLS
  
 diff --git a/libvirt-gobject/libvirt-gobject.sym 
 b/libvirt-gobject/libvirt-gobject.sym
 index 927cad9..dcda675 100644
 --- a/libvirt-gobject/libvirt-gobject.sym
 +++ b/libvirt-gobject/libvirt-gobject.sym
 @@ -265,4 +265,10 @@ LIBVIRT_GOBJECT_0.2.0 {
   gvir_domain_open_graphics_fd;
  } LIBVIRT_GOBJECT_0.1.9;
  
 +LIBVIRT_GOBJECT_0.2.1 {
 +  global:
 + gvir_storage_pool_get_autostart;
 + gvir_storage_pool_set_autostart;
 +} LIBVIRT_GOBJECT_0.2.0;
 +
  #  define new API here using predicted next version number 
 -- 
 2.4.2
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


pgpAYPbokrUo3.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 2/4] docs: Clarify that attribute name is not used for vhostuser

2015-06-04 Thread Martin Kletzander
From: Maxime Leroy maxime.le...@6wind.com

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in | 5 +
 1 file changed, 5 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 178199679ed3..72ad54cee188 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4190,6 +4190,11 @@ qemu-kvm -net nic,model=? /dev/null
 span class=sinceSince 1.0.5 (QEMU and KVM only, requires
 kernel 3.6 or newer)/span
   /dd
+  dd
+For interfaces of type='vhostuser', the codename/code
+attribute is ignored. The backend driver used is always
+vhost-user.
+  /dd

   dtcodetxmode/code/dt
   dd
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] qemu: add multiqueue vhost-user support

2015-06-04 Thread Maxime Leroy
On Thu, Jun 4, 2015 at 3:43 PM, Martin Kletzander mklet...@redhat.com wrote:
 From: Maxime Leroy maxime.le...@6wind.com

 This patch adds the support of queues attribute of the driver element
 for vhost-user interface type. Example:

 interface type='vhostuser'
   mac address='52:54:00:ee:96:6d'/
   source type='unix' path='/tmp/vhost2.sock' mode='client'/
   model type='virtio'/
   driver queues='4'/
 /interface

 Signed-off-by: Maxime Leroy maxime.le...@6wind.com
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  docs/formatdomain.html.in | 11 +--
  src/qemu/qemu_command.c   | 15 
 ++-
  ...stuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
  ...hostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
  tests/qemuxml2argvtest.c  |  3 +++
  5 files changed, 37 insertions(+), 4 deletions(-)
  copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
 qemuxml2argv-net-vhostuser-multiq.args} (75%)
  copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
 qemuxml2argv-net-vhostuser-multiq.xml} (87%)

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 72ad54cee188..85238a16af8d 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -4260,7 +4260,8 @@ qemu-kvm -net nic,model=? /dev/null
  type='virtio'/gt;/code, multiple packet processing queues can be
  created; each queue will potentially be handled by a different
  processor, resulting in much higher throughput.
 -span class=sinceSince 1.0.6 (QEMU and KVM only)/span
 +span class=sinceSince 1.0.6 (QEMU and KVM only) and for 
 vhost-user
 +  since 1.2.17/span
/dd
dtcodehost/code offloading options/dt
dd
 @@ -4581,9 +4582,15 @@ qemu-kvm -net nic,model=? /dev/null
lt;devicesgt;
  lt;interface type='vhostuser'gt;
lt;mac address='52:54:00:3b:83:1a'/gt;
 -  lt;source type='unix' path='/tmp/vhost.sock' mode='server'/gt;
 +  lt;source type='unix' path='/tmp/vhost1.sock' mode='server'/gt;
lt;model type='virtio'/gt;
  lt;/interfacegt;
 +lt;interface type='vhostuser'gt;
 +  lt;mac address='52:54:00:3b:83:1b'/gt;
 +  lt;source type='unix' path='/tmp/vhost2.sock' mode='client'/gt;
 +  lt;model type='virtio'/gt;
 +  lt;driver queues='5'/gt;
 +lt;/interfacegt;
lt;/devicesgt;
.../pre

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 61faa576e11b..f805f6700e71 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -8089,6 +8089,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
  {
  virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
  virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
 +unsigned int queues = 1;

Why setting queues to 1 and not to net-driver.virtio.queues directly ?

  char *nic = NULL;

  if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
 @@ -8126,13 +8127,25 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
  virBufferAsprintf(netdev_buf, 
 type=vhost-user,id=host%s,chardev=char%s,
net-info.alias, net-info.alias);

 +queues = net-driver.virtio.queues;
 +if (queues) {

I know it's never set to 1 thanks to your patch: conf: Ignore
multiqueue with one queue.

Anyway I think we should check if queues is superior to 1 for
improving code readability.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/4] qemu: Add capability for vhost-user multiqueue

2015-06-04 Thread Martin Kletzander

On Thu, Jun 04, 2015 at 05:56:20PM +0200, Maxime Leroy wrote:

On Thu, Jun 4, 2015 at 3:43 PM, Martin Kletzander mklet...@redhat.com wrote:

The support for this was added in QEMU with commit
830d70db692e374b5f4407f96a1ceefdcc97.  Unfortunately we have to do
another ugly version-based capability check.  The other option would be
not to check for the capability at all and leave that to qemu as it's
doen with multiqueue tap devices.


typo: doen -- done



good catch :)  I'll repost the series just so it's cleaner for other
reviewers.  Thanks for having a look at it.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 3/4] qemu: Add capability for vhost-user multiqueue

2015-06-04 Thread Martin Kletzander
The support for this was added in QEMU with commit
830d70db692e374b5f4407f96a1ceefdcc97.  Unfortunately we have to do
another ugly version-based capability check.  The other option would be
not to check for the capability at all and leave that to qemu as it's
doen with multiqueue tap devices.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_capabilities.c | 6 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 960afa4ac0db..f102ed80f15e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -284,6 +284,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   aes-key-wrap,
   dea-key-wrap,
   pci-serial,
+  vhost-user-multiq,
 );


@@ -3283,6 +3284,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (qemuCaps-version = 2002000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);

+/* vhost-user supports multi-queue from v2.4.0 onwards,
+ * but there is no way to query for that capability */
+if (qemuCaps-version = 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ);
+
 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon)  0)
 goto cleanup;
 if (virQEMUCapsProbeQMPEvents(qemuCaps, mon)  0)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9c956f3007be..3dbd767f2516 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -228,6 +228,7 @@ typedef enum {
 QEMU_CAPS_AES_KEY_WRAP   = 186, /* -machine aes_key_wrap */
 QEMU_CAPS_DEA_KEY_WRAP   = 187, /* -machine dea_key_wrap */
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
+QEMU_CAPS_VHOSTUSER_MULTIQ   = 189, /* vhost-user with -netdev queues= */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 1/4] conf: Ignore multiqueue with one queue.

2015-06-04 Thread Martin Kletzander
Multi != One.  And indeed, libvirt behaves the same way for queues='1'
as without such setting.  Let's make it clear in the XML.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/conf/domain_conf.c  | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 6 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 6 ++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 36de8441990e..2e7961001090 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8626,7 +8626,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
queues);
 goto error;
 }
-def-driver.virtio.queues = q;
+if (q  1)
+def-driver.virtio.queues = q;
 }
 if ((str = virXPathString(string(./driver/host/@csum), ctxt))) {
 if ((val = virTristateSwitchTypeFromString(str)) = 0) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
index 2cf312f0ca53..28f93474136e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
@@ -28,6 +28,12 @@
   driver name='vhost' queues='5'/
   backend tap='/dev/null' vhost='/dev/zero'/
 /interface
+interface type='user'
+  mac address='52:54:00:e5:48:59'/
+  model type='virtio'/
+  driver name='vhost' queues='1'/
+  backend tap='/dev/null' vhost='/dev/zero'/
+/interface
 serial type='pty'
   target port='0'/
 /serial
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
index 266cbf0a72b8..d419cc3b8e15 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
@@ -27,6 +27,12 @@
   model type='definitely-not-virtio'/
   backend tap='/dev/null'/
 /interface
+interface type='user'
+  mac address='52:54:00:e5:48:59'/
+  model type='virtio'/
+  driver name='vhost'/
+  backend tap='/dev/null' vhost='/dev/zero'/
+/interface
 serial type='pty'
   target port='0'/
 /serial
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/4] qemu: Add capability for vhost-user multiqueue

2015-06-04 Thread Maxime Leroy
On Thu, Jun 4, 2015 at 3:43 PM, Martin Kletzander mklet...@redhat.com wrote:
 The support for this was added in QEMU with commit
 830d70db692e374b5f4407f96a1ceefdcc97.  Unfortunately we have to do
 another ugly version-based capability check.  The other option would be
 not to check for the capability at all and leave that to qemu as it's
 doen with multiqueue tap devices.

typo: doen -- done

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 0/4] Add support for vhost-user with multi-queue

2015-06-04 Thread Maxime Leroy
Hi Martin,

On Thu, Jun 4, 2015 at 3:43 PM, Martin Kletzander mklet...@redhat.com wrote:
 Also some tiny clean-up.

 Martin Kletzander (2):
   conf: Ignore multiqueue with one queue.
   qemu: Add capability for vhost-user multiqueue

 Maxime Leroy (2):
   docs: Clarify that attribute name is not used for vhostuser
   qemu: add multiqueue vhost-user support

  docs/formatdomain.html.in| 16 
 ++--
  src/conf/domain_conf.c   |  3 ++-
  src/qemu/qemu_capabilities.c |  6 ++
  src/qemu/qemu_capabilities.h |  1 +
  src/qemu/qemu_command.c  | 15 ++-
  ...tuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
  ...ostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
  .../qemuxml2argv-tap-vhost-incorrect.xml |  6 ++
  tests/qemuxml2argvtest.c |  3 +++
  .../qemuxml2xmlout-tap-vhost-incorrect.xml   |  6 ++
  10 files changed, 63 insertions(+), 5 deletions(-)
  copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
 qemuxml2argv-net-vhostuser-multiq.args} (75%)
  copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
 qemuxml2argv-net-vhostuser-multiq.xml} (87%)

 --
 2.4.2

Thanks to send theses patches on the mailing list. Except few
comments, it looks good for me.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr

2015-06-04 Thread Matthias Gatto
On Tue, May 12, 2015 at 5:38 PM, Peter Krempa pkre...@redhat.com wrote:
 On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote:
 Allow to libvirt to build the quorum string used by quemu.

 Add 2 static functions: qemuBuildQuorumStr and
 qemuBuildAndAppendDriveStrToVirBuffer.
 qemuBuildQuorumStr is made because a quorum can have another quorum
 as a child, so we may need to call qemuBuildQuorumStr recursively.

 qemuBuildQuorumFileSourceStr was basically made to share
 the code use to build the source between qemuBuildQuorumStr and
 qemuBuildDriveStr, but there is some difference betwin the syntax
 use by libvirt to declare a disk and the one qemu need to build a quorum:
 a quorum need a syntaxe like:
 domaine_name.children.X.file.filename=filename
 where libvirt don't use file.filename= but directly file=.
 Therfore I use this function only for quorum.

 But as explained in the cover letter and here:
 http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
 We miss some informations in _virStorageSource to have a complet
 quorum support in libvirt.
 Ideally I'd like to refactore virDomainDiskDefFormat to allow
 qemuBuildQuorumStr to call this function in a loop.

 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/qemu/qemu_command.c | 110 
 
  1 file changed, 110 insertions(+)


 ...

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 6c40d3e..80cbb7d 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
  return -1;
  }

 +static bool
 +qemuBuildQuorumFileSourceStr(virConnectPtr conn,
 +  virStorageSourcePtr src,
 +  virBuffer *opt,
 +  const char *toAppend)
 +{
 +char *source = NULL;
 +int actualType = virStorageSourceGetActualType(src);
 +
 +if (qemuGetDriveSourceString(src, conn, source)  0)
 +goto error;
 +
 +if (source) {
 +
 +virBufferAsprintf(opt, ,%sfilename=, toAppend);

 virBufferStrcat

 +
 +switch (actualType) {
 +case VIR_STORAGE_TYPE_DIR:

 I'd forbid the DIR type altogether with quorums.

 +/* QEMU only supports magic FAT format for now */
 +if (src-format  0 
 +src-format != VIR_STORAGE_FILE_FAT) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unsupported disk driver type for '%s'),
 +   
 virStorageFileFormatTypeToString(src-format));
 +goto error;
 +}
 +
 +if (!src-readonly) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot create virtual FAT disks in 
 read-write mode));
 +goto error;
 +}
 +
 +virBufferAddLit(opt, fat:);
 +
 +break;
 +
 +default:
 +break;
 +}
 +virBufferAsprintf(opt, %s, source);

 virBufferAdd

 +}
 +
 +return true;
 + error:
 +return false;

 Error can be returned right away since there is nothing to clean up.

 +}
 +
 +
 +static bool
 +qemuBuildQuorumStr(virConnectPtr conn,
 +   virDomainDiskDefPtr disk,
 +   virStorageSourcePtr src,
 +   virBuffer *opt,
 +   const char *toAppend)
 +{
 +char *tmp = NULL;
 +int ret;
 +virStorageSourcePtr backingStore;
 +size_t i;
 +
 +if (!src-threshold) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold missing in the quorum configuration));
 +return false;
 +}
 +if (src-nBackingStores  2) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(a quorum must have at last 2 children));
 +return false;
 +}
 +if (src-threshold  src-nBackingStores) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold must not exceed the number of 
 childrens));

 'children' is the proper plural

 +return false;
 +}
 +virBufferAsprintf(opt, ,%svote-threshold=%lu,
 +  toAppend, src-threshold);
 +for (i = 0;  i  src-nBackingStores; ++i) {
 +backingStore = virStorageSourceGetBackingStore(src, i);
 +ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i);
 +if (ret  0)
 +return false;
 +
 +virBufferAsprintf(opt, ,%schildren.%lu.driver=%s,
 +  toAppend, i,
 +  
 virStorageFileFormatTypeToString(backingStore-format));
 +
 +if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == 
 false)
 +goto error;
 +
 +/* This operation avoid me to made another copy */
 +tmp[ret - sizeof(file)] = '\0';
 +if 

[libvirt] [PATCH 04/10] Add endjob label to qemuDomainMemoryStats

2015-06-04 Thread Ján Tomko
Reduce the indentation level.
---
 src/qemu/qemu_driver.c | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e031a17..818862b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11577,6 +11577,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
   unsigned int flags)
 {
 virQEMUDriverPtr driver = dom-conn-privateData;
+qemuDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 int ret = -1;
 
@@ -11594,27 +11595,29 @@ qemuDomainMemoryStats(virDomainPtr dom,
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
-} else {
-qemuDomainObjPrivatePtr priv = vm-privateData;
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorGetMemoryStats(priv-mon, stats, nr_stats);
-if (qemuDomainObjExitMonitor(driver, vm)  0)
-ret = -1;
+goto endjob;
+}
 
-if (ret = 0  ret  nr_stats) {
-long rss;
-if (qemuGetProcessInfo(NULL, NULL, rss, vm-pid, 0)  0) {
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
-   _(cannot get RSS for domain));
-} else {
-stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
-stats[ret].val = rss;
-ret++;
-}
+priv = vm-privateData;
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorGetMemoryStats(priv-mon, stats, nr_stats);
+if (qemuDomainObjExitMonitor(driver, vm)  0)
+ret = -1;
 
+if (ret = 0  ret  nr_stats) {
+long rss;
+if (qemuGetProcessInfo(NULL, NULL, rss, vm-pid, 0)  0) {
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+   _(cannot get RSS for domain));
+} else {
+stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
+stats[ret].val = rss;
+ret++;
 }
+
 }
 
+ endjob:
 qemuDomainObjEndJob(driver, vm);
 
  cleanup:
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] qemu: add multiqueue vhost-user support

2015-06-04 Thread Martin Kletzander

On Thu, Jun 04, 2015 at 03:43:54PM +0200, Martin Kletzander wrote:

From: Maxime Leroy maxime.le...@6wind.com

This patch adds the support of queues attribute of the driver element
for vhost-user interface type. Example:

interface type='vhostuser'
 mac address='52:54:00:ee:96:6d'/
 source type='unix' path='/tmp/vhost2.sock' mode='client'/
 model type='virtio'/
 driver queues='4'/
/interface



I forgot to mention that this patch is for the following BZ:

https://bugzilla.redhat.com/show_bug.cgi?id=1207692


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/10] Only call qemuMonitorGetMemoryStats for virtio memballoon

2015-06-04 Thread Ján Tomko
There is nothing to get from the monitor for model='none'.
---
 src/qemu/qemu_driver.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 50eebf9..4690406 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11599,14 +11599,19 @@ qemuDomainMemoryStats(virDomainPtr dom,
 goto endjob;
 }
 
-priv = vm-privateData;
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorGetMemoryStats(priv-mon, stats, nr_stats);
-if (qemuDomainObjExitMonitor(driver, vm)  0)
-ret = -1;
+if (vm-def-memballoon 
+vm-def-memballoon-model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+priv = vm-privateData;
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorGetMemoryStats(priv-mon, stats, nr_stats);
+if (qemuDomainObjExitMonitor(driver, vm)  0)
+ret = -1;
 
-if (ret  0 || ret = nr_stats)
-goto endjob;
+if (ret  0 || ret = nr_stats)
+goto endjob;
+} else {
+ret = 0;
+}
 
 if (qemuGetProcessInfo(NULL, NULL, rss, vm-pid, 0)  0) {
 virReportError(VIR_ERR_OPERATION_FAILED, %s,
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] maint: document use of zanata for translations

2015-06-04 Thread Eric Blake
ping

On 05/27/2015 08:43 AM, Eric Blake wrote:
 Based on recent list questions on how to contribute a translation fix.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 
 Should be safe for freeze, but as I have never contributed a
 translation fix, I'll wait for review.
 
  HACKING  | 19 ---
  docs/hacking.html.in |  7 +++
  2 files changed, 19 insertions(+), 7 deletions(-)
 
 diff --git a/HACKING b/HACKING
 index fbe838b..e308568 100644
 --- a/HACKING
 +++ b/HACKING
 @@ -18,7 +18,12 @@ listen to feedback.
  and is browsable along with other libvirt-related repositories (e.g.
  libvirt-python) online http://libvirt.org/git/.
 
 -(3) Post patches in unified diff format, with git rename detection enabled. 
 You
 +(3) Patches to translations are maintained via the zanata project
 +https://fedora.zanata.org/. If you want to fix a translation in a .po file,
 +join the appropriate language team. The libvirt release process automatically
 +pulls the latest version of each translation file from zanata.
 +
 +(4) Post patches in unified diff format, with git rename detection enabled. 
 You
  need a one-time setup of:
 
git config diff.renames true
 @@ -70,7 +75,7 @@ the correct version if needed though).
 
 
 
 -(4) In your commit message, make the summary line reasonably short (60 
 characters
 +(5) In your commit message, make the summary line reasonably short (60 
 characters
  is typical), followed by a blank line, followed by any longer description of
  why your patch makes sense. If the patch fixes a regression, and you know 
 what
  commit introduced the problem, mentioning that is useful. If the patch
 @@ -82,7 +87,7 @@ is up to you if you want to include or omit them in the 
 commit message.
 
 
 
 -(5) Split large changes into a series of smaller patches, self-contained if
 +(6) Split large changes into a series of smaller patches, self-contained if
  possible, with an explanation of each patch and an explanation of how the
  sequence of patches fits together. Moreover, please keep in mind that it's
  required to be able to compile cleanly (*including* make check and make
 @@ -93,10 +98,10 @@ things).
 
 
 
 -(6) Make sure your patches apply against libvirt GIT. Developers only follow 
 GIT
 +(7) Make sure your patches apply against libvirt GIT. Developers only follow 
 GIT
  and don't care much about released versions.
 
 -(7) Run the automated tests on your code before submitting any changes. In
 +(8) Run the automated tests on your code before submitting any changes. In
  particular, configure with compile warnings set to -Werror. This is done
  automatically for a git checkout; from a tarball, use:
 
 @@ -149,7 +154,7 @@ various tests under gdb or Valgrind.
 
 
 
 -(8) The Valgrind test should produce similar output to make check. If the 
 output
 +(9) The Valgrind test should produce similar output to make check. If the 
 output
  has traces within libvirt API's, then investigation is required in order to
  determine the cause of the issue. Output such as the following indicates some
  sort of leak:
 @@ -225,7 +230,7 @@ to tests/.valgrind.supp in order to suppress the 
 warning:
 
 
 
 -(9) Update tests and/or documentation, particularly if you are adding a new
 +(10) Update tests and/or documentation, particularly if you are adding a new
  feature or changing the output of a program.
 
 
 diff --git a/docs/hacking.html.in b/docs/hacking.html.in
 index 408ea50..5cd23a2 100644
 --- a/docs/hacking.html.in
 +++ b/docs/hacking.html.in
 @@ -16,6 +16,13 @@
  along with other libvirt-related repositories
  (e.g. libvirt-python) a 
 href=http://libvirt.org/git/;online/a./li
 
 +  liPatches to translations are maintained via
 +the a href=https://fedora.zanata.org/;zanata project/a.
 +If you want to fix a translation in a .po file, join the
 +appropriate language team. The libvirt release process
 +automatically pulls the latest version of each translation
 +file from zanata./li
 +
lipPost patches in unified diff format, with git rename
  detection enabled.  You need a one-time setup of:/p
  pre
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3] qemu: fix unsuitable error report when get memory stats

2015-06-04 Thread Ján Tomko
On Wed, Jun 03, 2015 at 09:07:49AM +0800, Wang Yufei wrote:
 From: Zhang Bo oscar.zhan...@huawei.com
 
 when we run the command 'virsh dommemstat xxx',
 althrough memballoon's model is set 'none' in vm's XML,
 it still reports an error in libvirtd.log.
 error : qemuMonitorFindBalloonObjectPath:1042 : internal error: Cannot 
 determine balloon device path
 Apparently, if we don't set memballoon, we don't need to
 set balloon device path.

We shouldn't even be calling qemuMonitorFindBalloonObjectPath if there
is no balloon device.

I have sent a patch that moves the check to qemuDomainMemoryStats as a
part of a larger series:
https://www.redhat.com/archives/libvir-list/2015-June/msg00213.html

Jan

 Signed-off-by: Wang Yufei james.wangyu...@huawei.com
 Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
 ---
  src/qemu/qemu_monitor.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 09/10] Do not access the domain definition in qemuMonitorFindBalloonObjectPath

2015-06-04 Thread Ján Tomko
The monitor code does not hold the virDomainObjPtr lock and should
not access the defitinion.
---
 src/qemu/qemu_monitor.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9add05c..6947b08 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1091,7 +1091,6 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
 int ret = -1;
 char *path = NULL;
 qemuMonitorJSONListPathPtr *bprops = NULL;
-virDomainObjPtr vm = mon-vm;
 
 if (mon-balloonpath) {
 return 0;
@@ -1101,15 +1100,6 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
 return -1;
 }
 
-/* Not supported */
-if (!vm-def-memballoon ||
-vm-def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(Memory balloon model must be virtio to 
- get memballoon path));
-return -1;
-}
-
 if (qemuMonitorJSONFindLinkPath(mon, virtio-balloon-pci, path)  0)
 return -1;
 
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 03/10] Remove path argument from qemuMonitorJSONFindLinkPath

2015-06-04 Thread Ján Tomko
All the callers use / anyway.
---
 src/qemu/qemu_monitor.c  | 11 +--
 src/qemu/qemu_monitor_json.c |  3 +--
 src/qemu/qemu_monitor_json.h |  1 -
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4a5e13c..9add05c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1085,8 +1085,7 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr 
options)
  * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
  */
 static int
-qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
- const char *curpath)
+qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
 {
 ssize_t i, nprops = 0;
 int ret = -1;
@@ -,7 +1110,7 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 return -1;
 }
 
-if (qemuMonitorJSONFindLinkPath(mon, curpath, virtio-balloon-pci, path) 
 0)
+if (qemuMonitorJSONFindLinkPath(mon, virtio-balloon-pci, path)  0)
 return -1;
 
 nprops = qemuMonitorJSONGetObjectListPaths(mon, path, bprops);
@@ -1160,7 +1159,7 @@ qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon,
 QEMU_CHECK_MONITOR(mon);
 
 if (mon-json) {
-ret = qemuMonitorJSONFindLinkPath(mon, /, videoName, path);
+ret = qemuMonitorJSONFindLinkPath(mon, videoName, path);
 if (ret  0) {
 if (ret == -2)
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1643,7 +1642,7 @@ qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
 QEMU_CHECK_MONITOR(mon);
 
 if (mon-json) {
-ignore_value(qemuMonitorFindBalloonObjectPath(mon, /));
+ignore_value(qemuMonitorFindBalloonObjectPath(mon));
 mon-ballooninit = true;
 return qemuMonitorJSONGetMemoryStats(mon, mon-balloonpath,
  stats, nr_stats);
@@ -1676,7 +1675,7 @@ qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
 if (period  0)
 return -1;
 
-if (qemuMonitorFindBalloonObjectPath(mon, /) == 0) {
+if (qemuMonitorFindBalloonObjectPath(mon) == 0) {
 ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon-balloonpath,
   period);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6fafe81..13c57d2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6721,7 +6721,6 @@ qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
  */
 int
 qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon,
-const char *curpath,
 const char *name,
 char **path)
 {
@@ -6731,7 +6730,7 @@ qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon,
 if (virAsprintf(linkname, link%s, name)  0)
 return -1;
 
-ret = qemuMonitorJSONFindObjectPath(mon, curpath, linkname, path);
+ret = qemuMonitorJSONFindObjectPath(mon, /, linkname, path);
 VIR_FREE(linkname);
 return ret;
 }
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 953266c..ae8ef7c 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -483,7 +483,6 @@ int qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon,
-const char *curpath,
 const char *name,
 char **path);
 #endif /* QEMU_MONITOR_JSON_H */
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 01/10] Move qemuMonitorFindObjectPath to qemu_monitor_json

2015-06-04 Thread Ján Tomko
This function is specific to the JSON monitor.
---
 src/qemu/qemu_monitor.c  | 76 ++--
 src/qemu/qemu_monitor_json.c | 72 +
 src/qemu/qemu_monitor_json.h |  4 +++
 3 files changed, 78 insertions(+), 74 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e9c57f1..d761f51 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1069,78 +1069,6 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
virJSONValuePtr options)
 
 
 /**
- * Search the qom objects by it's known name.  The name is compared against
- * filed 'type' formatted as 'link%name'.
- *
- * This procedure will be call recursively until found or the qom-list is
- * exhausted.
- *
- * Returns:
- *
- *   0  - Found
- *  -1  - Error bail out
- *  -2  - Not found
- *
- * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
- */
-static int
-qemuMonitorFindObjectPath(qemuMonitorPtr mon,
-  const char *curpath,
-  const char *name,
-  char **path)
-{
-ssize_t i, npaths = 0;
-int ret = -2;
-char *nextpath = NULL;
-char *type = NULL;
-qemuMonitorJSONListPathPtr *paths = NULL;
-
-if (virAsprintf(type, link%s, name)  0)
-return -1;
-
-VIR_DEBUG(Searching for '%s' Object Path starting at '%s', type, 
curpath);
-
-npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
-if (npaths  0)
-goto cleanup;
-
-for (i = 0; i  npaths  ret == -2; i++) {
-
-if (STREQ_NULLABLE(paths[i]-type, type)) {
-VIR_DEBUG(Path to '%s' is '%s/%s', type, curpath, 
paths[i]-name);
-ret = 0;
-if (virAsprintf(path, %s/%s, curpath, paths[i]-name)  0) {
-*path = NULL;
-ret = -1;
-}
-goto cleanup;
-}
-
-/* Type entries that begin with child are a branch that can be
- * traversed looking for more entries
- */
-if (paths[i]-type  STRPREFIX(paths[i]-type, child)) {
-if (virAsprintf(nextpath, %s/%s, curpath, paths[i]-name)  0) {
-ret = -1;
-goto cleanup;
-}
-
-ret = qemuMonitorFindObjectPath(mon, nextpath, name, path);
-VIR_FREE(nextpath);
-}
-}
-
- cleanup:
-for (i = 0; i  npaths; i++)
-qemuMonitorJSONListPathFree(paths[i]);
-VIR_FREE(paths);
-VIR_FREE(nextpath);
-VIR_FREE(type);
-return ret;
-}
-
-
-/**
  * Search the qom objects for the balloon driver object by it's known name
  * of virtio-balloon-pci.  The entry for the driver will be found by using
  * function qemuMonitorFindObjectPath.
@@ -1183,7 +,7 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 return -1;
 }
 
-if (qemuMonitorFindObjectPath(mon, curpath, virtio-balloon-pci, path)  
0)
+if (qemuMonitorJSONFindObjectPath(mon, curpath, virtio-balloon-pci, 
path)  0)
 return -1;
 
 nprops = qemuMonitorJSONGetObjectListPaths(mon, path, bprops);
@@ -1232,7 +1160,7 @@ qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon,
 QEMU_CHECK_MONITOR(mon);
 
 if (mon-json) {
-ret = qemuMonitorFindObjectPath(mon, /, videoName, path);
+ret = qemuMonitorJSONFindObjectPath(mon, /, videoName, path);
 if (ret  0) {
 if (ret == -2)
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e6da804..69c342d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6642,3 +6642,75 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
 virJSONValueFree(reply);
 return ret;
 }
+
+
+/**
+ * Search the qom objects by it's known name.  The name is compared against
+ * filed 'type' formatted as 'link%name'.
+ *
+ * This procedure will be call recursively until found or the qom-list is
+ * exhausted.
+ *
+ * Returns:
+ *
+ *   0  - Found
+ *  -1  - Error bail out
+ *  -2  - Not found
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+int
+qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
+  const char *curpath,
+  const char *name,
+  char **path)
+{
+ssize_t i, npaths = 0;
+int ret = -2;
+char *nextpath = NULL;
+char *type = NULL;
+qemuMonitorJSONListPathPtr *paths = NULL;
+
+if (virAsprintf(type, link%s, name)  0)
+return -1;
+
+VIR_DEBUG(Searching for '%s' Object Path starting at '%s', type, 
curpath);
+
+npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
+if (npaths  0)
+goto cleanup;
+
+for (i = 0; i  npaths  ret == -2; i++) {
+
+if (STREQ_NULLABLE(paths[i]-type, type)) {
+VIR_DEBUG(Path to '%s' is '%s/%s', type, curpath, 

[libvirt] [PATCH] parallels: implement attach/detach network.

2015-06-04 Thread Mikhail Feoktistov
Support nova commands interface-attach and interface-detach.
For containers only.
---
 src/parallels/parallels_driver.c |   16 
 src/parallels/parallels_sdk.c|  144 +-
 src/parallels/parallels_sdk.h|4 +
 3 files changed, 161 insertions(+), 3 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index e385f3d..a49a030 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1050,6 +1050,14 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr 
dom, const char *xml,
 goto cleanup;
 }
 break;
+case VIR_DOMAIN_DEVICE_NET:
+ret = prlsdkAttachNet(privdom, privconn, dev-data.net);
+if (ret) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(network attach failed));
+goto cleanup;
+}
+break;
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(device type '%s' cannot be attached),
@@ -1133,6 +1141,14 @@ static int parallelsDomainDetachDeviceFlags(virDomainPtr 
dom, const char *xml,
 goto cleanup;
 }
 break;
+case VIR_DOMAIN_DEVICE_NET:
+ret = prlsdkDetachNet(privdom, privconn, dev-data.net);
+if (ret) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(network detach failed));
+goto cleanup;
+}
+break;
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(device type '%s' cannot be detached),
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 6e293f7..a1c9131 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2751,6 +2751,12 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
 pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
 prlsdkCheckRetGoto(pret, cleanup);
 
+pret = PrlVmDevNet_SetConfigureWithDhcp(sdknet, true);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetAutoApply(sdknet, true);
+prlsdkCheckRetGoto(pret, cleanup);
+
 if (isCt) {
 if (net-model)
  VIR_WARN(Setting network adapter for containers is not 
@@ -2821,14 +2827,15 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
 return ret;
 }
 
-static void prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net)
+static int prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net)
 {
+int ret = -1;
 PRL_RESULT pret;
 PRL_HANDLE vnet = PRL_INVALID_HANDLE;
 PRL_HANDLE job = PRL_INVALID_HANDLE;
 
 if (net-type != VIR_DOMAIN_NET_TYPE_BRIDGE)
-return;
+return 0;
 
 pret = PrlVirtNet_Create(vnet);
 prlsdkCheckRetGoto(pret, cleanup);
@@ -2836,12 +2843,142 @@ static void prlsdkDelNet(parallelsConnPtr privconn, 
virDomainNetDefPtr net)
 pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name);
 prlsdkCheckRetGoto(pret, cleanup);
 
-PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);
+job = PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);
 if (PRL_FAILED(pret = waitJob(job)))
 goto cleanup;
 
+ret = 0;
+
  cleanup:
 PrlHandle_Free(vnet);
+return ret;
+}
+
+int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, 
virDomainNetDefPtr net)
+{
+int ret = -1;
+parallelsDomObjPtr privdom = dom-privateData;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+
+if (!IS_CT(dom-def)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(network device cannot be attached));
+goto cleanup;
+}
+
+job = PrlVm_BeginEdit(privdom-sdkdom);
+if (PRL_FAILED(waitJob(job)))
+goto cleanup;
+
+ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def));
+if (ret == 0) {
+job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE);
+if (PRL_FAILED(waitJob(job))) {
+ret = -1;
+goto cleanup;
+}
+}
+
+ cleanup:
+return ret;
+}
+
+static int
+prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net)
+{
+int idx = -1;
+PRL_RESULT pret;
+PRL_UINT32 netCount;
+PRL_UINT32 i;
+PRL_HANDLE adapter = PRL_INVALID_HANDLE;
+PRL_UINT32 len;
+char adapterMac[PRL_MAC_STRING_BUFNAME];
+char netMac[PRL_MAC_STRING_BUFNAME];
+
+prlsdkFormatMac(net-mac, netMac);
+pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, netCount);
+prlsdkCheckRetGoto(pret, cleanup);
+
+for (i = 0; i  netCount; ++i) {
+
+pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter);
+prlsdkCheckRetGoto(pret, cleanup);
+
+len = sizeof(adapterMac);
+memset(adapterMac, 0, sizeof(adapterMac));
+pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len);
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (memcmp(adapterMac, netMac, PRL_MAC_STRING_BUFNAME)) {
+
+  

[libvirt] [PATCH 10/10] Turn qemuMonitorFindBalloonObjectPath into a void function

2015-06-04 Thread Ján Tomko
We were efectively ignoring its errors anyway.
---
 src/qemu/qemu_monitor.c | 29 ++---
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6947b08..33600f0 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1076,32 +1076,25 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
virJSONValuePtr options)
  * Once found, check the entry to ensure it has the correct property listed.
  * If it does not, then obtaining statistics from QEMU will not be possible.
  * This feature was added to QEMU 1.5.
- *
- * Returns:
- *
- *   0  - Found
- *  -1  - Not found or error
- *
- * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
  */
-static int
-qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
+static void
+qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon)
 {
 ssize_t i, nprops = 0;
-int ret = -1;
 char *path = NULL;
 qemuMonitorJSONListPathPtr *bprops = NULL;
 
 if (mon-balloonpath) {
-return 0;
+return;
 } else if (mon-ballooninit) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot determine balloon device path));
-return -1;
+return;
 }
+mon-ballooninit = true;
 
 if (qemuMonitorJSONFindLinkPath(mon, virtio-balloon-pci, path)  0)
-return -1;
+return;
 
 nprops = qemuMonitorJSONGetObjectListPaths(mon, path, bprops);
 if (nprops  0)
@@ -1112,7 +1105,6 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
 VIR_DEBUG(Found Balloon Object Path %s, path);
 mon-balloonpath = path;
 path = NULL;
-ret = 0;
 goto cleanup;
 }
 }
@@ -1128,7 +1120,7 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon)
 qemuMonitorJSONListPathFree(bprops[i]);
 VIR_FREE(bprops);
 VIR_FREE(path);
-return ret;
+return;
 }
 
 
@@ -1632,8 +1624,7 @@ qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
 QEMU_CHECK_MONITOR(mon);
 
 if (mon-json) {
-ignore_value(qemuMonitorFindBalloonObjectPath(mon));
-mon-ballooninit = true;
+qemuMonitorInitBalloonObjectPath(mon);
 return qemuMonitorJSONGetMemoryStats(mon, mon-balloonpath,
  stats, nr_stats);
 } else {
@@ -1665,7 +1656,8 @@ qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
 if (period  0)
 return -1;
 
-if (qemuMonitorFindBalloonObjectPath(mon) == 0) {
+qemuMonitorInitBalloonObjectPath(mon);
+if (mon-balloonpath) {
 ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon-balloonpath,
   period);
 
@@ -1678,7 +1670,6 @@ qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
 if (ret  0)
 virResetLastError();
 }
-mon-ballooninit = true;
 return ret;
 }
 
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 08/10] Only call SetMemoryStatsPeriod for virtio memballoon

2015-06-04 Thread Ján Tomko
---
 src/qemu/qemu_process.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7f154f0..64ee049 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5517,7 +5517,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
 if (running) {
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
  VIR_DOMAIN_RUNNING_UNPAUSED);
-if (vm-def-memballoon  vm-def-memballoon-period) {
+if (vm-def-memballoon 
+vm-def-memballoon-model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO 
+vm-def-memballoon-period) {
 qemuDomainObjEnterMonitor(driver, vm);
 qemuMonitorSetMemoryStatsPeriod(priv-mon,
 vm-def-memballoon-period);
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 05/10] Invert the condition in qemuDomainMemoryStats

2015-06-04 Thread Ján Tomko
It only makes sense if qemuMonitorGetMemoryStats is called,
but the following patch will make that call conditional.
---
 src/qemu/qemu_driver.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 818862b..50eebf9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11580,6 +11580,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
 qemuDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 int ret = -1;
+long rss;
 
 virCheckFlags(0, -1);
 
@@ -11604,17 +11605,16 @@ qemuDomainMemoryStats(virDomainPtr dom,
 if (qemuDomainObjExitMonitor(driver, vm)  0)
 ret = -1;
 
-if (ret = 0  ret  nr_stats) {
-long rss;
-if (qemuGetProcessInfo(NULL, NULL, rss, vm-pid, 0)  0) {
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
-   _(cannot get RSS for domain));
-} else {
-stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
-stats[ret].val = rss;
-ret++;
-}
+if (ret  0 || ret = nr_stats)
+goto endjob;
 
+if (qemuGetProcessInfo(NULL, NULL, rss, vm-pid, 0)  0) {
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+   _(cannot get RSS for domain));
+} else {
+stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
+stats[ret].val = rss;
+ret++;
 }
 
  endjob:
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 02/10] Introduce qemuMonitorJSONFindLinkPath

2015-06-04 Thread Ján Tomko
When traversing through the QOM tree, we're looking for
a link to a device, e.g.:
linkvirtio-balloon-pci

Introduce a helper that will format the link name at the start,
instead of doing it every time while recursing through the tree.
---
 src/qemu/qemu_monitor.c  |  4 ++--
 src/qemu/qemu_monitor_json.c | 55 ++--
 src/qemu/qemu_monitor_json.h |  8 +++
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d761f51..4a5e13c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -,7 +,7 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 return -1;
 }
 
-if (qemuMonitorJSONFindObjectPath(mon, curpath, virtio-balloon-pci, 
path)  0)
+if (qemuMonitorJSONFindLinkPath(mon, curpath, virtio-balloon-pci, path) 
 0)
 return -1;
 
 nprops = qemuMonitorJSONGetObjectListPaths(mon, path, bprops);
@@ -1160,7 +1160,7 @@ qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon,
 QEMU_CHECK_MONITOR(mon);
 
 if (mon-json) {
-ret = qemuMonitorJSONFindObjectPath(mon, /, videoName, path);
+ret = qemuMonitorJSONFindLinkPath(mon, /, videoName, path);
 if (ret  0) {
 if (ret == -2)
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 69c342d..6fafe81 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6645,21 +6645,18 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
 
 
 /**
- * Search the qom objects by it's known name.  The name is compared against
- * filed 'type' formatted as 'link%name'.
+ * Recursively search for a QOM object link.
  *
- * This procedure will be call recursively until found or the qom-list is
- * exhausted.
+ * For @name, this function finds the first QOM object
+ * named @name, recursively going through all the child
+ * entries, starting from @curpath.
  *
  * Returns:
- *
  *   0  - Found
- *  -1  - Error bail out
+ *  -1  - Error - bail out
  *  -2  - Not found
- *
- * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
  */
-int
+static int
 qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
   const char *curpath,
   const char *name,
@@ -6668,13 +6665,9 @@ qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
 ssize_t i, npaths = 0;
 int ret = -2;
 char *nextpath = NULL;
-char *type = NULL;
 qemuMonitorJSONListPathPtr *paths = NULL;
 
-if (virAsprintf(type, link%s, name)  0)
-return -1;
-
-VIR_DEBUG(Searching for '%s' Object Path starting at '%s', type, 
curpath);
+VIR_DEBUG(Searching for '%s' Object Path starting at '%s', name, 
curpath);
 
 npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
 if (npaths  0)
@@ -6682,8 +6675,8 @@ qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
 
 for (i = 0; i  npaths  ret == -2; i++) {
 
-if (STREQ_NULLABLE(paths[i]-type, type)) {
-VIR_DEBUG(Path to '%s' is '%s/%s', type, curpath, 
paths[i]-name);
+if (STREQ_NULLABLE(paths[i]-type, name)) {
+VIR_DEBUG(Path to '%s' is '%s/%s', name, curpath, 
paths[i]-name);
 ret = 0;
 if (virAsprintf(path, %s/%s, curpath, paths[i]-name)  0) {
 *path = NULL;
@@ -6711,6 +6704,34 @@ qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
 qemuMonitorJSONListPathFree(paths[i]);
 VIR_FREE(paths);
 VIR_FREE(nextpath);
-VIR_FREE(type);
+return ret;
+}
+
+
+/**
+ * Recursively search for a QOM object link.
+ *
+ * For @name, this function finds the first QOM object
+ * pointed to by a link in the form of 'link@name'
+ *
+ * Returns:
+ *   0  - Found
+ *  -1  - Error
+ *  -2  - Not found
+ */
+int
+qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon,
+const char *curpath,
+const char *name,
+char **path)
+{
+char *linkname = NULL;
+int ret = -1;
+
+if (virAsprintf(linkname, link%s, name)  0)
+return -1;
+
+ret = qemuMonitorJSONFindObjectPath(mon, curpath, linkname, path);
+VIR_FREE(linkname);
 return ret;
 }
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index fcc2c86..953266c 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -482,8 +482,8 @@ int qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
virHashTablePtr info)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int qemuMonitorJSONFindObjectPath(qemuMonitorPtr mon,
-  const char *curpath,
-  const char *name,
-  char **path);
+int qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon,
+const char 

[libvirt] [PATCH 00/10] qemu: balloon QOM-path related cleanups

2015-06-04 Thread Ján Tomko
While reviewing the patch adding virtio-balloon-ccw,
I found that we are not reporting errors consistently.
This turned out to be on purpose.

This series
* moves the object path search into qemu_monitor_json
* reduces the number of allocations during search
  (more a cosmetic chagne than optimization)
* moves the balloon model checking out of the monitor code
  * vm-def should not be accessed without a virDomainObj lock
  * this should also get rid of the errors on dommemstat with
'none' balloon reported by:
https://www.redhat.com/archives/libvir-list/2015-May/msg01110.html
* changes qemuMonitorFindBalloonObjectPath to void
  to make it obvious that errors are ignored

Ján Tomko (10):
  Move qemuMonitorFindObjectPath to qemu_monitor_json
  Introduce qemuMonitorJSONFindLinkPath
  Remove path argument from qemuMonitorJSONFindLinkPath
  Add endjob label to qemuDomainMemoryStats
  Invert the condition in qemuDomainMemoryStats
  Only call qemuMonitorGetMemoryStats for virtio memballoon
  Check for balloon model in qemuDomainSetMemoryStatsPeriod
  Only call SetMemoryStatsPeriod for virtio memballoon
  Do not access the domain definition in
qemuMonitorFindBalloonObjectPath
  Turn qemuMonitorFindBalloonObjectPath into a void function

 src/qemu/qemu_driver.c   |  49 +-
 src/qemu/qemu_monitor.c  | 116 +--
 src/qemu/qemu_monitor_json.c |  92 ++
 src/qemu/qemu_monitor_json.h |   3 ++
 src/qemu/qemu_process.c  |   4 +-
 5 files changed, 146 insertions(+), 118 deletions(-)

-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 07/10] Check for balloon model in qemuDomainSetMemoryStatsPeriod

2015-06-04 Thread Ján Tomko
There's no point in calling the monitor if there is no balloon.
---
 src/qemu/qemu_driver.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4690406..bfd59a9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2459,6 +2459,14 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr 
dom, int period,
 priv = vm-privateData;
 
 if (def) {
+if (!def-memballoon ||
+def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Memory balloon model must be virtio to set the
+  collection period));
+goto endjob;
+}
+
 qemuDomainObjEnterMonitor(driver, vm);
 r = qemuMonitorSetMemoryStatsPeriod(priv-mon, period);
 if (qemuDomainObjExitMonitor(driver, vm)  0)
@@ -2475,6 +2483,13 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr 
dom, int period,
 }
 
 if (persistentDef) {
+if (!def-memballoon ||
+def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Memory balloon model must be virtio to set the
+  collection period));
+goto endjob;
+}
 persistentDef-memballoon-period = period;
 ret = virDomainSaveConfig(cfg-configDir, persistentDef);
 goto endjob;
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

2015-06-04 Thread Peter Krempa
On Thu, Jun 04, 2015 at 08:34:00 -0400, John Ferlan wrote:
 
 
 On 06/04/2015 08:15 AM, Peter Krempa wrote:
  Refactor the code flow a bit more to clear coverity errors.
  
  Store the cpu count in an intermediate variable and reuse it rather than
  caluclating the index.
  ---
   src/util/virprocess.c | 11 +++
   1 file changed, 7 insertions(+), 4 deletions(-)
  
  diff --git a/src/util/virprocess.c b/src/util/virprocess.c
  index a38cb75..c07e5cd 100644
  --- a/src/util/virprocess.c
  +++ b/src/util/virprocess.c
  @@ -474,12 +474,14 @@ virProcessGetAffinity(pid_t pid)
   size_t i;
   cpu_set_t *mask;
   size_t masklen;
  +size_t ncpus;
   virBitmapPtr ret = NULL;
  
   # ifdef CPU_ALLOC
   /* 262144 cpus ought to be enough for anyone */
  -masklen = CPU_ALLOC_SIZE(1024  8);
  -mask = CPU_ALLOC(1024  8);
  +ncpus = 1024  8;
  +masklen = CPU_ALLOC_SIZE(ncpus);
  +mask = CPU_ALLOC(ncpus);
  
   if (!mask) {
   virReportOOMError();
  @@ -488,6 +490,7 @@ virProcessGetAffinity(pid_t pid)
  
   CPU_ZERO_S(masklen, mask);
   # else
  +ncpus = 1024;
   if (VIR_ALLOC(mask)  0)
   return NULL;
  
  @@ -501,10 +504,10 @@ virProcessGetAffinity(pid_t pid)
   goto cleanup;
   }
  
  -if (!(ret = virBitmapNew(masklen * 8)))
  +if (!(ret = virBitmapNew(ncpus)))
 goto cleanup;
  
  -for (i = 0; i  masklen * 8; i++) {
  +for (i = 0; i  ncpus; i++) {
   # ifdef CPU_ALLOC
   if (CPU_ISSET_S(i, masklen, mask))
 
 ^^^ Coverity still complains here
 
 No real change since previous...

Would you mind sharing the error after applying this patch?

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

2015-06-04 Thread John Ferlan


On 06/04/2015 11:13 AM, Peter Krempa wrote:
 On Thu, Jun 04, 2015 at 08:34:00 -0400, John Ferlan wrote:


 On 06/04/2015 08:15 AM, Peter Krempa wrote:
 Refactor the code flow a bit more to clear coverity errors.

 Store the cpu count in an intermediate variable and reuse it rather than
 caluclating the index.
 ---
  src/util/virprocess.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

 diff --git a/src/util/virprocess.c b/src/util/virprocess.c
 index a38cb75..c07e5cd 100644
 --- a/src/util/virprocess.c
 +++ b/src/util/virprocess.c
 @@ -474,12 +474,14 @@ virProcessGetAffinity(pid_t pid)
  size_t i;
  cpu_set_t *mask;
  size_t masklen;
 +size_t ncpus;
  virBitmapPtr ret = NULL;

  # ifdef CPU_ALLOC
  /* 262144 cpus ought to be enough for anyone */
 -masklen = CPU_ALLOC_SIZE(1024  8);
 -mask = CPU_ALLOC(1024  8);
 +ncpus = 1024  8;
 +masklen = CPU_ALLOC_SIZE(ncpus);
 +mask = CPU_ALLOC(ncpus);

  if (!mask) {
  virReportOOMError();
 @@ -488,6 +490,7 @@ virProcessGetAffinity(pid_t pid)

  CPU_ZERO_S(masklen, mask);
  # else
 +ncpus = 1024;
  if (VIR_ALLOC(mask)  0)
  return NULL;

 @@ -501,10 +504,10 @@ virProcessGetAffinity(pid_t pid)
  goto cleanup;
  }

 -if (!(ret = virBitmapNew(masklen * 8)))
 +if (!(ret = virBitmapNew(ncpus)))
goto cleanup;

 -for (i = 0; i  masklen * 8; i++) {
 +for (i = 0; i  ncpus; i++) {
  # ifdef CPU_ALLOC
  if (CPU_ISSET_S(i, masklen, mask))

 ^^^ Coverity still complains here

 No real change since previous...
 
 Would you mind sharing the error after applying this patch?
 
 Peter
 

Sure (it's cut-n-paste)

471 virBitmapPtr
472 virProcessGetAffinity(pid_t pid)
473 {
474 size_t i;
475 cpu_set_t *mask;
476 size_t masklen;
477 size_t ncpus;
478 virBitmapPtr ret = NULL;
479 
480 # ifdef CPU_ALLOC
481 /* 262144 cpus ought to be enough for anyone */

(1) Event assignment:   Assigning: ncpus = 262144UL.
Also see events:[cond_at_most][assignment][overrun-local]

482 ncpus = 1024  8;
483 masklen = CPU_ALLOC_SIZE(ncpus);
484 mask = CPU_ALLOC(ncpus);
485 

(2) Event cond_false:   Condition !mask, taking false branch

486 if (!mask) {
487 virReportOOMError();
488 return NULL;

(3) Event if_end:   End of if statement

489 }
490 
491 CPU_ZERO_S(masklen, mask);
492 # else
493 ncpus = 1024;
494 if (VIR_ALLOC(mask)  0)
495 return NULL;
496 
497 masklen = sizeof(*mask);
498 CPU_ZERO(mask);
499 # endif
500 

(4) Event cond_false:   Condition sched_getaffinity(pid, masklen, mask)  0, 
taking false branch

501 if (sched_getaffinity(pid, masklen, mask)  0) {
502 virReportSystemError(errno,
503  _(cannot get CPU affinity of process 
%d), pid);
504 goto cleanup;

(5) Event if_end:   End of if statement

505 }
506 

(6) Event cond_false:   Condition !(ret = virBitmapNew(ncpus)), taking false 
branch

507 if (!(ret = virBitmapNew(ncpus)))

(7) Event if_end:   End of if statement

508   goto cleanup;
509 

(8) Event cond_true:Condition i  ncpus, taking true branch
(13) Event loop_begin:  Jumped back to beginning of loop
(14) Event cond_true:   Condition i  ncpus, taking true branch
(15) Event cond_at_most:Checking i  ncpus implies that i may be up 
to 262143 on the true branch.
Also see events:[assignment][assignment][overrun-local]

510 for (i = 0; i  ncpus; i++) {
511 # ifdef CPU_ALLOC

(9) Event cond_true:Condition __cpu / 8  masklen, taking true branch
(10) Event cond_true:   Condition ((__cpu_mask const *)mask-__bits[__cpu / 
(64UL /* 8 * sizeof (__cpu_mask) */)]  (1UL /* (__cpu_mask)1 */  __cpu % 
(64UL /* 8 * sizeof (__cpu_mask) */))) != 0, taking true branch
(11) Event cond_true:   Condition ({...}), taking true branch
(16) Event assignment:  Assigning: __cpu = i. The value of __cpu may now 
be up to 262143.
(17) Event cond_true:   Condition __cpu / 8  masklen, taking true branch
(18) Event overrun-local:   Overrunning array of 16 8-byte elements at 
element index 4095 (byte offset 32760) by dereferencing pointer (__cpu_mask 
const *)mask-__bits + __cpu / 64UL.
Also see events:[assignment][cond_at_most]

512 if (CPU_ISSET_S(i, masklen, mask))
513 ignore_value(virBitmapSetBit(ret, i));
514 # else
515 if (CPU_ISSET(i, mask))
516 ignore_value(virBitmapSetBit(ret, i));
517 # endif

(12) Event loop:Jumping back to the beginning of the loop

518 }
519 
520  cleanup:
521 # ifdef CPU_ALLOC
522 CPU_FREE(mask);
523 # else
524 VIR_FREE(mask);
525 # endif
526 
527 return 

Re: [libvirt] [PATCH 1/4] storage: Remove extraneous @conn from function comments

2015-06-04 Thread Erik Skultety

On 06/03/2015 01:44 PM, John Ferlan wrote:
 Over time the parameters changed, but the comment wasn't updated
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_fs.c | 3 ---
  1 file changed, 3 deletions(-)
 
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index 337b8d3..b70902a 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -336,7 +336,6 @@ 
 virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn 
 ATTRIBUTE_UNUSE
  
  
  /**
 - * @conn connection to report errors against
   * @pool storage pool to check for status
   *
   * Determine if a storage pool is already mounted
 @@ -369,7 +368,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
 pool)
  }
  
  /**
 - * @conn connection to report errors against
   * @pool storage pool to mount
   *
   * Ensure that a FS storage pool is mounted on its target location.
 @@ -474,7 +472,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr 
 pool)
  }
  
  /**
 - * @conn connection to report errors against
   * @pool storage pool to unmount
   *
   * Ensure that a FS storage pool is not mounted on its target location.
 
ACK, I'll have a look at the rest of the series later today.
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] parallels: add block device statistics to driver

2015-06-04 Thread Nikolay Shirokovskiy


On 03.06.2015 15:16, Dmitry Guryanov wrote:
 On 05/22/2015 10:42 AM, Nikolay Shirokovskiy wrote:
 Statistics provided through PCS SDK. As we have only async interface in SDK 
 we
 need to be subscribed to statistics in order to get it. Trivial solution on
 every stat request to subscribe, wait event and then unsubscribe will lead to
 significant delays in case of a number of successive requests, as the event
 will be delivered on next PCS server notify cycle. On the other hand we don't
 want to keep unnesessary subscribtion. So we take an hibrid solution to
 subcsribe on first request and then keep a subscription while requests are
 active. We populate cache of statistics on subscribtion events and use this
 cache to serve libvirts requests.

 Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com
 ---
   src/parallels/parallels_driver.c |  106 +
   src/parallels/parallels_sdk.c|  193 
 --
   src/parallels/parallels_sdk.h|2 +
   src/parallels/parallels_utils.h  |   15 +++
   4 files changed, 285 insertions(+), 31 deletions(-)


 +parallelsDomainBlockStats(virDomainPtr domain, const char *path,
 + virDomainBlockStatsPtr stats)
 +{
 +virDomainObjPtr dom = NULL;
 +int ret = -1;
 +size_t i;
 +int idx;
 +
 +if (!(dom = parallelsDomObjFromDomain(domain)))
 +return -1;
 +
 +if (*path) {
 +if ((idx = virDomainDiskIndexByName(dom-def, path, false))  0) {
 +virReportError(VIR_ERR_INVALID_ARG, _(invalid path: %s), 
 path);
 +goto cleanup;
 +}
 +if (prlsdkGetBlockStats(dom, dom-def-disks[idx], stats)  0)
 +goto cleanup;
 +} else {
 +virDomainBlockStatsStruct s;
 +
 +#define PARALLELS_ZERO_STATS(VAR, TYPE, NAME)   \
 +stats-VAR = 0;
 +
 +PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_ZERO_STATS)
 +
 +#undef PARALLELS_ZERO_STATS
 +
 
 Why don't you use memset here?
to make code uniform. I use PARALLELS_BLOCK_STATS_FOREACH macro to
iterate on all disk stats PCS supports in different places, here
i use this macro make initialization somewhat *explicit* instead of
*implicit* memset.

 

 +
   static virHypervisorDriver parallelsDriver = {
   .name = Parallels,
   .connectOpen = parallelsConnectOpen,/* 0.10.0 */
 @@ -1228,6 +1332,8 @@ static virHypervisorDriver parallelsDriver = {
   .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */
   .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 
 */
   .domainGetMaxMemory = parallelsDomainGetMaxMemory, /* 1.2.15 */
 +.domainBlockStats = parallelsDomainBlockStats, /* 1.2.16 */
 +.domainBlockStatsFlags = parallelsDomainBlockStatsFlags, /* 1.2.16 */
 
 Could you, please, update there versions to 1.2.17?
   };

ok
 static virConnectDriver parallelsConnectDriver = {
 diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
 index 88ad59b..eb8d965 100644
 --- a/src/parallels/parallels_sdk.c
 +++ b/src/parallels/parallels_sdk.c
 @@ -21,6 +21,7 @@
*/
 
 
 +goto cleanup;
 +
   switch (prlEventType) {
   case PET_DSP_EVT_VM_STATE_CHANGED:
 +prlsdkHandleVmStateEvent(privconn, prlEvent, uuid);
 +break;
   case PET_DSP_EVT_VM_CONFIG_CHANGED:
 +prlsdkHandleVmConfigEvent(privconn, uuid);
 +break;
   case PET_DSP_EVT_VM_CREATED:
   case PET_DSP_EVT_VM_ADDED:
 +prlsdkHandleVmAddedEvent(privconn, uuid);
 +break;
   case PET_DSP_EVT_VM_DELETED:
   case PET_DSP_EVT_VM_UNREGISTERED:
 -pret = prlsdkHandleVmEvent(privconn, prlEvent);
 +prlsdkHandleVmRemovedEvent(privconn, uuid);
 +break;
 +case PET_DSP_EVT_VM_PERFSTATS:
 +prlsdkHandlePerfEvent(privconn, prlEvent, uuid);
 +// above function takes own of event
 +prlEvent = PRL_INVALID_HANDLE;
   break;
   default:
   VIR_DEBUG(Skipping event of type %d, prlEventType);
 @@ -3455,3 +3481,108 @@ prlsdkDomainManagedSaveRemove(virDomainObjPtr dom)
 return 0;
   }
 +
 
 Could you describe the idea with stats cache in more detail? Why do you keer 
 up to 3 prlsdk stats, but use only one? And where do you free these handles?
 
ok, this will go to commit message
Shortly.

1. 3 - this is implicit timeout to drop cache. Explicit is is 3 * 
PCS_INTERVAL_BETWEEN_STAT_EVENTS. If
nobody asks for statistics for this time interval we unsubscribe and make cache 
invalid, so if somebody
will want statistics again we will need to subsciribe again and wait for first 
event to arrive to proceed.

2. Event handle is freed when next event arrived.



 +int
 +prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, 
 virDomainBlockStatsPtr stats)
 +{
 +virDomainDeviceDriveAddressPtr address;
 +int 

Re: [libvirt] [PATCH 35/35] qemu: Refactor qemuDomainSetVcpusFlags by reusing virDomainObjGetDefs

2015-06-04 Thread Peter Krempa
On Wed, Jun 03, 2015 at 16:12:40 +0200, Ján Tomko wrote:
 On Fri, May 29, 2015 at 03:33:56PM +0200, Peter Krempa wrote:
  ---
   src/qemu/qemu_driver.c | 22 --
   1 file changed, 8 insertions(+), 14 deletions(-)
  
 
 ACK to patches 31-35

I've fixed patches 16 and 17 according to your review and pushed this
series. Thanks.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 23/35] conf: Add new helpers to resolve virDomainModificationImpact to domain defs

2015-06-04 Thread John Ferlan


On 05/29/2015 09:33 AM, Peter Krempa wrote:
 virDomainLiveConfigHelperMethod that is used for this job now does
 modify the flags but still requires the callers to extract the correct
 definition objects.
 
 In addition coverity and other static analyzers are usually unhappy as
 they don't grasp the fact that @flags are upadted according to the
 correct def to be present.
 
 To work this issue around and simplify the calling chain let's add a new
 helper that will work only on drivers that always copy the persistent
 def to a transient at start of a vm. This will allow to drop a few
 arguments. The new function syntax will also fill two definition
 pointers rather than modifying the @flags parameter.
 ---
  src/conf/domain_conf.c   | 113 
 ++-
  src/conf/domain_conf.h   |   8 
  src/libvirt_private.syms |   2 +
  3 files changed, 102 insertions(+), 21 deletions(-)
 

Well Coverity is most unhappy with these changes - causes 14 new issues
- all related...  I didn't get a chance to run your series through my
checker because not all of the patches made it through even though they
were in the archive (strange).

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4a8c1b5..e8bda73 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c

...

 +
 +/**
 + * virDomainObjGetDefs:
 + *
 + * @vm: domain object
 + * @flags: for virDomainModificationImpact
 + * @liveDef: Set to the pointer to the live definition of @vm.
 + * @persDef: Set to the pointer to the config definition of @vm.
 + *
 + * Helper function to resolve @flags and retrieve correct domain pointer
 + * objects. This function should be used only when the hypervisor driver 
 always
 + * creates vm-newDef once the vm is started. (qemu driver does that)
 + *
 + * If @liveDef or @persDef are set it implies that @flags request 
 modification
 + * of thereof.
 + *
 + * Returns 0 on success and sets @liveDef and @persDef; -1 if @flags are
 + * inappropriate.
 + */
 +int
 +virDomainObjGetDefs(virDomainObjPtr vm,
 +unsigned int flags,
 +virDomainDefPtr *liveDef,
 +virDomainDefPtr *persDef)
 +{
 +if (liveDef)
 +*liveDef = NULL;
 +
 +if (*persDef)
 +*persDef = NULL;

You dereference *persDef here without checking and furthermore this
causes Coverity to complain about UNINIT usage in each of the callers
since non initialized their def to NULL.

 +
 +if (virDomainObjUpdateModificationImpact(vm, flags)  0)
 +return -1;
 +
 +if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 +if (liveDef)
 +*liveDef = vm-def;
 +
 +if (persDef)
 +*liveDef = vm-newDef;

here you check for persDef, but adjust *liveDef - so if liveDef == NULL,
then there's going to be a failure...

 +} else {
 +if (persDef)
 +*persDef = vm-def;

Also persDef is checked again...

 +}
 +
 +return 0;
  }
 
 +
  /*
   * The caller must hold a lock on the driver owning 'doms',
   * and must also have locked 'dom', to ensure no one else
 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index 34b669d..262d267 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -2545,6 +2545,14 @@ virDomainObjGetPersistentDef(virCapsPtr caps,
   virDomainXMLOptionPtr xmlopt,
   virDomainObjPtr domain);
 
 +int virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
 + unsigned int *flags);
 +
 +int virDomainObjGetDefs(virDomainObjPtr vm,
 +unsigned int flags,
 +virDomainDefPtr *liveDef,
 +virDomainDefPtr *persDef);

How about a ATTRIBUTE_NONNULL(4) here?


John

I'd paste a diff here that worked for me, except that I know you prefer
not to have inlined diffs like that in email... So I'll wait to see how
you fix this...

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [libvirt-php PATCH] docs: remove reference to Red Hat

2015-06-04 Thread Ján Tomko
---
 docs/index.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/index.html.in b/docs/index.html.in
index da40d7a..bb03d65 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -4,7 +4,7 @@
 h1Libvirt-php/h1
 
 plibvirt-php, originally called php-libvirt, is a project that was 
started by Radek Hladik in 2010 to integrate libvirt support to PHP./p
-pIn February 2011 the binding has been moved to libvirt.org site and 
it's currently maintained by Red Hat./p
+pIn February 2011 the binding was moved to libvirt.org./p
 p
bThis project is not affiliated with The PHP Group and the PHP 
project itself./b
 /p
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v5 7/9] domain_conf: Read and Write quorum config

2015-06-04 Thread Matthias Gatto
On Tue, May 12, 2015 at 5:04 PM, Peter Krempa pkre...@redhat.com wrote:
 On Thu, Apr 23, 2015 at 14:41:19 +0200, Matthias Gatto wrote:
 Add the capabiltty to libvirt to parse and format the quorum syntax
 as described here:
 http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html

 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/conf/domain_conf.c | 164 
 +++--
  1 file changed, 119 insertions(+), 45 deletions(-)

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index a3a6c13..ec93b6f 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -5952,20 +5952,56 @@ virDomainDiskSourceParse(xmlNodePtr node,
  }


 +static bool
 +virDomainDiskThresholdParse(virStorageSourcePtr src,
 +xmlNodePtr node)
 +{
 +char *threshold = virXMLPropString(node, threshold);
 +int ret;
 +
 +if (!threshold) {
 +virReportError(VIR_ERR_XML_ERROR,
 +   %s, _(missing threshold in quorum));
 +return false;
 +}
 +ret = virStrToLong_ul(threshold, NULL, 10, src-threshold);
 +if (ret  0 || src-threshold  2) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(unexpected threshold %s),
 +   threshold must be a decimal number superior to 2 
 + and inferior to the number of children);
 +VIR_FREE(threshold);
 +return false;
 +}
 +VIR_FREE(threshold);
 +return true;
 +}
 +
 +
  static int
  virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
 -   virStorageSourcePtr src)
 +   virStorageSourcePtr src,
 +   xmlNodePtr node,
 +   size_t pos)
  {
  virStorageSourcePtr backingStore = NULL;
  xmlNodePtr save_ctxt = ctxt-node;
 -xmlNodePtr source;
 +xmlNodePtr source = NULL;
  char *type = NULL;
  char *format = NULL;
  int ret = -1;

 -if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) {
 -ret = 0;
 -goto cleanup;
 +if (src-type == VIR_STORAGE_TYPE_QUORUM) {
 +if (!node) {
 +ret = 0;
 +goto cleanup;
 +}
 +ctxt-node = node;
 +} else {
 +if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) {
 +ret = 0;
 +goto cleanup;
 +}
  }

  if (VIR_ALLOC(backingStore)  0)
 @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr 
 ctxt,
  goto cleanup;
  }

 +if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) {
 +xmlNodePtr cur = NULL;
 +
 +if (!virDomainDiskThresholdParse(backingStore, node))
 +goto cleanup;
 +
 +for (cur = node-children; cur != NULL; cur = cur-next) {
 +if (xmlStrEqual(cur-name, BAD_CAST backingStore)) {
 +if ((virDomainDiskBackingStoreParse(ctxt,
 +backingStore,
 +cur,
 +
 backingStore-nBackingStores)  0)) {
 +goto cleanup;
 +}
 +}
 +}
 +goto exit;
 +}
 +
  if (!(source = virXPathNode(./source, ctxt))) {
  virReportError(VIR_ERR_XML_ERROR, %s,
 _(missing disk backing store source));
 @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr 
 ctxt,
  }

  if (virDomainDiskSourceParse(source, ctxt, backingStore)  0 ||
 -virDomainDiskBackingStoreParse(ctxt, backingStore)  0)
 +virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0)  0)
  goto cleanup;

 -if (!virStorageSourceSetBackingStore(src, backingStore, 0))
 + exit:
 +if (!virStorageSourceSetBackingStore(src, backingStore, pos))
  goto cleanup;
  ret = 0;

 @@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
  return ret;
  }

 -
  #define VENDOR_LEN  8
  #define PRODUCT_LEN 16

 @@ -6518,6 +6573,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
  }
  } else if (xmlStrEqual(cur-name, BAD_CAST boot)) {
  /* boot is parsed as part of virDomainDeviceInfoParseXML */
 +} else if (xmlStrEqual(cur-name, BAD_CAST backingStore)) {
 +if (virDomainDiskBackingStoreParse(ctxt, def-src, cur,
 +   
 def-src-nBackingStores)  0)
 +goto error;
  }
  }
  cur = cur-next;
 @@ -6541,12 +6600,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr 
 xmlopt,
  def-device = VIR_DOMAIN_DISK_DEVICE_DISK;
  }

 +if (def-src-type == VIR_STORAGE_TYPE_QUORUM 
 +!virDomainDiskThresholdParse(def-src, node))
 +   

Re: [libvirt] [PATCH] maint: remove redundant apostrophes from 'its'

2015-06-04 Thread Ján Tomko
On Wed, Jun 03, 2015 at 11:04:08AM -0600, Eric Blake wrote:
 On 06/03/2015 06:48 AM, Ján Tomko wrote:
 
 s/redundant/incorrect/
 
 They would only be redundant if having them was correct but optional;
 but in all cases you touched, they were grammatically wrong: there is no
 optional ', it is either its (possessive) or it's (short for it is).
 

Oh, I didn't realize 'redundant' implied that both were correct.

  ---
   docs/formatnode.html.in | 2 +-
   src/conf/storage_conf.c | 2 +-
   src/esx/esx_driver.c| 2 +-
   src/esx/esx_network_driver.c| 2 +-
   src/esx/esx_storage_backend_iscsi.c | 2 +-
   src/esx/esx_storage_backend_vmfs.c  | 2 +-
   src/libxl/libxl_domain.c| 2 +-
   src/network/bridge_driver.c | 2 +-
   src/parallels/parallels_storage.c   | 2 +-
   src/qemu/qemu_driver.c  | 4 ++--
   src/qemu/qemu_migration.c   | 2 +-
   src/vbox/vbox_common.c  | 2 +-
   12 files changed, 13 insertions(+), 13 deletions(-)
  
 
 ACK with the subject tweaked
 

Thanks, I tweaked the commit summary and pushed it.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: php: remove reference to Red Hat

2015-06-04 Thread Ján Tomko
On Wed, Jun 03, 2015 at 03:13:02PM +0200, Erik Skultety wrote:
 
 
 On 06/03/2015 02:48 PM, Ján Tomko wrote:
  Also remove the redudant apostrophe from it's.
  ---
   docs/php.html.in | 5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)
  
  diff --git a/docs/php.html.in b/docs/php.html.in
  index d9a3c1b..c10b0aa 100644
  --- a/docs/php.html.in
  +++ b/docs/php.html.in
  @@ -6,8 +6,7 @@
   
   h2Presentation/h2
   pThe libvirt-php, originally called php-libvirt, is the PHP API 
  bindings for
  -   the libvirt virtualization toolkit originally developed by Radek 
  Hladik but
  -   currently maintained by Red Hat./p
  +   the libvirt virtualization toolkit originally developed by Radek 
  Hladik./p
   
   h2Getting the source/h2
   p The PHP bindings code source is now maintained in a a
  @@ -26,7 +25,7 @@ It can also be browsed at
   
   p/p
   h2Project pages/h2
  -pSince February 2011 the project have it's own pages hosted at 
  libvirt.org. For more information on the project
  +pSince February 2011 the project has its own pages hosted at 
  libvirt.org. For more information on the project
  please refer to a 
  href=http://libvirt.org/php;http://libvirt.org/php/a.
   
   /p
  
 You missed index.html.in in libvirt-php repo. ACK with that 1 adjustment.

I can't make an adjustment in another repo in this patch :)

I pushed this one and sent the libvirt-php patch separately.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Check for qemu capability when calling virDomainGetBlockIoTune()

2015-06-04 Thread Martin Kletzander

ping

On Thu, May 28, 2015 at 08:18:31PM +0200, Martin Kletzander wrote:

When getting block device I/O tuning data there is no check for whether
QEMU supports such options and the call fails on
qemuMonitorGetBlockIoThrottle() when getting the particular throttle
data.  So try reporting a better error when blkdeviotune is not
supported.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1224053

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1b00a2014ba..8f8ad8ba5bf5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18065,6 +18065,12 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
 * because we need vm-privateData which need
 * virDomainLiveConfigHelperMethod to do so. */
priv = vm-privateData;
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(block I/O throttling not supported with this 
+ QEMU binary));
+goto endjob;
+}
supportMaxOptions = virQEMUCapsGet(priv-qemuCaps, 
QEMU_CAPS_DRIVE_IOTUNE_MAX);
}

--
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] maint: remove redundant apostrophes from 'its'

2015-06-04 Thread Ján Tomko
On Wed, Jun 03, 2015 at 03:46:17PM +0200, Andrea Bolognani wrote:
 On Wed, 2015-06-03 at 14:48 +0200, Ján Tomko wrote:
  ---
   docs/formatnode.html.in | 2 +-
   src/conf/storage_conf.c | 2 +-
   src/esx/esx_driver.c| 2 +-
   src/esx/esx_network_driver.c| 2 +-
   src/esx/esx_storage_backend_iscsi.c | 2 +-
   src/esx/esx_storage_backend_vmfs.c  | 2 +-
   src/libxl/libxl_domain.c| 2 +-
   src/network/bridge_driver.c | 2 +-
   src/parallels/parallels_storage.c   | 2 +-
   src/qemu/qemu_driver.c  | 4 ++--
   src/qemu/qemu_migration.c   | 2 +-
   src/vbox/vbox_common.c  | 2 +-
   12 files changed, 13 insertions(+), 13 deletions(-)
 
 Redundant: I don't think it means what you think it means ;)
 
 Cheers.
 

Thank you for your helpful review.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Check for qemu capability when calling virDomainGetBlockIoTune()

2015-06-04 Thread Ján Tomko
On Thu, May 28, 2015 at 08:18:31PM +0200, Martin Kletzander wrote:
 When getting block device I/O tuning data there is no check for whether
 QEMU supports such options and the call fails on
 qemuMonitorGetBlockIoThrottle() when getting the particular throttle
 data.  So try reporting a better error when blkdeviotune is not
 supported.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1224053
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/qemu/qemu_driver.c | 6 ++
  1 file changed, 6 insertions(+)

ACK

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] parallels: simplify event types discrimination

2015-06-04 Thread Nikolay Shirokovskiy


On 04.06.2015 12:56, Maxim Nestratov wrote:
 04.06.2015 12:50, Nikolay Shirokovskiy пишет:
 Use issue type instead of event type to group
 vm related events. This saves us from
 explicit enumeration of all vm even types in
 prlsdkHandleVmEvent.
 s/issue/issuer
 ---
ok 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] parallels: simplify event types discrimination

2015-06-04 Thread Nikolay Shirokovskiy
Use issue type instead of event type to group
vm related events. This saves us from
explicit enumeration of all vm even types in
prlsdkHandleVmEvent.
---
 src/parallels/parallels_sdk.c |   15 +--
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 88ad59b..d5a9790 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1681,7 +1681,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 parallelsConnPtr privconn = opaque;
 PRL_RESULT pret = PRL_ERR_UNINITIALIZED;
 PRL_HANDLE_TYPE handleType;
-PRL_EVENT_TYPE prlEventType;
+PRL_EVENT_ISSUER_TYPE prlIssuerType = PIE_UNKNOWN;
 
 pret = PrlHandle_GetType(prlEvent, handleType);
 prlsdkCheckRetGoto(pret, cleanup);
@@ -1697,20 +1697,15 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 goto cleanup;
 }
 
-PrlEvent_GetType(prlEvent, prlEventType);
+PrlEvent_GetIssuerType(prlEvent, prlIssuerType);
 prlsdkCheckRetGoto(pret, cleanup);
 
-switch (prlEventType) {
-case PET_DSP_EVT_VM_STATE_CHANGED:
-case PET_DSP_EVT_VM_CONFIG_CHANGED:
-case PET_DSP_EVT_VM_CREATED:
-case PET_DSP_EVT_VM_ADDED:
-case PET_DSP_EVT_VM_DELETED:
-case PET_DSP_EVT_VM_UNREGISTERED:
+switch (prlIssuerType) {
+case PIE_VIRTUAL_MACHINE:
 pret = prlsdkHandleVmEvent(privconn, prlEvent);
 break;
 default:
-VIR_DEBUG(Skipping event of type %d, prlEventType);
+VIR_DEBUG(Skipping event of issuer type %d, prlIssuerType);
 }
 
 pret = PRL_ERR_SUCCESS;
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] parallels: simplify event types discrimination

2015-06-04 Thread Maxim Nestratov

04.06.2015 12:50, Nikolay Shirokovskiy пишет:

Use issue type instead of event type to group
vm related events. This saves us from
explicit enumeration of all vm even types in
prlsdkHandleVmEvent.

s/issue/issuer

---
  src/parallels/parallels_sdk.c |   15 +--
  1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 88ad59b..d5a9790 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1681,7 +1681,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
  parallelsConnPtr privconn = opaque;
  PRL_RESULT pret = PRL_ERR_UNINITIALIZED;
  PRL_HANDLE_TYPE handleType;
-PRL_EVENT_TYPE prlEventType;
+PRL_EVENT_ISSUER_TYPE prlIssuerType = PIE_UNKNOWN;
  
  pret = PrlHandle_GetType(prlEvent, handleType);

  prlsdkCheckRetGoto(pret, cleanup);
@@ -1697,20 +1697,15 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
  goto cleanup;
  }
  
-PrlEvent_GetType(prlEvent, prlEventType);

+PrlEvent_GetIssuerType(prlEvent, prlIssuerType);
  prlsdkCheckRetGoto(pret, cleanup);
  
-switch (prlEventType) {

-case PET_DSP_EVT_VM_STATE_CHANGED:
-case PET_DSP_EVT_VM_CONFIG_CHANGED:
-case PET_DSP_EVT_VM_CREATED:
-case PET_DSP_EVT_VM_ADDED:
-case PET_DSP_EVT_VM_DELETED:
-case PET_DSP_EVT_VM_UNREGISTERED:
+switch (prlIssuerType) {
+case PIE_VIRTUAL_MACHINE:
  pret = prlsdkHandleVmEvent(privconn, prlEvent);
  break;
  default:
-VIR_DEBUG(Skipping event of type %d, prlEventType);
+VIR_DEBUG(Skipping event of issuer type %d, prlIssuerType);
  }
  
  pret = PRL_ERR_SUCCESS;


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 23/35] conf: Add new helpers to resolve virDomainModificationImpact to domain defs

2015-06-04 Thread Peter Krempa
On Thu, Jun 04, 2015 at 07:02:00 -0400, John Ferlan wrote:
 
 
 On 05/29/2015 09:33 AM, Peter Krempa wrote:
  virDomainLiveConfigHelperMethod that is used for this job now does
  modify the flags but still requires the callers to extract the correct
  definition objects.
  
  In addition coverity and other static analyzers are usually unhappy as
  they don't grasp the fact that @flags are upadted according to the
  correct def to be present.
  
  To work this issue around and simplify the calling chain let's add a new
  helper that will work only on drivers that always copy the persistent
  def to a transient at start of a vm. This will allow to drop a few
  arguments. The new function syntax will also fill two definition
  pointers rather than modifying the @flags parameter.
  ---
   src/conf/domain_conf.c   | 113 
  ++-
   src/conf/domain_conf.h   |   8 
   src/libvirt_private.syms |   2 +
   3 files changed, 102 insertions(+), 21 deletions(-)
  
 
 Well Coverity is most unhappy with these changes - causes 14 new issues
 - all related...  I didn't get a chance to run your series through my
 checker because not all of the patches made it through even though they
 were in the archive (strange).
 
  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
  index 4a8c1b5..e8bda73 100644
  --- a/src/conf/domain_conf.c
  +++ b/src/conf/domain_conf.c
 
 ...
 
  +
  +/**
  + * virDomainObjGetDefs:
  + *
  + * @vm: domain object
  + * @flags: for virDomainModificationImpact
  + * @liveDef: Set to the pointer to the live definition of @vm.
  + * @persDef: Set to the pointer to the config definition of @vm.
  + *
  + * Helper function to resolve @flags and retrieve correct domain pointer
  + * objects. This function should be used only when the hypervisor driver 
  always
  + * creates vm-newDef once the vm is started. (qemu driver does that)
  + *
  + * If @liveDef or @persDef are set it implies that @flags request 
  modification
  + * of thereof.
  + *
  + * Returns 0 on success and sets @liveDef and @persDef; -1 if @flags are
  + * inappropriate.
  + */
  +int
  +virDomainObjGetDefs(virDomainObjPtr vm,
  +unsigned int flags,
  +virDomainDefPtr *liveDef,
  +virDomainDefPtr *persDef)
  +{
  +if (liveDef)
  +*liveDef = NULL;
  +
  +if (*persDef)
  +*persDef = NULL;
 
 You dereference *persDef here without checking and furthermore this
 causes Coverity to complain about UNINIT usage in each of the callers
 since non initialized their def to NULL.a

[1]


 
  +
  +if (virDomainObjUpdateModificationImpact(vm, flags)  0)
  +return -1;
  +
  +if (flags  VIR_DOMAIN_AFFECT_LIVE) {
  +if (liveDef)
  +*liveDef = vm-def;
  +
  +if (persDef)
  +*liveDef = vm-newDef;
 
 here you check for persDef, but adjust *liveDef - so if liveDef == NULL,
 then there's going to be a failure...
 
  +} else {
  +if (persDef)
  +*persDef = vm-def;
 
 Also persDef is checked again...
 
  +}
  +
  +return 0;
   }
  
  +
   /*
* The caller must hold a lock on the driver owning 'doms',
* and must also have locked 'dom', to ensure no one else
  diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
  index 34b669d..262d267 100644
  --- a/src/conf/domain_conf.h
  +++ b/src/conf/domain_conf.h
  @@ -2545,6 +2545,14 @@ virDomainObjGetPersistentDef(virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
virDomainObjPtr domain);
  
  +int virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
  + unsigned int *flags);
  +
  +int virDomainObjGetDefs(virDomainObjPtr vm,
  +unsigned int flags,
  +virDomainDefPtr *liveDef,
  +virDomainDefPtr *persDef);
 
 How about a ATTRIBUTE_NONNULL(4) here?

Actually, the first check has an extra dereference that should not be
there. The function expects that NULL is passed here.

 
 
 John
 
 I'd paste a diff here that worked for me, except that I know you prefer
 not to have inlined diffs like that in email... So I'll wait to see how
 you fix this...
 

The change is to remove the deref at [1].

Peter



signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] parallels: add block device statistics to driver

2015-06-04 Thread Nikolay Shirokovskiy
Statistics provided through PCS SDK. As we have only async interface in SDK we
need to be subscribed to statistics in order to get it. Trivial solution on
every stat request to subscribe, wait event and then unsubscribe will lead to
significant delays in case of a number of successive requests, as the event
will be delivered on next PCS server notify cycle. On the other hand we don't
want to keep unnesessary subscribtion. So we take an hibrid solution to
subcsribe on first request and then keep a subscription while requests are
active. We populate cache of statistics on subscribtion events and use this
cache to serve libvirts requests.

 * Cache details.
Cache is just handle to last arrived event, we call this cache
as if this handle is valid it is used to serve synchronous
statistics requests. We use number of successive events count
to detect that user lost interest to statistics. We reset this
count to 0 on every request. If more than PARALLELS_STATISTICS_DROP_COUNT
successive events arrive we unsubscribe. Special value of -1
of this counter is used to differentiate between subscribed/unsubscribed state
to protect from delayed events.

Values of RALLELS_STATISTICS_DROP_COUNT and PARALLELS_STATISTICS_TIMEOUT are
just drop-ins, choosen without special consideration.

 * Thread safety issues
Use parallelsDomObjFromDomainRef in parallelsDomainBlockStats as
we could wait on domain lock down on stack in prlsdkGetStatsParam
and if we won't keep reference we could get dangling pointer
on return from wait.

Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com
---
 src/parallels/parallels_driver.c |  106 ++
 src/parallels/parallels_sdk.c|  183 ++
 src/parallels/parallels_sdk.h|2 +
 src/parallels/parallels_utils.c  |   28 ++
 src/parallels/parallels_utils.h  |   18 
 5 files changed, 337 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 4b87213..33c112e 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -51,6 +51,7 @@
 #include nodeinfo.h
 #include virstring.h
 #include cpu/cpu.h
+#include virtypedparam.h
 
 #include parallels_driver.h
 #include parallels_utils.h
@@ -1179,6 +1180,109 @@ parallelsDomainGetMaxMemory(virDomainPtr domain)
 return ret;
 }
 
+static int
+parallelsDomainBlockStats(virDomainPtr domain, const char *path,
+ virDomainBlockStatsPtr stats)
+{
+virDomainObjPtr dom = NULL;
+int ret = -1;
+size_t i;
+int idx;
+
+if (!(dom = parallelsDomObjFromDomainRef(domain)))
+return -1;
+
+if (*path) {
+if ((idx = virDomainDiskIndexByName(dom-def, path, false))  0) {
+virReportError(VIR_ERR_INVALID_ARG, _(invalid path: %s), path);
+goto cleanup;
+}
+if (prlsdkGetBlockStats(dom, dom-def-disks[idx], stats)  0)
+goto cleanup;
+} else {
+virDomainBlockStatsStruct s;
+
+#define PARALLELS_ZERO_STATS(VAR, TYPE, NAME)   \
+stats-VAR = 0;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_ZERO_STATS)
+
+#undef PARALLELS_ZERO_STATS
+
+for (i = 0; i  dom-def-ndisks; i++) {
+if (prlsdkGetBlockStats(dom, dom-def-disks[i], s)  0)
+goto cleanup;
+
+#define PARALLELS_SUM_STATS(VAR, TYPE, NAME)\
+if (s.VAR != -1)\
+stats-VAR += s.VAR;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
+
+#undef  PARALLELS_SUM_STATS
+}
+}
+stats-errs = -1;
+ret = 0;
+
+ cleanup:
+if (dom)
+virDomainObjEndAPI(dom);
+
+return ret;
+}
+
+static int
+parallelsDomainBlockStatsFlags(virDomainPtr domain,
+  const char *path,
+  virTypedParameterPtr params,
+  int *nparams,
+  unsigned int flags)
+{
+virDomainBlockStatsStruct stats;
+int ret = -1;
+size_t i;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+/* We don't return strings, and thus trivially support this flag.  */
+flags = ~VIR_TYPED_PARAM_STRING_OKAY;
+
+if (parallelsDomainBlockStats(domain, path, stats)  0)
+goto cleanup;
+
+if (*nparams == 0) {
+#define PARALLELS_COUNT_STATS(VAR, TYPE, NAME)   \
+if ((stats.VAR) != -1)   \
+++*nparams;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_COUNT_STATS)
+
+#undef PARALLELS_COUNT_STATS
+ret = 0;
+goto cleanup;
+}
+
+i = 0;
+#define PARALLELS_BLOCK_STATS_ASSIGN_PARAM(VAR, TYPE, NAME)
\
+if (i  *nparams  (stats.VAR) != -1) {   
\
+if (virTypedParameterAssign(params + i, TYPE,  
\
+VIR_TYPED_PARAM_LLONG, (stats.VAR))  0)   
\
+ 

[libvirt] [PATCH v2] parallels: simplify event types discrimination

2015-06-04 Thread Nikolay Shirokovskiy
Use issuer type instead of event type to group
vm related events. This saves us from
explicit enumeration of all vm event types in
prlsdkHandleVmEvent.
---
 src/parallels/parallels_sdk.c |   15 +--
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 88ad59b..d5a9790 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1681,7 +1681,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 parallelsConnPtr privconn = opaque;
 PRL_RESULT pret = PRL_ERR_UNINITIALIZED;
 PRL_HANDLE_TYPE handleType;
-PRL_EVENT_TYPE prlEventType;
+PRL_EVENT_ISSUER_TYPE prlIssuerType = PIE_UNKNOWN;
 
 pret = PrlHandle_GetType(prlEvent, handleType);
 prlsdkCheckRetGoto(pret, cleanup);
@@ -1697,20 +1697,15 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 goto cleanup;
 }
 
-PrlEvent_GetType(prlEvent, prlEventType);
+PrlEvent_GetIssuerType(prlEvent, prlIssuerType);
 prlsdkCheckRetGoto(pret, cleanup);
 
-switch (prlEventType) {
-case PET_DSP_EVT_VM_STATE_CHANGED:
-case PET_DSP_EVT_VM_CONFIG_CHANGED:
-case PET_DSP_EVT_VM_CREATED:
-case PET_DSP_EVT_VM_ADDED:
-case PET_DSP_EVT_VM_DELETED:
-case PET_DSP_EVT_VM_UNREGISTERED:
+switch (prlIssuerType) {
+case PIE_VIRTUAL_MACHINE:
 pret = prlsdkHandleVmEvent(privconn, prlEvent);
 break;
 default:
-VIR_DEBUG(Skipping event of type %d, prlEventType);
+VIR_DEBUG(Skipping event of issuer type %d, prlIssuerType);
 }
 
 pret = PRL_ERR_SUCCESS;
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Jiri Denemark
On Wed, Jun 03, 2015 at 15:13:17 +, Feng, Shaohe wrote:
 Eric, 
 Thank you for reply.
 
 Is it necessary to expose these two APIs to user application? 
 + virdomainMigrateSetCapabilities
 + virdomainMigrateGetCapabilities
 
 For qemu , the migration capabilities are xbzrle, rdma-pin-all,  
 auto-converge,
  zero-blocks and compress.

No, these capabilities are either turned on/off based on flags passed to
virDomainMigrate3 API.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2 0/2] Fix code after the vcpupin series

2015-06-04 Thread Peter Krempa
Both patches are pushed as trivial.

Peter Krempa (2):
  conf: Fix mistakes in pointer usage in virDomainObjGetDefs
  qemu: Update balloon info only if job is allowed

 src/conf/domain_conf.c | 4 ++--
 src/qemu/qemu_domain.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] parallels: add block device statistics to driver

2015-06-04 Thread Nikolay Shirokovskiy


On 04.06.2015 14:45, Nikolay Shirokovskiy wrote:
 Statistics provided through PCS SDK. As we have only async interface in SDK we
 need to be subscribed to statistics in order to get it. Trivial solution on
 every stat request to subscribe, wait event and then unsubscribe will lead to
 significant delays in case of a number of successive requests, as the event
 will be delivered on next PCS server notify cycle. On the other hand we don't
 want to keep unnesessary subscribtion. So we take an hibrid solution to
 subcsribe on first request and then keep a subscription while requests are
 active. We populate cache of statistics on subscribtion events and use this
 cache to serve libvirts requests.
 
  * Cache details.
 Cache is just handle to last arrived event, we call this cache
 as if this handle is valid it is used to serve synchronous
 statistics requests. We use number of successive events count
 to detect that user lost interest to statistics. We reset this
 count to 0 on every request. If more than PARALLELS_STATISTICS_DROP_COUNT
 successive events arrive we unsubscribe. Special value of -1
 of this counter is used to differentiate between subscribed/unsubscribed state
 to protect from delayed events.
 
 Values of RALLELS_STATISTICS_DROP_COUNT and PARALLELS_STATISTICS_TIMEOUT are
 just drop-ins, choosen without special consideration.
 
  * Thread safety issues
 Use parallelsDomObjFromDomainRef in parallelsDomainBlockStats as
 we could wait on domain lock down on stack in prlsdkGetStatsParam
 and if we won't keep reference we could get dangling pointer
 on return from wait.
 
 Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com
 ---
  src/parallels/parallels_driver.c |  106 ++
  src/parallels/parallels_sdk.c|  183 
 ++
  src/parallels/parallels_sdk.h|2 +
  src/parallels/parallels_utils.c  |   28 ++
  src/parallels/parallels_utils.h  |   18 
  5 files changed, 337 insertions(+), 0 deletions(-)
 
Need more checks in case of unsuccessful unsubscribe, will resend.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

2015-06-04 Thread Jiri Denemark
On Tue, Jun 02, 2015 at 11:02:27 -0600, Eric Blake wrote:
 On 06/02/2015 07:56 AM, Qiao, Liyong wrote:
  Hi Eric
  Thanks for replying the mail, replied in lines.
  
 
  +virdomainMigrateGetParameters(virDomainPtr domain,
  +  int *level,
  +  int *threads,
  +  int *dthreads,
  +  int flags)
  +
  
  I'd rather we used virTypedParameters, to make it easier to use the same 
  API to pass ALL future possible tuning parameters, rather than just 
  hard-coding to only the parameters of this one feature.
  
  Okay ,that sound good, but if virTypedParameters can be passed to 
  qemu_driver such as qemu_monitor_json.c qemu_monitor_text.c ?
 
 [Your quoting style makes it very hard to distinguish original text from
 added text.  Please consider changing your outgoing mail process to
 ensure that you add another level of quoting to all previous text so
 that your added text is the only unquoted text.  Also, configure your
 mailer to wrap long lines.]
 
 Use existing API for a guide - for example, virDomainSetBlockIoTune
 takes virTypedParamters, as well as defines a specific set of parameters
 that it will understand.

And do we actually need to changed these migration parameters on the fly
when migration is already running. I can imagine we would need to do so
for some parameters but multithreaded compression sounds like something
that needs to be configured when migration starts and there's nothing to
be tuned on the fly. If this is the case, I think we should just add
these new parameters to virDomainMigrate3 instead of requiring another
API to be called to actually configure multithreaded compression.

The separate API makes it a bit harder to repeat migration (which
previously failed) with a different parameters (e.g., without
multithreaded compression).

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3] parallels: add block device statistics to driver

2015-06-04 Thread Nikolay Shirokovskiy
Statistics provided through PCS SDK. As we have only async interface in SDK we
need to be subscribed to statistics in order to get it. Trivial solution on
every stat request to subscribe, wait event and then unsubscribe will lead to
significant delays in case of a number of successive requests, as the event
will be delivered on next PCS server notify cycle. On the other hand we don't
want to keep unnesessary subscribtion. So we take an hibrid solution to
subcsribe on first request and then keep a subscription while requests are
active. We populate cache of statistics on subscribtion events and use this
cache to serve libvirts requests.

 * Cache details.
Cache is just handle to last arrived event, we call this cache
as if this handle is valid it is used to serve synchronous
statistics requests. We use number of successive events count
to detect that user lost interest to statistics. We reset this
count to 0 on every request. If more than PARALLELS_STATISTICS_DROP_COUNT
successive events arrive we unsubscribe. Special value of -1
of this counter is used to differentiate between subscribed/unsubscribed state
to protect from delayed events.

Values of PARALLELS_STATISTICS_DROP_COUNT and PARALLELS_STATISTICS_TIMEOUT are
just drop-ins, choosen without special consideration.

 * Thread safety issues
Use parallelsDomObjFromDomainRef in parallelsDomainBlockStats as
we could wait on domain lock down on stack in prlsdkGetStatsParam
and if we won't keep reference we could get dangling pointer
on return from wait.

Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com
---
 src/parallels/parallels_driver.c |  106 +
 src/parallels/parallels_sdk.c|  189 ++
 src/parallels/parallels_sdk.h|2 +
 src/parallels/parallels_utils.c  |   28 ++
 src/parallels/parallels_utils.h  |   18 
 5 files changed, 343 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 4b87213..33c112e 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -51,6 +51,7 @@
 #include nodeinfo.h
 #include virstring.h
 #include cpu/cpu.h
+#include virtypedparam.h
 
 #include parallels_driver.h
 #include parallels_utils.h
@@ -1179,6 +1180,109 @@ parallelsDomainGetMaxMemory(virDomainPtr domain)
 return ret;
 }
 
+static int
+parallelsDomainBlockStats(virDomainPtr domain, const char *path,
+ virDomainBlockStatsPtr stats)
+{
+virDomainObjPtr dom = NULL;
+int ret = -1;
+size_t i;
+int idx;
+
+if (!(dom = parallelsDomObjFromDomainRef(domain)))
+return -1;
+
+if (*path) {
+if ((idx = virDomainDiskIndexByName(dom-def, path, false))  0) {
+virReportError(VIR_ERR_INVALID_ARG, _(invalid path: %s), path);
+goto cleanup;
+}
+if (prlsdkGetBlockStats(dom, dom-def-disks[idx], stats)  0)
+goto cleanup;
+} else {
+virDomainBlockStatsStruct s;
+
+#define PARALLELS_ZERO_STATS(VAR, TYPE, NAME)   \
+stats-VAR = 0;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_ZERO_STATS)
+
+#undef PARALLELS_ZERO_STATS
+
+for (i = 0; i  dom-def-ndisks; i++) {
+if (prlsdkGetBlockStats(dom, dom-def-disks[i], s)  0)
+goto cleanup;
+
+#define PARALLELS_SUM_STATS(VAR, TYPE, NAME)\
+if (s.VAR != -1)\
+stats-VAR += s.VAR;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
+
+#undef  PARALLELS_SUM_STATS
+}
+}
+stats-errs = -1;
+ret = 0;
+
+ cleanup:
+if (dom)
+virDomainObjEndAPI(dom);
+
+return ret;
+}
+
+static int
+parallelsDomainBlockStatsFlags(virDomainPtr domain,
+  const char *path,
+  virTypedParameterPtr params,
+  int *nparams,
+  unsigned int flags)
+{
+virDomainBlockStatsStruct stats;
+int ret = -1;
+size_t i;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+/* We don't return strings, and thus trivially support this flag.  */
+flags = ~VIR_TYPED_PARAM_STRING_OKAY;
+
+if (parallelsDomainBlockStats(domain, path, stats)  0)
+goto cleanup;
+
+if (*nparams == 0) {
+#define PARALLELS_COUNT_STATS(VAR, TYPE, NAME)   \
+if ((stats.VAR) != -1)   \
+++*nparams;
+
+PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_COUNT_STATS)
+
+#undef PARALLELS_COUNT_STATS
+ret = 0;
+goto cleanup;
+}
+
+i = 0;
+#define PARALLELS_BLOCK_STATS_ASSIGN_PARAM(VAR, TYPE, NAME)
\
+if (i  *nparams  (stats.VAR) != -1) {   
\
+if (virTypedParameterAssign(params + i, TYPE,  
\
+VIR_TYPED_PARAM_LLONG, (stats.VAR))  0)   
\
+

[libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

2015-06-04 Thread Peter Krempa
Refactor the code flow a bit more to clear coverity errors.

Store the cpu count in an intermediate variable and reuse it rather than
caluclating the index.
---
 src/util/virprocess.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index a38cb75..c07e5cd 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -474,12 +474,14 @@ virProcessGetAffinity(pid_t pid)
 size_t i;
 cpu_set_t *mask;
 size_t masklen;
+size_t ncpus;
 virBitmapPtr ret = NULL;

 # ifdef CPU_ALLOC
 /* 262144 cpus ought to be enough for anyone */
-masklen = CPU_ALLOC_SIZE(1024  8);
-mask = CPU_ALLOC(1024  8);
+ncpus = 1024  8;
+masklen = CPU_ALLOC_SIZE(ncpus);
+mask = CPU_ALLOC(ncpus);

 if (!mask) {
 virReportOOMError();
@@ -488,6 +490,7 @@ virProcessGetAffinity(pid_t pid)

 CPU_ZERO_S(masklen, mask);
 # else
+ncpus = 1024;
 if (VIR_ALLOC(mask)  0)
 return NULL;

@@ -501,10 +504,10 @@ virProcessGetAffinity(pid_t pid)
 goto cleanup;
 }

-if (!(ret = virBitmapNew(masklen * 8)))
+if (!(ret = virBitmapNew(ncpus)))
   goto cleanup;

-for (i = 0; i  masklen * 8; i++) {
+for (i = 0; i  ncpus; i++) {
 # ifdef CPU_ALLOC
 if (CPU_ISSET_S(i, masklen, mask))
 ignore_value(virBitmapSetBit(ret, i));
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

2015-06-04 Thread John Ferlan


On 06/04/2015 08:15 AM, Peter Krempa wrote:
 Refactor the code flow a bit more to clear coverity errors.
 
 Store the cpu count in an intermediate variable and reuse it rather than
 caluclating the index.
 ---
  src/util/virprocess.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)
 
 diff --git a/src/util/virprocess.c b/src/util/virprocess.c
 index a38cb75..c07e5cd 100644
 --- a/src/util/virprocess.c
 +++ b/src/util/virprocess.c
 @@ -474,12 +474,14 @@ virProcessGetAffinity(pid_t pid)
  size_t i;
  cpu_set_t *mask;
  size_t masklen;
 +size_t ncpus;
  virBitmapPtr ret = NULL;
 
  # ifdef CPU_ALLOC
  /* 262144 cpus ought to be enough for anyone */
 -masklen = CPU_ALLOC_SIZE(1024  8);
 -mask = CPU_ALLOC(1024  8);
 +ncpus = 1024  8;
 +masklen = CPU_ALLOC_SIZE(ncpus);
 +mask = CPU_ALLOC(ncpus);
 
  if (!mask) {
  virReportOOMError();
 @@ -488,6 +490,7 @@ virProcessGetAffinity(pid_t pid)
 
  CPU_ZERO_S(masklen, mask);
  # else
 +ncpus = 1024;
  if (VIR_ALLOC(mask)  0)
  return NULL;
 
 @@ -501,10 +504,10 @@ virProcessGetAffinity(pid_t pid)
  goto cleanup;
  }
 
 -if (!(ret = virBitmapNew(masklen * 8)))
 +if (!(ret = virBitmapNew(ncpus)))
goto cleanup;
 
 -for (i = 0; i  masklen * 8; i++) {
 +for (i = 0; i  ncpus; i++) {
  # ifdef CPU_ALLOC
  if (CPU_ISSET_S(i, masklen, mask))

^^^ Coverity still complains here

No real change since previous...

John
  ignore_value(virBitmapSetBit(ret, i));
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 16/35] qemu: Add helper to update domain balloon size and refactor usage places

2015-06-04 Thread John Ferlan


On 06/03/2015 09:16 AM, Ján Tomko wrote:
 On Fri, May 29, 2015 at 03:33:37PM +0200, Peter Krempa wrote:
 When qemu does not support the balloon event the current memory size
 needs to be queried. Since there are two places that implement the same
 logic, split it out into a function and reuse.
 ---
  src/qemu/qemu_domain.c | 64 ++
  src/qemu/qemu_domain.h |  3 ++
  src/qemu/qemu_driver.c | 84 
 +-
  3 files changed, 75 insertions(+), 76 deletions(-)

 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index db8554b..661181f 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -3182,3 +3182,67 @@ qemuDomainMachineIsI440FX(const virDomainDef *def)
  STRPREFIX(def-os.machine, pc-i440) ||
  STRPREFIX(def-os.machine, rhel));
  }
 +
 +
 +/**
 + * qemuDomainUpdateCurrentMemorySize:
 + *
 + * Updates the current balloon size from the monitor if necessary. In case 
 when
 + * the balloon is not present for the domain, the function recalculates the
 + * maximum size to reflect possible changes.
 + *
 + * Returns 0 on success and updates vm-def-mem.cur_balloon if necessary, 
 -1 on
 + * error and reports libvirt error.
 + */
 +int
 +qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
 +  virDomainObjPtr vm)
 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +unsigned long long balloon;
 +int ret = -1;
 +
 +/* inactive domain doesn't need size update */
 +if (!virDomainObjIsActive(vm))
 +return 0;
 +
 +/* if no balloning is available, the current size equals to the current
 + * full memory size */
 +if (!vm-def-memballoon ||
 +vm-def-memballoon-model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
 +vm-def-mem.cur_balloon = virDomainDefGetMemoryActual(vm-def);
 +return 0;
 +}
 +
 +/* current size is always automagically updated via the event */
 +if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BALLOON_EVENT))
 +return 0;
 +
 +/* here we need to ask the monitor */
 +
 +/* Don't delay if someone's using the monitor, just use existing most
 + * recent data instead */
 +if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {
 +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY)  0)
 +return -1;
 +
 +if (!virDomainObjIsActive(vm)) {
 +virReportError(VIR_ERR_OPERATION_INVALID, %s,
 +   _(domain is not running));
 +goto endjob;
 +}
 +
 +qemuDomainObjEnterMonitor(driver, vm);
 +ret = qemuMonitorGetBalloonInfo(priv-mon, balloon);
 +if (qemuDomainObjExitMonitor(driver, vm)  0)
 +ret = -1;
 +
 + endjob:
 +qemuDomainObjEndJob(driver, vm);
 +
 +if (ret  0)
 +return -1;
 
 ACK if you actually use the 'balloon' value to update cur_balloon here.
 
 Jan
 

Making Coverity unhappy...

3215qemuDomainObjPrivatePtr priv = vm-privateData;

(1) Event var_decl: Declaring variable balloon without initializer.
Also see events:[uninit_use]

3216unsigned long long balloon;

...
238  * recent data instead */

(9) Event cond_false:   Condition qemuDomainJobAllowed(priv,
QEMU_JOB_QUERY), taking false branch

3239if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {

...

3258return -1;

(10) Event if_end:  End of if statement

3259}
3260

(11) Event uninit_use:  Using uninitialized value balloon.
Also see events:[var_decl]

3261vm-def-mem.cur_balloon = balloon;
3262


So if QEMU_JOB_QUERY is not allowed, then balloon is not initialized and
setting to cur_balloon is bad

Adding an if (ret == 0) prior to the setting works.


John

 +}
 +
 +return 0;
 +}


 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-php PATCH] docs: remove reference to Red Hat

2015-06-04 Thread Erik Skultety
ACK

Erik

On 06/04/2015 09:59 AM, Ján Tomko wrote:
 ---
  docs/index.html.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/docs/index.html.in b/docs/index.html.in
 index da40d7a..bb03d65 100644
 --- a/docs/index.html.in
 +++ b/docs/index.html.in
 @@ -4,7 +4,7 @@
  h1Libvirt-php/h1
  
  plibvirt-php, originally called php-libvirt, is a project that was 
 started by Radek Hladik in 2010 to integrate libvirt support to PHP./p
 -pIn February 2011 the binding has been moved to libvirt.org site and 
 it's currently maintained by Red Hat./p
 +pIn February 2011 the binding was moved to libvirt.org./p
  p
 bThis project is not affiliated with The PHP Group and the PHP 
 project itself./b
  /p
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 16/35] qemu: Add helper to update domain balloon size and refactor usage places

2015-06-04 Thread Peter Krempa
On Thu, Jun 04, 2015 at 07:10:58 -0400, John Ferlan wrote:
 
 
 On 06/03/2015 09:16 AM, Ján Tomko wrote:
  On Fri, May 29, 2015 at 03:33:37PM +0200, Peter Krempa wrote:
  When qemu does not support the balloon event the current memory size
  needs to be queried. Since there are two places that implement the same
  logic, split it out into a function and reuse.
  ---
   src/qemu/qemu_domain.c | 64 ++
   src/qemu/qemu_domain.h |  3 ++
   src/qemu/qemu_driver.c | 84 
  +-
   3 files changed, 75 insertions(+), 76 deletions(-)
 
  diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
  index db8554b..661181f 100644
  --- a/src/qemu/qemu_domain.c
  +++ b/src/qemu/qemu_domain.c
  @@ -3182,3 +3182,67 @@ qemuDomainMachineIsI440FX(const virDomainDef *def)
   STRPREFIX(def-os.machine, pc-i440) ||
   STRPREFIX(def-os.machine, rhel));
   }
  +
  +
  +/**
  + * qemuDomainUpdateCurrentMemorySize:
  + *
  + * Updates the current balloon size from the monitor if necessary. In 
  case when
  + * the balloon is not present for the domain, the function recalculates 
  the
  + * maximum size to reflect possible changes.
  + *
  + * Returns 0 on success and updates vm-def-mem.cur_balloon if 
  necessary, -1 on
  + * error and reports libvirt error.
  + */
  +int
  +qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
  +  virDomainObjPtr vm)
  +{
  +qemuDomainObjPrivatePtr priv = vm-privateData;
  +unsigned long long balloon;
  +int ret = -1;
  +
  +/* inactive domain doesn't need size update */
  +if (!virDomainObjIsActive(vm))
  +return 0;
  +
  +/* if no balloning is available, the current size equals to the 
  current
  + * full memory size */
  +if (!vm-def-memballoon ||
  +vm-def-memballoon-model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
  +vm-def-mem.cur_balloon = virDomainDefGetMemoryActual(vm-def);
  +return 0;
  +}
  +
  +/* current size is always automagically updated via the event */
  +if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BALLOON_EVENT))
  +return 0;
  +
  +/* here we need to ask the monitor */
  +
  +/* Don't delay if someone's using the monitor, just use existing most
  + * recent data instead */
  +if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {
  +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY)  0)
  +return -1;
  +
  +if (!virDomainObjIsActive(vm)) {
  +virReportError(VIR_ERR_OPERATION_INVALID, %s,
  +   _(domain is not running));
  +goto endjob;
  +}
  +
  +qemuDomainObjEnterMonitor(driver, vm);
  +ret = qemuMonitorGetBalloonInfo(priv-mon, balloon);
  +if (qemuDomainObjExitMonitor(driver, vm)  0)
  +ret = -1;
  +
  + endjob:
  +qemuDomainObjEndJob(driver, vm);
  +
  +if (ret  0)
  +return -1;
  
  ACK if you actually use the 'balloon' value to update cur_balloon here.
  
  Jan
  
 
 Making Coverity unhappy...
 
 3215  qemuDomainObjPrivatePtr priv = vm-privateData;
 
 (1) Event var_decl:   Declaring variable balloon without initializer.
 Also see events:  [uninit_use]
 
 3216  unsigned long long balloon;
 
 ...
 238* recent data instead */
 
 (9) Event cond_false: Condition qemuDomainJobAllowed(priv,
 QEMU_JOB_QUERY), taking false branch
 
 3239  if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {
 
 ...
 
 3258  return -1;
 
 (10) Event if_end:End of if statement
 
 3259  }
 3260  
 
 (11) Event uninit_use:Using uninitialized value balloon.
 Also see events:  [var_decl]
 
 3261  vm-def-mem.cur_balloon = balloon;
 3262  
 
 
 So if QEMU_JOB_QUERY is not allowed, then balloon is not initialized and
 setting to cur_balloon is bad
 
 Adding an if (ret == 0) prior to the setting works.

I intended to put the assignment inside of the block, rather than
outside. I'll move it inside.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: Fix invalid pointer check in virDomainObjGetDefs

2015-06-04 Thread Peter Krempa
On Thu, Jun 04, 2015 at 07:44:20 -0400, John Ferlan wrote:
 
 
 On 06/04/2015 07:28 AM, Peter Krempa wrote:
  Coverity rightfully determined that in commit 3d021381c71221e563182f03
  I made a mistake in the first check if @persDef is not NULL is
  dereferencing it rather than checking.
  ---
  
  Pushed as trivial.
  
   src/conf/domain_conf.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
  index 77e198c..68d28f6 100644
  --- a/src/conf/domain_conf.c
  +++ b/src/conf/domain_conf.c
  @@ -2927,7 +2927,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
   if (liveDef)
   *liveDef = NULL;
  
  -if (*persDef)
  +if (persDef)
   *persDef = NULL;
  
   if (virDomainObjUpdateModificationImpact(vm, flags)  0)
  
 
 
 yes - this makes Coverity happy... Still curious about :
 
 +if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 +if (liveDef)
 +*liveDef = vm-def;
 +
 +if (persDef)
 +*liveDef = vm-newDef;
 
 
 Coverity doesn't flag the second *liveDef setting, but from just reading
 the code it seems if NULL is passed for liveDef and something is passed
 for persDef, then we could run into an issue here deref'ing liveDef
 

Yup, I didn't originally notice this in the previous mail. I've already
prepared a fix for this.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] conf: Fix mistakes in pointer usage in virDomainObjGetDefs

2015-06-04 Thread Peter Krempa
Coverity rightfully determined that in commit 3d021381c71221e563182f03
I made a mistake in the first check if @persDef is not NULL is
dereferencing it rather than checking.

Additionally if the vm is online the code would set @liveDef twice
rather than modifying @persDef. Fix both mistakes.
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 77e198c..36de844 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2927,7 +2927,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
 if (liveDef)
 *liveDef = NULL;

-if (*persDef)
+if (persDef)
 *persDef = NULL;

 if (virDomainObjUpdateModificationImpact(vm, flags)  0)
@@ -2938,7 +2938,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
 *liveDef = vm-def;

 if (persDef)
-*liveDef = vm-newDef;
+*persDef = vm-newDef;
 } else {
 if (persDef)
 *persDef = vm-def;
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] conf: Fix invalid pointer check in virDomainObjGetDefs

2015-06-04 Thread Peter Krempa
Coverity rightfully determined that in commit 3d021381c71221e563182f03
I made a mistake in the first check if @persDef is not NULL is
dereferencing it rather than checking.
---

Pushed as trivial.

 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 77e198c..68d28f6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2927,7 +2927,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
 if (liveDef)
 *liveDef = NULL;

-if (*persDef)
+if (persDef)
 *persDef = NULL;

 if (virDomainObjUpdateModificationImpact(vm, flags)  0)
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] conf: Fix invalid pointer check in virDomainObjGetDefs

2015-06-04 Thread John Ferlan


On 06/04/2015 07:28 AM, Peter Krempa wrote:
 Coverity rightfully determined that in commit 3d021381c71221e563182f03
 I made a mistake in the first check if @persDef is not NULL is
 dereferencing it rather than checking.
 ---
 
 Pushed as trivial.
 
  src/conf/domain_conf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 77e198c..68d28f6 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -2927,7 +2927,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
  if (liveDef)
  *liveDef = NULL;
 
 -if (*persDef)
 +if (persDef)
  *persDef = NULL;
 
  if (virDomainObjUpdateModificationImpact(vm, flags)  0)
 


yes - this makes Coverity happy... Still curious about :

+if (flags  VIR_DOMAIN_AFFECT_LIVE) {
+if (liveDef)
+*liveDef = vm-def;
+
+if (persDef)
+*liveDef = vm-newDef;


Coverity doesn't flag the second *liveDef setting, but from just reading
the code it seems if NULL is passed for liveDef and something is passed
for persDef, then we could run into an issue here deref'ing liveDef

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] conf: Fix invalid pointer check in virDomainObjGetDefs

2015-06-04 Thread Peter Krempa
On Thu, Jun 04, 2015 at 07:44:20 -0400, John Ferlan wrote:
 
 
 On 06/04/2015 07:28 AM, Peter Krempa wrote:
  Coverity rightfully determined that in commit 3d021381c71221e563182f03
  I made a mistake in the first check if @persDef is not NULL is
  dereferencing it rather than checking.
  ---
  
  Pushed as trivial.
  
   src/conf/domain_conf.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
  index 77e198c..68d28f6 100644
  --- a/src/conf/domain_conf.c
  +++ b/src/conf/domain_conf.c
  @@ -2927,7 +2927,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
   if (liveDef)
   *liveDef = NULL;
  
  -if (*persDef)
  +if (persDef)
   *persDef = NULL;
  
   if (virDomainObjUpdateModificationImpact(vm, flags)  0)
  
 
 
 yes - this makes Coverity happy... Still curious about :
 
 +if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 +if (liveDef)
 +*liveDef = vm-def;
 +
 +if (persDef)
 +*liveDef = vm-newDef;
 
 
 Coverity doesn't flag the second *liveDef setting, but from just reading
 the code it seems if NULL is passed for liveDef and something is passed
 for persDef, then we could run into an issue here deref'ing liveDef

I actually did not push the patch yet, so I'll squash this fix to that
one too.

 
 John
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] qemu: Update balloon info only if job is allowed

2015-06-04 Thread Peter Krempa
In qemuDomainUpdateCurrentMemorySize I misplaced the actual update of
the balloon size to a place where it may not be initialized. Move it a
few lines above.
---
 src/qemu/qemu_domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3826b2f..0682390 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3256,9 +3256,9 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,

 if (ret  0)
 return -1;
-}

-vm-def-mem.cur_balloon = balloon;
+vm-def-mem.cur_balloon = balloon;
+}

 return 0;
 }
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv2 0/2] Fix code after the vcpupin series

2015-06-04 Thread John Ferlan


On 06/04/2015 08:06 AM, Peter Krempa wrote:
 Both patches are pushed as trivial.
 
 Peter Krempa (2):
   conf: Fix mistakes in pointer usage in virDomainObjGetDefs
   qemu: Update balloon info only if job is allowed
 
  src/conf/domain_conf.c | 4 ++--
  src/qemu/qemu_domain.c | 4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)
 

Confirm that the Coverity issues from these changes are resolved


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/4] conf: Ignore multiqueue with one queue.

2015-06-04 Thread Martin Kletzander
Multi != One.  And indeed, libvirt behaves the same way for queues='1'
as without such setting.  Let's make it clear in the XML.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/conf/domain_conf.c  | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 6 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 6 ++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 36de8441990e..2e7961001090 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8626,7 +8626,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
queues);
 goto error;
 }
-def-driver.virtio.queues = q;
+if (q  1)
+def-driver.virtio.queues = q;
 }
 if ((str = virXPathString(string(./driver/host/@csum), ctxt))) {
 if ((val = virTristateSwitchTypeFromString(str)) = 0) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
index 2cf312f0ca53..28f93474136e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
@@ -28,6 +28,12 @@
   driver name='vhost' queues='5'/
   backend tap='/dev/null' vhost='/dev/zero'/
 /interface
+interface type='user'
+  mac address='52:54:00:e5:48:59'/
+  model type='virtio'/
+  driver name='vhost' queues='1'/
+  backend tap='/dev/null' vhost='/dev/zero'/
+/interface
 serial type='pty'
   target port='0'/
 /serial
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
index 266cbf0a72b8..d419cc3b8e15 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
@@ -27,6 +27,12 @@
   model type='definitely-not-virtio'/
   backend tap='/dev/null'/
 /interface
+interface type='user'
+  mac address='52:54:00:e5:48:59'/
+  model type='virtio'/
+  driver name='vhost'/
+  backend tap='/dev/null' vhost='/dev/zero'/
+/interface
 serial type='pty'
   target port='0'/
 /serial
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/4] Add support for vhost-user with multi-queue

2015-06-04 Thread Martin Kletzander
Also some tiny clean-up.

Martin Kletzander (2):
  conf: Ignore multiqueue with one queue.
  qemu: Add capability for vhost-user multiqueue

Maxime Leroy (2):
  docs: Clarify that attribute name is not used for vhostuser
  qemu: add multiqueue vhost-user support

 docs/formatdomain.html.in| 16 ++--
 src/conf/domain_conf.c   |  3 ++-
 src/qemu/qemu_capabilities.c |  6 ++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  | 15 ++-
 ...tuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
 ...ostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
 .../qemuxml2argv-tap-vhost-incorrect.xml |  6 ++
 tests/qemuxml2argvtest.c |  3 +++
 .../qemuxml2xmlout-tap-vhost-incorrect.xml   |  6 ++
 10 files changed, 63 insertions(+), 5 deletions(-)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
qemuxml2argv-net-vhostuser-multiq.args} (75%)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
qemuxml2argv-net-vhostuser-multiq.xml} (87%)

--
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 3/4] qemu: Add capability for vhost-user multiqueue

2015-06-04 Thread Martin Kletzander
The support for this was added in QEMU with commit
830d70db692e374b5f4407f96a1ceefdcc97.  Unfortunately we have to do
another ugly version-based capability check.  The other option would be
not to check for the capability at all and leave that to qemu as it's
doen with multiqueue tap devices.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_capabilities.c | 6 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 960afa4ac0db..f102ed80f15e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -284,6 +284,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   aes-key-wrap,
   dea-key-wrap,
   pci-serial,
+  vhost-user-multiq,
 );


@@ -3283,6 +3284,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (qemuCaps-version = 2002000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);

+/* vhost-user supports multi-queue from v2.4.0 onwards,
+ * but there is no way to query for that capability */
+if (qemuCaps-version = 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ);
+
 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon)  0)
 goto cleanup;
 if (virQEMUCapsProbeQMPEvents(qemuCaps, mon)  0)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9c956f3007be..3dbd767f2516 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -228,6 +228,7 @@ typedef enum {
 QEMU_CAPS_AES_KEY_WRAP   = 186, /* -machine aes_key_wrap */
 QEMU_CAPS_DEA_KEY_WRAP   = 187, /* -machine dea_key_wrap */
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
+QEMU_CAPS_VHOSTUSER_MULTIQ   = 189, /* vhost-user with -netdev queues= */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 4/4] qemu: add multiqueue vhost-user support

2015-06-04 Thread Martin Kletzander
From: Maxime Leroy maxime.le...@6wind.com

This patch adds the support of queues attribute of the driver element
for vhost-user interface type. Example:

interface type='vhostuser'
  mac address='52:54:00:ee:96:6d'/
  source type='unix' path='/tmp/vhost2.sock' mode='client'/
  model type='virtio'/
  driver queues='4'/
/interface

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in | 11 +--
 src/qemu/qemu_command.c   | 15 ++-
 ...stuser.args = qemuxml2argv-net-vhostuser-multiq.args} |  6 +-
 ...hostuser.xml = qemuxml2argv-net-vhostuser-multiq.xml} |  6 ++
 tests/qemuxml2argvtest.c  |  3 +++
 5 files changed, 37 insertions(+), 4 deletions(-)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.args = 
qemuxml2argv-net-vhostuser-multiq.args} (75%)
 copy tests/qemuxml2argvdata/{qemuxml2argv-net-vhostuser.xml = 
qemuxml2argv-net-vhostuser-multiq.xml} (87%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 72ad54cee188..85238a16af8d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4260,7 +4260,8 @@ qemu-kvm -net nic,model=? /dev/null
 type='virtio'/gt;/code, multiple packet processing queues can be
 created; each queue will potentially be handled by a different
 processor, resulting in much higher throughput.
-span class=sinceSince 1.0.6 (QEMU and KVM only)/span
+span class=sinceSince 1.0.6 (QEMU and KVM only) and for vhost-user
+  since 1.2.17/span
   /dd
   dtcodehost/code offloading options/dt
   dd
@@ -4581,9 +4582,15 @@ qemu-kvm -net nic,model=? /dev/null
   lt;devicesgt;
 lt;interface type='vhostuser'gt;
   lt;mac address='52:54:00:3b:83:1a'/gt;
-  lt;source type='unix' path='/tmp/vhost.sock' mode='server'/gt;
+  lt;source type='unix' path='/tmp/vhost1.sock' mode='server'/gt;
   lt;model type='virtio'/gt;
 lt;/interfacegt;
+lt;interface type='vhostuser'gt;
+  lt;mac address='52:54:00:3b:83:1b'/gt;
+  lt;source type='unix' path='/tmp/vhost2.sock' mode='client'/gt;
+  lt;model type='virtio'/gt;
+  lt;driver queues='5'/gt;
+lt;/interfacegt;
   lt;/devicesgt;
   .../pre

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 61faa576e11b..f805f6700e71 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8089,6 +8089,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 {
 virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
 virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
+unsigned int queues = 1;
 char *nic = NULL;

 if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
@@ -8126,13 +8127,25 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
 virBufferAsprintf(netdev_buf, type=vhost-user,id=host%s,chardev=char%s,
   net-info.alias, net-info.alias);

+queues = net-driver.virtio.queues;
+if (queues) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(multi-queue is not supported for vhost-user 
+ with this QEMU binary));
+goto error;
+}
+virBufferAsprintf(netdev_buf, ,queues=%u, queues);
+}
+
 virCommandAddArg(cmd, -chardev);
 virCommandAddArgBuffer(cmd, chardev_buf);

 virCommandAddArg(cmd, -netdev);
 virCommandAddArgBuffer(cmd, netdev_buf);

-if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, 0, qemuCaps))) {
+if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex,
+   queues, qemuCaps))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(Error generating NIC -device string));
 goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
similarity index 75%
copy from tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
copy to tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
index ac43630979ad..8affb53b3958 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
@@ -9,4 +9,8 @@ pc -m 214 -smp 1 -nographic -nodefaults -monitor 
unix:/tmp/test-monitor,server,n
 -netdev type=vhost-user,id=hostnet1,chardev=charnet1 \
 -device 
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,addr=0x4 
\
 -netdev socket,listen=:2015,id=hostnet2 \
--device 
rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,addr=0x5
+-device 
rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,addr=0x5 \
+-chardev socket,id=charnet3,path=/tmp/vhost2.sock \
+-netdev 

[libvirt] [PATCH 2/4] docs: Clarify that attribute name is not used for vhostuser

2015-06-04 Thread Martin Kletzander
From: Maxime Leroy maxime.le...@6wind.com

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in | 5 +
 1 file changed, 5 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 178199679ed3..72ad54cee188 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4190,6 +4190,11 @@ qemu-kvm -net nic,model=? /dev/null
 span class=sinceSince 1.0.5 (QEMU and KVM only, requires
 kernel 3.6 or newer)/span
   /dd
+  dd
+For interfaces of type='vhostuser', the codename/code
+attribute is ignored. The backend driver used is always
+vhost-user.
+  /dd

   dtcodetxmode/code/dt
   dd
-- 
2.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list