Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration
-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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
--- 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
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'
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
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()
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'
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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