Re: [libvirt] [PATCH 0/3] storage: zfs: implement download and upload

2014-08-25 Thread Roman Bogorodskiy
  Michal Privoznik wrote:

 On 15.08.2014 10:44, Roman Bogorodskiy wrote:
 
  Roman Bogorodskiy (3):
 fdstream: report error if virSetNonBlock fails
 fdstream: introduce virFDStreamOpenBlockDevice
 storage: zfs: implement download and upload
 
src/fdstream.c| 30 +++---
src/fdstream.h|  5 +
src/libvirt_private.syms  |  1 +
src/storage/storage_backend.c |  6 --
src/storage/storage_backend_zfs.c |  2 ++
5 files changed, 35 insertions(+), 9 deletions(-)
 
 
 ACK to all the patches, if you work in Dan's comment in 2/3.

Updated commit message in 2/3 and pushed the series; thanks!

Roman Bogorodskiy


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

[libvirt] [PATCH] fix mingw build

2014-08-25 Thread Pavel Hrdina
The commit f5b4c141 introduced new force parameter
for virFDStreamOpenFileInternal but forget to update
one call of that function.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

Pushed as build breaker.

 src/fdstream.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index 4cf152a..9ff7e2a 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -769,7 +769,8 @@ int virFDStreamOpenPTY(virStreamPtr st,
 {
 return virFDStreamOpenFileInternal(st, path,
offset, length,
-   oflags | O_CREAT, 0);
+   oflags | O_CREAT, 0,
+   false);
 }
 #endif /* !HAVE_CFMAKERAW */
 
-- 
1.8.5.5

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


Re: [libvirt] [PATCH] fix mingw build

2014-08-25 Thread Roman Bogorodskiy
  Pavel Hrdina wrote:

 The commit f5b4c141 introduced new force parameter
 for virFDStreamOpenFileInternal but forget to update
 one call of that function.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
 
 Pushed as build breaker.
 
  src/fdstream.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Sorry about that and thanks for the fix!

Roman Bogorodskiy


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

Re: [libvirt] [RFCv2] Introduce API for retrieving bulk domain stats v2

2014-08-25 Thread Francesco Romani
- Original Message -
 From: Peter Krempa pkre...@redhat.com
 To: libvir-list@redhat.com
 Cc: ebl...@redhat.com, berra...@redhat.com, from...@redhat.com, Peter 
 Krempa pkre...@redhat.com
 Sent: Thursday, August 21, 2014 3:20:45 PM
 Subject: [RFCv2] Introduce API for retrieving bulk domain stats v2
 
 I'd like to propose a (hopefully) fairly future-proof API to retrieve
 various statistics for domains.
 
 The motivation is that management layers that use libvirt usually poll
 libvirt for statistics using various split up APIs we currently provide.
 To get all the necessary stuff, the mgmt app need to issue Ndomains *
 Napis calls and cope with the various returned formats. The APIs I'm
 wanting to introduce here will:
 
 1) Return data in a format that we can expand in the future and is
 hierarchical. This version returns the data as typed parameters where
 the fields are constructed as dot-separated strings containing names and
 other stuff in a list of typed params.
 
 2) Stats for multiple (all) domains can be queried at once and are
 returned in one call. This will allow to decrease the overhead necessary
 to issue multiple calls per domain multiplied by the count of domains.
 
 3) Selectable (bit mask) fields in the returned format. This will allow
 to retrieve only specific stats according to the APPs need.
 
 Initially the implementation will introduce the option to retrieve
 block, interface  and cpu stats with the possibility to add more in the
 future.
 
 The stats groups will be enabled using a bit field @stats passed as the
 function argument. A few groups for inspiration:
 
 VIR_DOMAIN_STATS_STATE
 VIR_DOMAIN_STATS_CPU
 VIR_DOMAIN_STATS_BLOCK
 VIR_DOMAIN_STATS_INTERFACE
 
 the returned typed params will use the following scheme
 
 state.state = running
 state.reason = started
 cpu.count = 8
 cpu.0.state = running
 cpu.0.time = 1234

OK for me

 +typedef struct _virDomainStatsRecord virDomainStatsRecord;
 +typedef virDomainStatsRecord *virDomainStatsRecordPtr;
 +struct _virDomainStatsRecord {
 +virDomainPtr dom;
 +unsigned int nparams;
 +virTypedParameterPtr params;
 +};
 +
 +typedef enum {
 +VIR_DOMAIN_STATS_ALL = (1  0), /* return all stats fields
 +   implemented in the daemon */
 +VIR_DOMAIN_STATS_STATE = (1  1), /* return domain state */
 +} virDomainStatsTypes;
 +
 +int virConnectGetAllDomainStats(virConnectPtr conn,
 +unsigned int stats,
 +virDomainStatsRecordPtr **retStats,
 +unsigned int flags);
 +
 +int virDomainListGetStats(virDomainPtr *doms,
 +  unsigned int stats,
 +  virDomainStatsRecordPtr **retStats,
 +  unsigned int flags);
 +
 +void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
 +

Minor question:
Would it be possible, maybe on a future extension, for the caller to
preallocate the virDomainStatsPtr output records?

Thanks and bests,

-- 
Francesco Romani
RedHat Engineering Virtualization R  D
Phone: 8261328
IRC: fromani

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


Re: [libvirt] [PATCH] libvirt: lxc: Add Get/Set vcpus for lxc

2014-08-25 Thread Wang Rui
On 2014/8/22 17:50, Li Yang wrote:
 1.Add function to get vcpu count for lxc(vcpucount)
 2.Add function to set vcpu count for lxc(setvcpus)
 
 Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
 ---
  src/lxc/lxc_driver.c |  159 
 ++
  1 files changed, 159 insertions(+), 0 deletions(-)

Does def-vcpus affect anything?
No matter how much vcpus I set in xml , it seems that the vcpu
count in container is equal to the host pcpu count.

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


[libvirt] [PATCHv2] iotune: setting an invalid value now reports error

2014-08-25 Thread Erik Skultety
When trying to set an invalid value into iotune element, standard
behavior was to not report any error, rather to reset all affected
subelements of the iotune element back to 0 which results in ignoring
those particular subelements by XML generator. Patch further
examines the return code of the virXPathULongLong function
and in case of an invalid non-integer value raises an error.
Fixed to preserve consistency with invalid value checking
of other elements.

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811
---
 src/conf/domain_conf.c | 67 --
 1 file changed, 49 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9557020..bf8d30c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5416,6 +5416,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *mirrorType = NULL;
 int expected_secret_usage = -1;
 int auth_secret_usage = -1;
+int ret = 0;
 
 if (!(def = virDomainDiskDefNew()))
 return NULL;
@@ -5644,39 +5645,69 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }
 } else if (xmlStrEqual(cur-name, BAD_CAST iotune)) {
-if (virXPathULongLong(string(./iotune/total_bytes_sec),
-  ctxt,
-  def-blkdeviotune.total_bytes_sec)  0) 
{
+ret = virXPathULongLong(string(./iotune/total_bytes_sec),
+ctxt,
+def-blkdeviotune.total_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(total throughput limit must be an 
integer));
+goto error;
+} else if (ret  0) {
 def-blkdeviotune.total_bytes_sec = 0;
 }
 
-if (virXPathULongLong(string(./iotune/read_bytes_sec),
-  ctxt,
-  def-blkdeviotune.read_bytes_sec)  0) {
+ret = virXPathULongLong(string(./iotune/read_bytes_sec),
+ctxt,
+def-blkdeviotune.read_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(read throughput limit must be an 
integer));
+goto error;
+} else if (ret  0) {
 def-blkdeviotune.read_bytes_sec = 0;
 }
 
-if (virXPathULongLong(string(./iotune/write_bytes_sec),
-  ctxt,
-  def-blkdeviotune.write_bytes_sec)  0) 
{
+ret = virXPathULongLong(string(./iotune/write_bytes_sec),
+ctxt,
+def-blkdeviotune.write_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(write throughput limit must be an 
integer));
+goto error;
+} else if (ret  0) {
 def-blkdeviotune.write_bytes_sec = 0;
 }
 
-if (virXPathULongLong(string(./iotune/total_iops_sec),
-  ctxt,
-  def-blkdeviotune.total_iops_sec)  0) {
+ret = virXPathULongLong(string(./iotune/total_iops_sec),
+ctxt,
+def-blkdeviotune.total_iops_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(total I/O operations limit must be an 
integer));
+goto error;
+} else if (ret  0) {
 def-blkdeviotune.total_iops_sec = 0;
 }
 
-if (virXPathULongLong(string(./iotune/read_iops_sec),
-  ctxt,
-  def-blkdeviotune.read_iops_sec)  0) {
+ret = virXPathULongLong(string(./iotune/read_iops_sec),
+ctxt,
+def-blkdeviotune.read_iops_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(read I/O operations limit must be an 
integer));
+goto error;
+} else if (ret  0) {
 def-blkdeviotune.read_iops_sec = 0;
 }
 
-if (virXPathULongLong(string(./iotune/write_iops_sec),
-  ctxt,
- 

Re: [libvirt] [Libvirt] segfault with patch libxl: fix framebuffer port setting for HVM domains

2014-08-25 Thread Ján Tomko
On 08/25/2014 01:39 AM, Chris wrote:
 Hello,
 
 I encountered segfaults with libvirt + libxl with this specific patch :
 http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b55cc5f4e31b488c4f9c3c8470c992c1f8f5d09c
 
 My Libvirt/Xen environment (on a slackware64-14.1) :
 
 virsh # version
 Compiled against library: libvirt 1.2.6
 Using library: libvirt 1.2.6
 Using API: Xen 1.2.6
 Running hypervisor: Xen 4.4.0
 

 
 The VM starts fine with virsh start (cf attached libxl log), the VNC 
 process is listening as requested on 0.0.0.0 but whenever I open a new virsh 
 cli and send a command (e.g. list), libvirtd segfaults. I attached a gdb 
 trace to this mail.
 If I rebuild libvirt without the patch mentioned earlier, no more segfaults, 
 but of course the VNC process is listening on 127.0.0.1 instead of requested 
 0.0.0.0 when I start the VM.
 

The trace shows SIGSEGV in malloc_consolidate, which usually means we touched
some memory we shouldn't have and overwritten malloc's internal data.

Could you run libvirtd under valgrind and see if it shows any invalid
reads/writes?

Jan



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

[libvirt] 答复: [PATCH] libvirt: lxc: Add Get/Set vcpus for lxc

2014-08-25 Thread Li, Yang
On 2014/8/22 17:50, Li Yang wrote:
 1.Add function to get vcpu count for lxc(vcpucount) 2.Add function to 
 set vcpu count for lxc(setvcpus)
 
 Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
 ---
  src/lxc/lxc_driver.c |  159
 ++
  1 files changed, 159 insertions(+), 0 deletions(-)

Does def-vcpus affect anything?
No matter how much vcpus I set in xml , it seems that the vcpu count in 
container is equal to the host pcpu count.

Thanks for your respond.
Yes, the vcpus in  xml doesn't  affect container internal vcpu count, I wrote 
these functions because I
 saw `virsh setmem/setmaxmem` has already been working on domain's xml although 
it doesn't 
affect container internal memory information.

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


Re: [libvirt] Automatically affinitize hostdev interrupts to vm vCpus (qemu/kvm)

2014-08-25 Thread Martin Kletzander

On Mon, Aug 18, 2014 at 07:01:40PM +, Mooney, Sean K wrote:

Hi
I would like to ask for comments and propose a possible new libvirt feature.



Hi, could you convince your e-mail agent to wrap long lines?


Problem statement:
At present when you boot a vm via libvirt it is possible  to pin vm vCPUs to 
host CPUs to improve performance in the  guest under certain conditions.
If hostdev interrupts are not pined to a specific core/cpuset suboptimal 
processing of irqs may occur reducing the performance of both guest and host.
I would like to propose extending libvirt to automatically pin interrupts for 
hostdev devices if they are present in the guest.

By affinitizing interrupts, cache line sharing between the specified interrupt 
and guest can be achieved.
If CPU affinity for the guest, as set by the cpuset parameter,
is intelligently chosen to place the guest on the same numa node as the 
hostdev, cross socket traffic can be mitigated.
As a result, latency which would be introduce if the interrupt processing was 
scheduled to a non-local numa node CPU can be reduced via interrupt pinning.

Proposed change:

* util/virpci and util/virhostdev will be extended to retrieve IRQ and 
msi_interupt information from sysfs.

* util/virinterupt  will be created

* util/virinterupt  will implement managing interrupt affinity via 
/proc/irq/x/smp_affinity



Nice that this will be abstracted out in a separate file, although if
it's not huge, it can be part of something else.


* qemuProcessInitCpuAffinity will be extended to conditionally 
affinitize hostdev interrupts to vm vCpus when hostdevs are present in the vm 
definition.


So it will only affect hostdevs, OK, that means there should be less
(or no) disadvantages).  Although beware that hostdevs can be
plugged/unplugged, number of vCPUs can be changed and, most
importantly, the affinities (either set with sched_set_affinity or
using cgroups) can be changed during the lifetime of the VM, and the
smp_affinity of each IRQ should track such changes.  Needless to say
the affinity should be restored after the machine dies/is turned off,
etc., not just on hostdev unplug.


Alternative implementation:

In addition to the above changes the hostdev element could be extended to 
include a cpuset attribute:

* if the cpuset is auto: the interrupts would be pinned to the same 
cpuset as the vms vCPU

* if a cpuset is specified: the interrupts would be affinitized as per 
the set cpuset

* if the cpuset is native: the interrupts will not be pinned and the 
current behaviour will not be changed.

* If the cpuset attribute is not present either the behaviour of auto 
or native could be used as a default.

o   Using auto as the default would allow transparent use of the feature.



I like defaulting to auto also because the transparent use will have
effect only in existing deployments with vCPU pinning.


o   Using native as the default would allow no changes to any existing 
deployments unless the feature is requested.


Although this version might be preferred by others.  This can,
however, be discussed (and changed) during reviews.


Any feedback is welcomed.
Regards
Sean.
--
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). Any review or distribution by others is 
strictly prohibited. If you are not the intended recipient, please contact the 
sender and delete all copies.



Since this mailing-list is public and archived, such disclaimer is
unenforceable.  As an upstream project, the development is done in the
open.  Such statements as the above one may be the cause of nobody
replying for some time to your e-mail.

The statement itself can be understood that even delivering the mail
through a list is prohibited.  Please consider removing such
statements in following e-mails (or use private e-mail address if the
disclaimer is enforced by your employer) as keeping them may result in
the rest of the community not being able to communicate with you.

Martin


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

Re: [libvirt] Mentors wanted for Outreach Program for Women October 2014

2014-08-25 Thread Martin Kletzander

On Thu, Aug 21, 2014 at 09:06:39PM +0100, Stefan Hajnoczi wrote:

Dear mentors and core contributors,
Outreach Program for Women is starting the next round in October 2014.
OPW funds women to work on open source software for 12 weeks with the
help of mentors:
https://wiki.gnome.org/OutreachProgramForWomen/

We just completed a successful round of OPW and Google Summer of Code.
Other organizations have also been participating successfully in OPW
like the Linux kernel with Greg KH and other mentors.

Would you like to mentor in OPW October 2014?



I could use some of my time to help others participate in the
community.


Regular code contributors to QEMU, KVM, and libvirt are eligible to
participate as mentors.

We also need project ideas that are achievable in 12 weeks by someone
skilled in programming but not necessarily familiar with open source
or our codebase.  Ideas welcome!



It's just a matter of ideas.  Maybe we could revisit some of those we
had for GSoC.  If I'm reading the deadline for project ideas is
October 22nd, so I think we'll definitely come up with something.

In first pitch this might be a rewriting of storage driver to handle
jobs (our failed GSoC project from this year), and if admin API gets
added, there will be many APIs and ideas how to expand it.

Martin


Stefan



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

Re: [libvirt] [PATCH 3/3] qemu: add support for splash-timeout

2014-08-25 Thread Ján Tomko
On 08/22/2014 05:10 PM, Martin Kletzander wrote:
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1021703
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/qemu/qemu_command.c   | 15 
 +++
  .../qemuxml2argv-boot-menu-enable-with-timeout.args   | 15 
 +++
  tests/qemuxml2argvtest.c  |  4 
  3 files changed, 34 insertions(+)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 0d7b12d..bb1c423 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -7927,6 +7927,21 @@ qemuBuildCommandLine(virConnectPtr conn,
def-os.bios.rt_delay);
  }
 
 +if (def-os.bm_timeout_set) {
 +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(splash timeout is not supported 
 + by this QEMU binary));
 +virBufferFreeAndReset(boot_buf);
 +goto error;
 +}
 +
 +if (boot_nparams++)
 +virBufferAddChar(boot_buf, ',');
 +
 +virBufferAsprintf(boot_buf, splash-time=%d, 
 def-os.bm_timeout);

This should be %u for usigned int.

Also, it would be nice to get rid of 'boot_nparams', but that's out of scope
of this patch.

ACK series

Jan



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

[libvirt] [PATCH] daemon: Fix option -v missing info priority log

2014-08-25 Thread Wang Rui
From: Zhou Yimin zhouyi...@huawei.com

Introduce by 63fbcc692.

When start libvirtd with commandline /usr/sbin/libvirtd -d -l -v,
we expect verbose(info level) log if neither environment variable
nor config file about logging controls is set. But in fact we can't
get any info priority log in the default output file.

The log priority of default output is VIR_LOG_DEFAULT(VIR_LOG_WARN),
so the info log is filtered out.
To record info priority log we must parse option -v before setting the
default output.

After this patch, we get all verbose log in the default output file.

Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
 daemon/libvirtd.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 4cf78e6..87af903 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -683,6 +683,12 @@ daemonSetupLogging(struct daemonConfig *config,
 virLogParseOutputs(config-log_outputs);
 
 /*
+ * Command line override for --verbose
+ */
+if ((verbose)  (virLogGetDefaultPriority()  VIR_LOG_INFO))
+virLogSetDefaultPriority(VIR_LOG_INFO);
+
+/*
  * If no defined outputs, and either running
  * as daemon or not on a tty, then first try
  * to direct it to the systemd journal
@@ -748,12 +754,6 @@ daemonSetupLogging(struct daemonConfig *config,
 VIR_FREE(tmp);
 }
 
-/*
- * Command line override for --verbose
- */
-if ((verbose)  (virLogGetDefaultPriority()  VIR_LOG_INFO))
-virLogSetDefaultPriority(VIR_LOG_INFO);
-
 return 0;
 
  error:
-- 
1.7.12.4


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


Re: [libvirt] Plan for the next release

2014-08-25 Thread Peter Krempa
On 08/20/14 13:26, Daniel Veillard wrote:
   We already have a number of commits in, over 150, I can't freeze next
 monday but I could do this next Tuesday (the 26) for a planned release
 around Sep 1st,
 
   So unless there is a good reason to change this, that's the plan :)

Well there are a few new API designs either posted or in progress which
we'd like to get into this release, so we'd appreciate a day or two
extra to finish stuff up.


Thanks in advance.

 
 Daniel
 

Peter




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

Re: [libvirt] [PATCH] blockcopy: allow block device destination

2014-08-25 Thread Adam Litke

On 22/08/14 14:26 -0600, Eric Blake wrote:

Ping


Sorry, I thought I'd responded to this.


On 08/13/2014 02:00 PM, Eric Blake wrote:

To date, anyone performing a block copy and pivot ends up with
the destination being treated as disk type='file'.  While this
works for data access for a block device, it has at least one
noticeable shortcoming: virDomainGetBlockInfo() reports allocation
differently for block devices visited as files (the size of the
device) than for block devices visited as disk type='block'
(the maximum sector used, as reported by qemu); and this difference
is significant when trying to manage qcow2 format on block devices
that can be grown as needed.

I still plan to add a virDomainBlockCopy() API that takes the
destination disk as XML, allowing full expressive capability
to copy to a network disk.  But a new API can't be backported,
while a new flag to an existing API can.  So this patch enhances
blockcopy to let the user flag when the resulting XML after the
copy must list the device as type='block'.


Is there any situation where you would not want this behavior?  The
only thing I can think of is that we need to keep the current behavior
for backwards compatibility.  If this is the case, then I'd say the
patch looks reasonable to me.


One more question... What happens if this flag is used with a drive of
type file?  Can we raise an error in that case?


* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV):
New flag.
* src/libvirt.c (virDomainBlockRebase): Document it.
* tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add
--blockdev option.
* tools/virsh.pod (blockcopy): Document it.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag.
(qemuDomainBlockCopy): Remember the flag.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 include/libvirt/libvirt.h.in   |2 ++
 src/libvirt.c  |8 ++--
 src/qemu/qemu_driver.c |   12 
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |4 ++--
 tools/virsh-domain.c   |6 ++
 tools/virsh.pod|7 +--
 6 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 47ea695..f2a02ea 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2590,6 +2590,8 @@ typedef enum {
 VIR_DOMAIN_BLOCK_REBASE_RELATIVE  = 1  4, /* Keep backing chain
referenced using relative
names */
+VIR_DOMAIN_BLOCK_REBASE_COPY_DEV  = 1  5, /* Treat destination as block
+   device instead of file */
 } virDomainBlockRebaseFlags;

 int   virDomainBlockRebase(virDomainPtr dom, const char *disk,
diff --git a/src/libvirt.c b/src/libvirt.c
index 992e4f2..c4643e8 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19881,7 +19881,10 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  * pre-create files with relative backing file names, rather than the default
  * of absolute backing file names; as a security precaution, you should
  * generally only use reuse_ext with the shallow flag and a non-raw
- * destination file.
+ * destination file.  By default, the copy destination will be treated as
+ * type='file', but using VIR_DOMAIN_BLOCK_REBASE_COPY_DEV treats the
+ * destination as type='block' (affecting how virDomainGetBlockInfo() will
+ * report allocation after pivoting).
  *
  * A copy job has two parts; in the first phase, the @bandwidth parameter
  * affects how fast the source is pulled into the destination, and the job
@@ -19950,7 +19953,8 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
 virCheckNonNullArgGoto(base, error);
 } else if (flags  (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
 VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
-VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)) {
+VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
+VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
 virReportInvalidArg(flags,
 _(use of flags in %s requires a copy job),
 __FUNCTION__);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6219ba..e74227e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15295,7 +15295,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,

 /* Preliminaries: find the disk we are editing, sanity checks */
 virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
-  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
+  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
+  VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);

 priv = vm-privateData;
 cfg = 

Re: [libvirt] [PATCHv2] iotune: setting an invalid value now reports error

2014-08-25 Thread Ján Tomko
On 08/25/2014 10:50 AM, Erik Skultety wrote:
 When trying to set an invalid value into iotune element, standard
 behavior was to not report any error, rather to reset all affected
 subelements of the iotune element back to 0 which results in ignoring
 those particular subelements by XML generator. Patch further
 examines the return code of the virXPathULongLong function
 and in case of an invalid non-integer value raises an error.
 Fixed to preserve consistency with invalid value checking
 of other elements.
 
 Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811
 ---
  src/conf/domain_conf.c | 67 
 --
  1 file changed, 49 insertions(+), 18 deletions(-)

ACK and pushed.

Jan



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

Re: [libvirt] [PATCH] daemon: Fix option -v missing info priority log

2014-08-25 Thread Ján Tomko
On 08/25/2014 02:18 PM, Wang Rui wrote:
 From: Zhou Yimin zhouyi...@huawei.com
 
 Introduce by 63fbcc692.
 
 When start libvirtd with commandline /usr/sbin/libvirtd -d -l -v,
 we expect verbose(info level) log if neither environment variable
 nor config file about logging controls is set. But in fact we can't
 get any info priority log in the default output file.
 
 The log priority of default output is VIR_LOG_DEFAULT(VIR_LOG_WARN),
 so the info log is filtered out.
 To record info priority log we must parse option -v before setting the
 default output.
 
 After this patch, we get all verbose log in the default output file.
 
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  daemon/libvirtd.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

ACK and pushed.

Jan



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

Re: [libvirt] [PATCH v2] Add new 'kvm' domain feature and ability to hide KVM signature

2014-08-25 Thread Ján Tomko
On 08/21/2014 07:04 PM, Alex Williamson wrote:
 QEMU 2.1 added support for the kvm=off option to the -cpu command,
 allowing the KVM hypervisor signature to be hidden from the guest.
 This enables disabling of some paravirualization features in the
 guest as well as allowing certain drivers which test for the
 hypervisor to load.  Domain XML syntax is as follows:
 
 domain type='kvm
   ...
   features
 ...
 kvm
   hidden state='on'/
 /kvm
   /features
   ...
 
 Signed-off-by: Alex Williamson alex.william...@redhat.com
 ---
 
 Hearing no further comments, here's v2:
  - white space fix in docs/formatdomain.html.in
  - s/virBufferAsprintf/virBufferAddLit/ in src/qemu/qemu_command.c
 
  docs/formatdomain.html.in  |   21 
  docs/schemas/domaincommon.rng  |   18 +++-
  src/conf/domain_conf.c |  100 
 
  src/conf/domain_conf.h |9 ++
  src/qemu/qemu_command.c|   22 
  tests/qemuargv2xmltest.c   |2 
  .../qemuxml2argv-kvm-features-off.args |5 +
  .../qemuxml2argv-kvm-features-off.xml  |   27 +
  .../qemuxml2argv-kvm-features.args |5 +
  .../qemuxml2argvdata/qemuxml2argv-kvm-features.xml |   27 +
  tests/qemuxml2argvtest.c   |3 +
  tests/qemuxml2xmltest.c|3 +
  12 files changed, 240 insertions(+), 2 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features.xml

ACK

I will push this tomorrow unless someone has a different opinion.

(This didn't compile for me at the first try. Turns out 'git am' misplaced the
virDomainDefFeaturesCheckABIStability hunk.)

Jan



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

Re: [libvirt] [PATCH 1/3] docs, conf: add support for bootmenu timeout

2014-08-25 Thread Eric Blake
On 08/22/2014 09:09 AM, Martin Kletzander wrote:
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  docs/formatdomain.html.in  |  7 +-
  docs/schemas/domaincommon.rng  |  5 
  src/conf/domain_conf.c | 21 ++--
  src/conf/domain_conf.h |  2 ++
  ...qemuxml2argv-boot-menu-disable-with-timeout.xml | 29 
 ++
  ...2argv-boot-menu-enable-with-timeout-invalid.xml | 29 
 ++
  .../qemuxml2argv-boot-menu-enable-with-timeout.xml | 29 
 ++
  tests/qemuxml2argvtest.c   |  1 +
  ...muxml2xmlout-boot-menu-disable-with-timeout.xml | 29 
 ++
  tests/qemuxml2xmltest.c|  2 ++
  10 files changed, 151 insertions(+), 3 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout-invalid.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.xml
  create mode 100644 
 tests/qemuxml2xmloutdata/qemuxml2xmlout-boot-menu-disable-with-timeout.xml

 @@ -158,6 +158,11 @@
startup. The codeenable/code attribute can be either yes or no.
If not specified, the hypervisor default is used. span class=since
Since 0.8.3/span
 +  Additional attribute codetimeout/code takes the number of 
 milliseconds
 +  the boot menu should wait until it times out.  Allowed values are 
 numbers
 +  in range [0, 65535] inclusive and it is valid if and only if the 
 previous
 +  codeenable/code is set to yes.

You know, it is possible to have RelaxNG enforce this restriction:

 +  span class=since Since 1.2.8/span
/dd
dtcodesmbios/code/dt
ddHow to populate SMBIOS information visible in the guest.
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 033f2f6..9a89dd8 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -259,6 +259,11 @@
  element name=bootmenu
attribute name=enable
  choice
valueyes/value
  valueno/value
/choice
  /attribute
 +optional
 +  attribute name=timeout
 +ref name=unsignedShort/
 +  /attribute
 +/optional

would instead be:

element name=bootmenu
  choice
attribute name=enable
  valueno/value
/attribute
group
  attribute name=enable
valueyes/value
  /attribute
  optional
attribute name=timeout
  ref name=unsignedShort/
/attribute
  /optional
/group
  /choice

-- 
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] [PATCH 1/3] docs, conf: add support for bootmenu timeout

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 09:48:17AM -0600, Eric Blake wrote:

On 08/22/2014 09:09 AM, Martin Kletzander wrote:

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/formatdomain.html.in  |  7 +-
 docs/schemas/domaincommon.rng  |  5 
 src/conf/domain_conf.c | 21 ++--
 src/conf/domain_conf.h |  2 ++
 ...qemuxml2argv-boot-menu-disable-with-timeout.xml | 29 ++
 ...2argv-boot-menu-enable-with-timeout-invalid.xml | 29 ++
 .../qemuxml2argv-boot-menu-enable-with-timeout.xml | 29 ++
 tests/qemuxml2argvtest.c   |  1 +
 ...muxml2xmlout-boot-menu-disable-with-timeout.xml | 29 ++
 tests/qemuxml2xmltest.c|  2 ++
 10 files changed, 151 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-boot-menu-disable-with-timeout.xml



@@ -158,6 +158,11 @@
   startup. The codeenable/code attribute can be either yes or no.
   If not specified, the hypervisor default is used. span class=since
   Since 0.8.3/span
+  Additional attribute codetimeout/code takes the number of 
milliseconds
+  the boot menu should wait until it times out.  Allowed values are numbers
+  in range [0, 65535] inclusive and it is valid if and only if the previous
+  codeenable/code is set to yes.


You know, it is possible to have RelaxNG enforce this restriction:



Yes, but I wanted it to be just ignored if enable=no.  There are, of
course, other options, like keeping it there, but not using it or
leaving what it does on qemu.  I gues I could add otherwise it gets
ignored at the end of the sentence.  Should I do that, change the
behaviour or add the checks (and adjust tests of course)?  It needs a
follow-up, though since I already pushed it thanks to Jan's ACK.

Martin


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

Re: [libvirt] [PATCH 1/3] docs, conf: add support for bootmenu timeout

2014-08-25 Thread Eric Blake
On 08/25/2014 10:09 AM, Martin Kletzander wrote:

 +  Additional attribute codetimeout/code takes the number of
 milliseconds
 +  the boot menu should wait until it times out.  Allowed values
 are numbers
 +  in range [0, 65535] inclusive and it is valid if and only if
 the previous
 +  codeenable/code is set to yes.

 You know, it is possible to have RelaxNG enforce this restriction:

 
 Yes, but I wanted it to be just ignored if enable=no.  There are, of
 course, other options, like keeping it there, but not using it or
 leaving what it does on qemu.  I gues I could add otherwise it gets
 ignored at the end of the sentence.  Should I do that, change the
 behaviour or add the checks (and adjust tests of course)?  It needs a
 follow-up, though since I already pushed it thanks to Jan's ACK.

Maybe the simplest thing is a followup which changes:

s/it is valid if and only if the previous enable is set to yes/it is
ignored unless enable is set to yes/


-- 
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

[libvirt] [PATCH] docs: fix bootmenu timeout description

2014-08-25 Thread Martin Kletzander
Signed-off-by: Martin Kletzander mklet...@redhat.com
---

Notes:
Pushed as 'trivial'.

 docs/formatdomain.html.in | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9b8802d..59127bb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -160,9 +160,8 @@
   Since 0.8.3/span
   Additional attribute codetimeout/code takes the number of 
milliseconds
   the boot menu should wait until it times out.  Allowed values are numbers
-  in range [0, 65535] inclusive and it is valid if and only if the previous
-  codeenable/code is set to yes.
-  span class=since Since 1.2.8/span
+  in range [0, 65535] inclusive and it is ignored unless 
codeenable/code
+  is set to yes. span class=sinceSince 1.2.8/span
   /dd
   dtcodesmbios/code/dt
   ddHow to populate SMBIOS information visible in the guest.
-- 
2.0.4

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


Re: [libvirt] Mentors wanted for Outreach Program for Women October 2014

2014-08-25 Thread Stefan Hajnoczi
On Mon, Aug 25, 2014 at 11:52 AM, Martin Kletzander mklet...@redhat.com wrote:
 On Thu, Aug 21, 2014 at 09:06:39PM +0100, Stefan Hajnoczi wrote:
 Regular code contributors to QEMU, KVM, and libvirt are eligible to
 participate as mentors.

 We also need project ideas that are achievable in 12 weeks by someone
 skilled in programming but not necessarily familiar with open source
 or our codebase.  Ideas welcome!


 It's just a matter of ideas.  Maybe we could revisit some of those we
 had for GSoC.  If I'm reading the deadline for project ideas is
 October 22nd, so I think we'll definitely come up with something.

Yes, we can continue to offer project ideas that were not done last round.

Thanks for your interest, Martin!  I'll send more information once we
have information on how many slots are funded.

Stefan

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


Re: [libvirt] [PATCH 3/3] vbox: Register per partes

2014-08-25 Thread Martin Kletzander

On Fri, Aug 22, 2014 at 11:37:52AM +0200, Michal Privoznik wrote:

Since times when vbox moved to the daemon (due to some licensing
issue) the subdrivers that vbox implements were registered, but not
opened since our generic subdrivers took priority. I've tried to fix
this in 65b7d553f39ff9 but it was not correct. Apparently moving
vbox driver registration upfront changes the default connection URI
which makes some users sad. So, this commit breaks vbox into pieces
and register vbox's network and storage drivers first, and vbox driver
then at the end. This way, the vbox driver is registered in the order
it always was, but its subdrivers are registered prior the generic
ones.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
daemon/libvirtd.c  | 16 ++--
src/Makefile.am| 41 ++---
src/vbox/vbox_driver.c | 50 +-
src/vbox/vbox_driver.h | 10 ++
4 files changed, 107 insertions(+), 10 deletions(-)


[...]

diff --git a/src/Makefile.am b/src/Makefile.am
index 538530e..46e411e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1135,13 +1135,27 @@ libvirt_driver_vmware_la_SOURCES = 
$(VMWARE_DRIVER_SOURCES)
endif WITH_VMWARE

if WITH_VBOX
-noinst_LTLIBRARIES += libvirt_driver_vbox_impl.la
+noinst_LTLIBRARIES += \
+   libvirt_driver_vbox_impl.la \
+   libvirt_driver_vbox_network_impl.la \
+   libvirt_driver_vbox_storage_impl.la
libvirt_driver_vbox_la_SOURCES =
libvirt_driver_vbox_la_LIBADD = libvirt_driver_vbox_impl.la
+libvirt_driver_vbox_network_la_SOURCES =
+libvirt_driver_vbox_network_la_LIBADD = libvirt_driver_vbox_network_impl.la
+libvirt_driver_vbox_storage_la_SOURCES =
+libvirt_driver_vbox_storage_la_LIBADD = libvirt_driver_vbox_storage_impl.la


Couldn't you just do:

libvirt_driver_vbox_network_la_LIBADD = libvirt_driver_vbox_impl.la
libvirt_driver_vbox_storage_la_LIBADD = libvirt_driver_vbox_impl.la

Or just symlink these to the original one?  Of course you would export
all three register symbols, but just use the one you want for each
load.  Or am I missing something why that wouldn't work?

[reorganizing the hunks so my responses follow logically]

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 4cf78e6..c3bd2ab 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -383,7 +383,7 @@ static void daemonInitialize(void)
 * is not loaded they'll get a suitable error at that point
 */
# ifdef WITH_VBOX
-virDriverLoadModule(vbox);
+virDriverLoadModule(vbox_network);
# endif


It would look a bit nicer, I guess, if we just loaded the symbols in
virDriverLoadModule() into some kind of table (or list) and then
register them later on, but I understand this is just a fix so 1.2.8
is not broken and that suggestion might be done later.

We could also do:

#define virDriverLoadModule(x) virDriverLoadModuleFull(x, 0)

and then load each driver like this, for example:

virDriverLoadModuleFull(vbox, VIR_DRIVER_NETWORK);

Anyway, back to the stuff that's relevant for 1.2.8...

Everything looks fine from my POV and I tested what I could.  I,
however, have neither vbox nor xen installed to test what were the
issues in the first place, so I would like to ask whomever reported
the issue or uses those drivers to let us know before we merge this.

ACK series, but we should wait until at least rc0 to push this.  If
nobody else replies, I'd merge this into rc0 to let others test it
before the actual release.

Martin


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

Re: [libvirt] Mentors wanted for Outreach Program for Women October 2014

2014-08-25 Thread Marina Zhurakhinskaya
- Original Message -
 From: Stefan Hajnoczi stefa...@gmail.com
 To: Martin Kletzander mklet...@redhat.com
 Cc: qemu-devel qemu-de...@nongnu.org, libvir-list@redhat.com, kvm 
 k...@vger.kernel.org, Marina
 Zhurakhinskaya mari...@redhat.com
 Sent: Monday, August 25, 2014 12:29:27 PM
 Subject: Re: [libvirt] Mentors wanted for Outreach Program for Women October 
 2014
 
 On Mon, Aug 25, 2014 at 11:52 AM, Martin Kletzander mklet...@redhat.com
 wrote:
  On Thu, Aug 21, 2014 at 09:06:39PM +0100, Stefan Hajnoczi wrote:
  Regular code contributors to QEMU, KVM, and libvirt are eligible to
  participate as mentors.
 
  We also need project ideas that are achievable in 12 weeks by someone
  skilled in programming but not necessarily familiar with open source
  or our codebase.  Ideas welcome!
 
 
  It's just a matter of ideas.  Maybe we could revisit some of those we
  had for GSoC.  If I'm reading the deadline for project ideas is
  October 22nd, so I think we'll definitely come up with something.

Thank you for your interest in helping revisit GSoC ideas and come up with new 
ones! October 22 is an application deadline for prospective interns. QEMU would 
need to have some project ideas listed by September 8, though you can add more 
ideas through September. The timeline for the program is at 
https://wiki.gnome.org/OutreachProgramForWomen/2014/DecemberMarch You don't 
need very many ideas, as you are likely to only have at most 2-3 participants.

We don't yet have any funding confirmed for QEMU, but Stefan and I will be 
working on this. If your organization might be able to sponsor QEMU internships 
in OPW, please contact me and Stefan off-list. You can learn more at 
https://wiki.gnome.org/OutreachProgramForWomen/Admin/InfoForOrgs

Thanks,
Marina

 
 Yes, we can continue to offer project ideas that were not done last round.
 
 Thanks for your interest, Martin!  I'll send more information once we
 have information on how many slots are funded.
 
 Stefan
 

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


[libvirt] [PATCH 0/2] Bulk stats APIs

2014-08-25 Thread Peter Krempa
Unfortunately I wasn't able to finish the virsh and qemu driver IMPL but as the
release is closer than I'd like I'm sending this out for an early review pass.

Thanks in advance for any takers.

Peter Krempa (2):
  lib: Introduce API for retrieving bulk domain stats
  remote: Implement bulk domain stats APIs in the remote driver

 daemon/remote.c  |  91 ++
 include/libvirt/libvirt.h.in |  26 +++
 src/driver.h |   9 +++
 src/libvirt.c| 179 +++
 src/libvirt_public.syms  |   7 ++
 src/remote/remote_driver.c   |  88 +
 src/remote/remote_protocol.x |  26 ++-
 src/remote_protocol-structs  |  23 ++
 8 files changed, 448 insertions(+), 1 deletion(-)

-- 
2.0.2

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


[libvirt] [PATCH 1/2] lib: Introduce API for retrieving bulk domain stats

2014-08-25 Thread Peter Krempa
The motivation for this API is that management layers that use libvirt
usually poll for statistics using various split up APIs we currently
provide. To get all the necessary stuff, the app needs to issue a lot of
calls and agregate the results.

The APIs I'm introducing here:
1) Returns data in a format that we can expand in the future and is
(pseudo) hierarchical. The data is returned as typed parameters where
the fields are constructed as dot-separated strings containing names and
other stuff in a list of typed params.

2) Stats for multiple (all) domains can be queried at once and are
returned in one call. This will allow to decrease the overhead necessary
to issue multiple calls per domain multiplied by the count of domains.

3) Selectable (bit mask) fields in the returned format. This will allow
to retrieve only specific stats according to the apps need.

The stats groups will be enabled using a bit field @stats passed as the
function argument. A few sample stats groups that this API will support:

VIR_DOMAIN_STATS_STATE
VIR_DOMAIN_STATS_CPU
VIR_DOMAIN_STATS_BLOCK
VIR_DOMAIN_STATS_INTERFACE

the returned typed params will use the following scheme

state.state = running
state.reason = started
cpu.count = 8
cpu.0.state = running
cpu.0.time = 1234
---
 include/libvirt/libvirt.h.in |  26 +++
 src/driver.h |   9 +++
 src/libvirt.c| 179 +++
 src/libvirt_public.syms  |   7 ++
 4 files changed, 221 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 47ea695..962f740 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2501,6 +2501,32 @@ int virDomainDetachDeviceFlags(virDomainPtr domain,
 int virDomainUpdateDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);

+typedef struct _virDomainStatsRecord virDomainStatsRecord;
+typedef virDomainStatsRecord *virDomainStatsRecordPtr;
+struct _virDomainStatsRecord {
+virDomainPtr dom;
+unsigned int nparams;
+virTypedParameterPtr params;
+};
+
+typedef enum {
+VIR_DOMAIN_STATS_ALL = (1  0), /* return all stats fields
+   implemented in the daemon */
+VIR_DOMAIN_STATS_STATE = (1  1), /* return domain state */
+} virDomainStatsTypes;
+
+int virConnectGetAllDomainStats(virConnectPtr conn,
+unsigned int stats,
+virDomainStatsRecordPtr **retStats,
+unsigned int flags);
+
+int virDomainListGetStats(virDomainPtr *doms,
+  unsigned int stats,
+  virDomainStatsRecordPtr **retStats,
+  unsigned int flags);
+
+void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
+
 /*
  * BlockJob API
  */
diff --git a/src/driver.h b/src/driver.h
index ba7c1fc..d5596ab 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1191,6 +1191,14 @@ typedef int
   unsigned int flags);


+typedef int
+(*virDrvDomainListGetStats)(virConnectPtr conn,
+virDomainPtr *doms,
+unsigned int ndoms,
+unsigned int stats,
+virDomainStatsRecordPtr **retStats,
+unsigned int flags);
+
 typedef struct _virDriver virDriver;
 typedef virDriver *virDriverPtr;

@@ -1411,6 +1419,7 @@ struct _virDriver {
 virDrvDomainSetTime domainSetTime;
 virDrvNodeGetFreePages nodeGetFreePages;
 virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
+virDrvDomainListGetStats domainListGetStats;
 };


diff --git a/src/libvirt.c b/src/libvirt.c
index 8349261..bbbc023 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21341,3 +21341,182 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
 virDispatchError(conn);
 return NULL;
 }
+
+
+/**
+ * virConnectGetAllDomainStats:
+ * @conn: pointer to the hypervisor connection
+ * @stats: stats to return, binary-OR of virDomainStatsTypes
+ * @retStats: Pointer that will be filled with the array of returned stats.
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Query statistics for all domains on a given connection.
+ *
+ * Report statistics of various parameters for a running VM according to @stats
+ * field. The statistics are returned as an array of structures for each 
queried
+ * domain. The structure contains an array of typed parameters containing the
+ * individual statistics. The typed parameter name for each statistic field
+ * consists of a dot-separated string containing name of the requested group
+ * followed by a group specific description of the statistic value.
+ *
+ * The statistic groups are enabled using the @stats parameter which is a
+ * binary-OR of enum virDomainStatsTypes. The following groups are available
+ * (although not 

[libvirt] [PATCH 2/2] remote: Implement bulk domain stats APIs in the remote driver

2014-08-25 Thread Peter Krempa
Implement the remote driver support for shuffling the domain stats
around.
---
 daemon/remote.c  | 91 
 src/remote/remote_driver.c   | 88 ++
 src/remote/remote_protocol.x | 26 -
 src/remote_protocol-structs  | 23 +++
 4 files changed, 227 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index ea16789..34e6950 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -6337,6 +6337,97 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr 
server ATTRIBUTE_UNUSED,
 }


+static int
+remoteDispatchDomainListGetStats(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_list_get_stats_args *args,
+ remote_domain_list_get_stats_ret *ret)
+{
+int rv = -1;
+size_t i;
+struct daemonClientPrivate *priv = 
virNetServerClientGetPrivateData(client);
+virDomainStatsRecordPtr *retStats = NULL;
+int nrecords = 0;
+virDomainPtr *doms = NULL;
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (args-doms.doms_len) {
+if (VIR_ALLOC_N(doms, args-doms.doms_len + 1)  0)
+goto cleanup;
+
+for (i = 0; i  args-doms.doms_len; i++) {
+if (!(doms[i] = get_nonnull_domain(priv-conn, 
args-doms.doms_val[i])))
+goto cleanup;
+}
+
+if ((nrecords = virDomainListGetStats(doms,
+  args-stats,
+  retStats,
+  args-flags))  0)
+goto cleanup;
+} else {
+if (!(virConnectGetAllDomainStats(priv-conn,
+  args-stats,
+  retStats,
+  args-flags))  0)
+goto cleanup;
+}
+
+if (nrecords  REMOTE_DOMAIN_LIST_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Number of domain stats records is %d, 
+ which exceeds max limit: %d),
+   nrecords, REMOTE_DOMAIN_LIST_MAX);
+goto cleanup;
+}
+
+if (retStats  nrecords) {
+ret-retStats.retStats_len = nrecords;
+
+if (VIR_ALLOC_N(ret-retStats.retStats_val, nrecords)  0)
+goto cleanup;
+
+for (i = 0; i  nrecords; i++) {
+virDomainStatsRecordPtr src = retStats[i];
+remote_domain_stats_record *dst = ret-retStats.retStats_val + i;
+
+make_nonnull_domain(dst-dom, src-dom);
+
+if (remoteSerializeTypedParameters(src-params, src-nparams,
+   dst-params.params_val,
+   dst-params.params_len,
+   VIR_TYPED_PARAM_STRING_OKAY)  
0)
+goto cleanup;
+}
+} else {
+ret-retStats.retStats_len = 0;
+ret-retStats.retStats_val = NULL;
+}
+
+ret-ret = nrecords;
+rv = 0;
+
+ cleanup:
+if (rv  0)
+virNetMessageSaveError(rerr);
+if (retStats)
+virDomainStatsRecordListFree(retStats);
+if (args-doms.doms_len) {
+for (i = 0; i  args-doms.doms_len; i++)
+virDomainFree(doms[i]);
+
+VIR_FREE(doms);
+}
+return rv;
+}
+
+
 /*- Helpers. -*/

 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9a1d78f..41c807a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7670,6 +7670,93 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net,
 }


+static int
+remoteDomainListGetStats(virConnectPtr conn,
+ virDomainPtr *doms,
+ unsigned int ndoms,
+ unsigned int stats,
+ virDomainStatsRecordPtr **retStats,
+ unsigned int flags)
+{
+struct private_data *priv = conn-networkPrivateData;
+int rv = -1;
+size_t i;
+remote_domain_list_get_stats_args args;
+remote_domain_list_get_stats_ret ret;
+
+virDomainStatsRecordPtr *tmpret = NULL;
+
+if (ndoms) {
+if (VIR_ALLOC_N(args.doms.doms_val, ndoms)  0)
+goto cleanup;
+
+for (i = 0; i  ndoms; i++)
+make_nonnull_domain(args.doms.doms_val + i, doms[i]);
+}
+args.doms.doms_len = ndoms;
+
+args.stats = stats;
+args.flags = flags;
+
+memset(ret, 0, sizeof(ret));
+
+remoteDriverLock(priv);
+if (call(conn, priv, 0, 

Re: [libvirt] [PATCH 1/5] blockcopy: virDomainBlockCopy with XML destination, typed params

2014-08-25 Thread Peter Krempa
On 08/24/14 05:32, Eric Blake wrote:
 This commit (finally) adds the virDomainBlockCopy API, with the
 intent that it will provide more power to the existing 'virsh
 blockcopy' command.
 
 'virsh blockcopy' was first added in Apr 2012 (v0.9.12), which
 corresponds to the upstream qemu 1.2 timeframe.  It was done as
 a hack on top of the existing virDomainBlockRebase() API call,
 for two reasons: 1) it was targetting a feature that landed first
 in downstream RHEL qemu, but had not stabilized in upstream qemu
 at the time (and indeed, 'drive-mirror' only landed upstream in
 qemu 1.3 with slight differences to the first RHEL attempt,
 and later gained further parameters like granularity and buf-size
 that are also worth exposing), and 2) extending an existing API
 allowed it to be backported without worrying about bumping .so
 versions.  A virDomainBlockCopy() API was proposed at that time
 [1], but we decided not to accept it into libvirt until after
 upstream qemu stabilized, and it ended up getting scrapped.
 Whether or not RHEL should have attempted adding a new feature
 without getting it upstream first is a debate that can be held
 another day; but enough time has now elapsed that we are ready to
 do the interface cleanly.
 
 [1] https://www.redhat.com/archives/libvir-list/2012-April/msg00768.html
 
 Delaying the creation of a clean API until now has also had a
 benefit: we've only recently learned of a shortcoming in the
 original design, namely, that it is unable to target a network
 destination (such as a gluster volume) because it hard-coded the
 assumption that the destination is a local file name.  Because
 of all the refactoring we've done to add virStorageSourcePtr, we
 are in a better position to declare an API that parses XML
 describing a host storage source as the copy destination, which
 was not possible had we implemented virDomainBlockCopy as it had
 been originally envisioned.
 
 At least I had the foresight to create 'virsh blockcopy' as a
 separate command at the UI level (commit 1f06c00) rather than
 leaking the underlying API overload of virDomainBlockRebase onto
 shell users.
 
 A note on the bandwidth option: virTypedParameters intentionally
 lacks unsigned long (since variable-width interaction between
 mixed 32- vs. 64-bit client/server setups is nasty), but we have
 to deal with the fact that we are interacting with existing older
 code that mistakenly chose unsigned long bandwidth at a point
 before we decided to prohibit it in all new API.  The typed
 parameter is therefore unsigned long long, and the implementation
 will have to do overflow detection on 32-bit platforms.
 
 * include/libvirt/libvirt.h.in (virDomainBlockCopy): New API.
 (virDomainBlockJobType, virConnectDomainEventBlockJobStatus):
 Update related documentation.
 * src/libvirt.c (virDomainBlockCopy): Implement it.
 * src/libvirt_public.syms (LIBVIRT_1.2.8): Export it.
 * src/driver.h (_virDriver): New driver callback.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  include/libvirt/libvirt.h.in |  57 +++--
  src/driver.h |  11 +++-
  src/libvirt.c| 118 
 ++-
  src/libvirt_public.syms  |   5 ++
  4 files changed, 184 insertions(+), 7 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 47ea695..89c8e63 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2518,16 +2518,16 @@ typedef enum {
   * flags), job ends on completion */
 
  VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2,
 -/* Block Copy (virDomainBlockRebase with flags), job exists as
 - * long as mirroring is active */
 +/* Block Copy (virDomainBlockCopy, or virDomainBlockRebase with
 + * flags), job exists as long as mirroring is active */
 
  VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3,
  /* Block Commit (virDomainBlockCommit without flags), job ends on
   * completion */
 
  VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT = 4,
 -/* Active Block Commit (virDomainBlockCommit with flags), job
 - * exists as long as sync is active */
 +/* Active Block Commit (virDomainBlockCommit with flags), job exists
 + * as long as sync is active */
 
  #ifdef VIR_ENUM_SENTINELS
  VIR_DOMAIN_BLOCK_JOB_TYPE_LAST
 @@ -2597,6 +2597,53 @@ int   virDomainBlockRebase(virDomainPtr dom, 
 const char *disk,
 unsigned int flags);
 
  /**
 + * virDomainBlockCopyFlags:
 + *
 + * Flags available for virDomainBlockCopy().
 + */
 +typedef enum {
 +VIR_DOMAIN_BLOCK_COPY_SHALLOW   = 1  0, /* Limit copy to top of source
 + backing chain */
 +VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1  1, /* Reuse existing external
 + file for a copy */
 +} virDomainBlockCopyFlags;
 +
 +/**
 + * VIR_DOMAIN_BLOCK_COPY_BANDWIDTH:
 + * Macro for the 

Re: [libvirt] [PATCH 1/5] blockcopy: virDomainBlockCopy with XML destination, typed params

2014-08-25 Thread Eric Blake
On 08/25/2014 11:20 AM, Peter Krempa wrote:
 On 08/24/14 05:32, Eric Blake wrote:
 This commit (finally) adds the virDomainBlockCopy API, with the
 intent that it will provide more power to the existing 'virsh
 blockcopy' command.


 +/**
 + * VIR_DOMAIN_BLOCK_COPY_BANDWIDTH:
 + * Macro for the virDomainBlockCopy bandwidth tunable: it represents
 + * the maximum bandwidth (in MiB/s) used while getting the copy
 + * operation into the mirrored phase, with a type of ullong (although
 
 MiB/s and ullong? thats an awful lot of speed. Shouldn't we go with
 KiB/s? This might benefit slower networks. (although it may never
 converge there)

No. We're forced to use back-compat to the existing design of:

virDomainBlockRebase(virDomainPtr dom, const char *disk,
 const char *base, unsigned long bandwidth,
 unsigned int flags)
virDomainBlockJobSetSpeed(virDomainPtr dom, const char *disk,
  unsigned long bandwidth, unsigned int flags)

which already document bandwidth as an unsigned long (32-bit maximum is
the only portable value).  It's just that virTypedParameters
intentionally lack support for 'unsigned long' (precisely because it is
variable-sized between 32-bit and 64-bit), so my (still-in-progress)
patch 6 has an overflow check that errors out if the user passed in a
bandwidth using ullong that doesn't fit in the ulong handed to qemu.
The other alternative is to declare the parameter as 'uint', and that
the new API is unable to represent uses of the old API that exceed
INT_MAX, but I really want the new API to be a superset of the old API.

I tried to cover that point in the commit message, and also in the
comments to the macro stating that the hypervisor can reject
out-of-range values.  Do I need to make the wording any stronger?

 +/**
 + * VIR_DOMAIN_BLOCK_COPY_GRANULARITY:
 + * Macro for the virDomainBlockCopy granularity tunable: it represents
 + * the granularity at which the copy operation recognizes dirty pages,
 
 I'd rather say dirty blocks. Pages might indicate RAM memory pages.
 

Sure.  I'm just trying to expose the qemu parameters, which are rather
sparsely documented as:

# @granularity: #optional granularity of the dirty bitmap, default is 64K
#   if the image format doesn't have clusters, 4K if the
clusters
#   are smaller than that, else the cluster size.  Must be a
#   power of 2 between 512 and 64M (since 1.4).
#
# @buf-size: #optional maximum amount of data in flight from source to
#target (since 1.4).


 + * as an unsigned int, and must be a power of 2.
 + */
 +#define VIR_DOMAIN_BLOCK_COPY_GRANULARITY granularity
 +
 +/**
 + * VIR_DOMAIN_BLOCK_COPY_BUF_SIZE:
 + * Macro for the virDomainBlockCopy buffer size tunable: it represents
 + * how much data can be in flight between source and destination, as
 + * an unsigned int.
 
 In bytes?

I assume so; again, see the qemu docs that I'm trying to expose.

 + *
 + * A copy job has two parts; in the first phase, the @bandwidth parameter
 
 @bandwidth is now provided as a typed param.

Too much copy-and-paste - yeah, I'll have to adjust that.

 +int
 +virDomainBlockCopy(virDomainPtr dom, const char *disk,
 +   const char *destxml,
 +   virTypedParameterPtr params,
 +   int nparams,
 +   unsigned int flags)
 
 Wow, XML, typed params and flags. Now that's future proof! :)

I sure hope so!  Although I'm _already_ slightly worried about image
fleecing, which says to expose the state of the disk not as it is
currently evolving, but as it existed at a fixed point in time in the
past, often in order to copy out that state to backup storage.  In that
case, fleecing may want to start from a known point whereas this API
kind of implies starting from the active image.  Hmm, I guess we have
the 'vda[1]' notation for picking a known point that is the backing file
of vda.  Then again, while fleecing is a form of copying data, it might
be distinct enough to warrant a separate API anyway.


 +LIBVIRT_1.2.8 {
 +global:
 +virDomainBlockCopy;
 
 One of us will have to rebase. I've recently posted a series that adds
 API too :)

Fortunately, the rebase is trivial.

 
 +} LIBVIRT_1.2.7;
 +
  #  define new API here using predicted next version number 

 
 Apart from a few DOC problems the API looks fine to me and should be
 fairly future proof.
 
 ACK to the design (once docs are fixed).
 
 Peter
 
 P.S.: I've run out of time to review the rest of this, but this should
 be good enough to merge the rest a bit later.

Thanks; still, I'll post a v2 with tweaks you mentioned above and with
anything else I learn today while implementing the rest, if it looks
like DV will give me enough time to still get it into rc0.

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



signature.asc
Description: OpenPGP digital signature
--

[libvirt] [PATCH 1/3] Introduce virDomainOpenGraphicsFD API

2014-08-25 Thread Ján Tomko
Define the public API implementation and declare internal
driver prototype.
---
 include/libvirt/libvirt.h.in |  5 
 src/driver.h |  7 ++
 src/libvirt.c| 58 
 src/libvirt_public.syms  |  5 
 4 files changed, 75 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 47ea695..153b386 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5321,6 +5321,11 @@ int virDomainOpenGraphics(virDomainPtr dom,
   int fd,
   unsigned int flags);
 
+int virDomainOpenGraphicsFD(virDomainPtr dom,
+unsigned int idx,
+int *fd,
+unsigned int flags);
+
 int virDomainInjectNMI(virDomainPtr domain, unsigned int flags);
 
 int virDomainFSTrim(virDomainPtr dom,
diff --git a/src/driver.h b/src/driver.h
index ba7c1fc..39bf219 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -888,6 +888,12 @@ typedef int
 unsigned int flags);
 
 typedef int
+(*virDrvDomainOpenGraphicsFD)(virDomainPtr dom,
+  unsigned int idx,
+  int *fd,
+  unsigned int flags);
+
+typedef int
 (*virDrvDomainInjectNMI)(virDomainPtr dom,
  unsigned int flags);
 
@@ -1369,6 +1375,7 @@ struct _virDriver {
 virDrvDomainOpenConsole domainOpenConsole;
 virDrvDomainOpenChannel domainOpenChannel;
 virDrvDomainOpenGraphics domainOpenGraphics;
+virDrvDomainOpenGraphicsFD domainOpenGraphicsFD;
 virDrvDomainInjectNMI domainInjectNMI;
 virDrvDomainMigrateBegin3 domainMigrateBegin3;
 virDrvDomainMigratePrepare3 domainMigratePrepare3;
diff --git a/src/libvirt.c b/src/libvirt.c
index 8349261..9de1e44 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20182,6 +20182,64 @@ virDomainOpenGraphics(virDomainPtr dom,
 
 
 /**
+ * virDomainOpenGraphicsFD:
+ * @dom: pointer to domain object
+ * @idx: index of graphics config to open
+ * @fd: returned file descriptor
+ * @flags: bitwise-OR of virDomainOpenGraphicsFlags
+ *
+ * This will create a socket pair connected to the graphics backend of @dom.
+ * One pair will be returned as @fd.
+ * If @dom has multiple graphics backends configured, then @idx will determine
+ * which one is opened, starting from @idx 0.
+ *
+ * To disable any authentication, pass the VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH
+ * constant for @flags.
+ *
+ * This method can only be used when connected to a local
+ * libvirt hypervisor, over a UNIX domain socket. Attempts
+ * to use this method over a TCP connection will always fail
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virDomainOpenGraphicsFD(virDomainPtr dom,
+unsigned int idx,
+int *fd,
+unsigned int flags)
+{
+VIR_DOMAIN_DEBUG(dom, idx=%u, fd=%p, flags=%x,
+ idx, fd, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(dom, -1);
+virCheckNonNullArgGoto(fd, error);
+
+virCheckReadOnlyGoto(dom-conn-flags, error);
+
+if (!VIR_DRV_SUPPORTS_FEATURE(dom-conn-driver, dom-conn,
+  VIR_DRV_FEATURE_FD_PASSING)) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(fd passing is not supported by this connection));
+goto error;
+}
+
+if (dom-conn-driver-domainOpenGraphicsFD) {
+int ret;
+ret = dom-conn-driver-domainOpenGraphicsFD(dom, idx, fd, flags);
+if (ret  0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(dom-conn);
+return -1;
+}
+/**
  * virConnectSetKeepAlive:
  * @conn: pointer to a hypervisor connection
  * @interval: number of seconds of inactivity before a keepalive message is 
sent
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 9f4016a..ce5aeeb 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -670,4 +670,9 @@ LIBVIRT_1.2.7 {
 virConnectGetDomainCapabilities;
 } LIBVIRT_1.2.6;
 
+LIBVIRT_1.2.8 {
+ global:
+ virDomainOpenGraphicsFD;
+} LIBVIRT_1.2.7;
+
 #  define new API here using predicted next version number 
-- 
1.8.5.5

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


[libvirt] [RFC PATCH 0/3] Introduce virDomainOpenGraphicsFD API

2014-08-25 Thread Ján Tomko
Introduce virDomainOpenGraphicsFD which returns a socket fd
created by the daemon, unlike virDomainOpenGraphics, where
it's created by client.

This should be usable by virt-viewer under SELinux, but
I was not able to verify that yet.

Ján Tomko (3):
  Introduce virDomainOpenGraphicsFD API
  Add RPC implementation for virDomainOpenGraphicsFd
  Wire up virDomainOpenGraphicsFD in QEMU driver

 daemon/remote.c  | 42 +++
 include/libvirt/libvirt.h.in |  5 +++
 src/driver.h |  7 
 src/libvirt.c| 58 
 src/libvirt_public.syms  |  5 +++
 src/qemu/qemu_driver.c   | 80 
 src/remote/remote_driver.c   | 39 +
 src/remote/remote_protocol.x | 15 -
 src/rpc/virnetmessage.c  | 26 ++
 src/rpc/virnetmessage.h  |  3 ++
 10 files changed, 279 insertions(+), 1 deletion(-)

-- 
1.8.5.5

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

[libvirt] [PATCH 3/3] Wire up virDomainOpenGraphicsFD in QEMU driver

2014-08-25 Thread Ján Tomko
Should fix https://bugzilla.redhat.com/show_bug.cgi?id=26
---
 src/qemu/qemu_driver.c | 80 ++
 1 file changed, 80 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ad75bd9..932c638 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15806,6 +15806,85 @@ qemuDomainOpenGraphics(virDomainPtr dom,
 }
 
 static int
+qemuDomainOpenGraphicsFD(virDomainPtr dom,
+ unsigned int idx,
+ int *fd,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+qemuDomainObjPrivatePtr priv;
+const char *protocol;
+int pair[2] = {-1, -1};
+
+virCheckFlags(VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH, -1);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+if (virDomainOpenGraphicsFdEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(domain is not running));
+goto cleanup;
+}
+
+priv = vm-privateData;
+
+if (idx = vm-def-ngraphics) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(No graphics backend with index %d), idx);
+goto cleanup;
+}
+switch (vm-def-graphics[idx]-type) {
+case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+protocol = vnc;
+break;
+case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+protocol = spice;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Can only open VNC or SPICE graphics backends, not 
%s),
+   
virDomainGraphicsTypeToString(vm-def-graphics[idx]-type));
+goto cleanup;
+}
+
+if (virSecurityManagerSetSocketLabel(driver-securityManager, vm-def)  0)
+goto cleanup;
+
+if (socketpair(PF_UNIX, SOCK_STREAM, 0, pair)  0)
+goto cleanup;
+
+if (virSecurityManagerClearSocketLabel(driver-securityManager, vm-def)  
0)
+goto cleanup;
+/* TODO create and label the socket here */
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
+goto cleanup;
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorOpenGraphics(priv-mon, protocol, pair[1], graphicsfd,
+  (flags  VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) 
!= 0);
+qemuDomainObjExitMonitor(driver, vm);
+if (!qemuDomainObjEndJob(driver, vm))
+vm = NULL;
+
+*fd = pair[0];
+
+ cleanup:
+if (ret  0) {
+VIR_FORCE_CLOSE(pair[0]);
+VIR_FORCE_CLOSE(pair[1]);
+}
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+static int
 qemuDomainSetBlockIoTune(virDomainPtr dom,
  const char *disk,
  virTypedParameterPtr params,
@@ -17262,6 +17341,7 @@ static virDriver qemuDriver = {
 .connectDomainQemuMonitorEventDeregister = 
qemuConnectDomainQemuMonitorEventDeregister, /* 1.2.3 */
 .domainOpenConsole = qemuDomainOpenConsole, /* 0.8.6 */
 .domainOpenGraphics = qemuDomainOpenGraphics, /* 0.9.7 */
+.domainOpenGraphicsFD = qemuDomainOpenGraphicsFD, /* 1.2.8 */
 .domainInjectNMI = qemuDomainInjectNMI, /* 0.9.2 */
 .domainMigrateBegin3 = qemuDomainMigrateBegin3, /* 0.9.2 */
 .domainMigratePrepare3 = qemuDomainMigratePrepare3, /* 0.9.2 */
-- 
1.8.5.5

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


[libvirt] [PATCH 2/3] Add RPC implementation for virDomainOpenGraphicsFd

2014-08-25 Thread Ján Tomko
---
 daemon/remote.c  | 42 ++
 src/remote/remote_driver.c   | 39 +++
 src/remote/remote_protocol.x | 15 ++-
 src/rpc/virnetmessage.c  | 26 ++
 src/rpc/virnetmessage.h  |  3 +++
 5 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index ea16789..bd3b377 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4399,6 +4399,48 @@ remoteDispatchDomainOpenGraphics(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 }
 
 static int
+remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED,
+   virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
+   virNetMessagePtr msg,
+   virNetMessageErrorPtr rerr,
+   remote_domain_open_graphics_fd_args *args)
+{
+virDomainPtr dom = NULL;
+int rv = -1;
+int fd = -1;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if (virDomainOpenGraphicsFD(dom,
+args-idx,
+fd,
+args-flags)  0)
+goto cleanup;
+
+if (virNetMessageAddFD(msg, fd)  0)
+goto cleanup;
+
+rv = 1;
+
+ cleanup:
+VIR_FORCE_CLOSE(fd);
+if (rv  0) {
+virNetMessageSaveError(rerr);
+}
+
+if (dom)
+virDomainFree(dom);
+return rv;
+}
+static int
 remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server 
ATTRIBUTE_UNUSED,
virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
virNetMessagePtr msg 
ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9a1d78f..fb1fea7 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6446,6 +6446,44 @@ remoteDomainOpenGraphics(virDomainPtr dom,
 
 
 static int
+remoteDomainOpenGraphicsFD(virDomainPtr dom,
+   unsigned int idx,
+   int *fd,
+   unsigned int flags)
+{
+int rv = -1;
+remote_domain_open_graphics_args args;
+struct private_data *priv = dom-conn-privateData;
+int *fdout = NULL;
+size_t fdoutlen = 0;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(args.dom, dom);
+args.idx = idx;
+args.flags = flags;
+
+if (callFull(dom-conn, priv, 0,
+ NULL, 0,
+ fdout, fdoutlen,
+ REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD,
+ (xdrproc_t) xdr_remote_domain_open_graphics_fd_args, (char *) 
args,
+ (xdrproc_t) xdr_void, NULL) == -1)
+goto done;
+
+/* TODO: Check fdoutlen */
+*fd = fdout[0];
+
+rv = 0;
+
+ done:
+remoteDriverUnlock(priv);
+
+return rv;
+}
+
+
+static int
 remoteConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count)
 {
 struct private_data *priv = conn-privateData;
@@ -7963,6 +8001,7 @@ static virDriver remote_driver = {
 .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */
 .domainOpenChannel = remoteDomainOpenChannel, /* 1.0.2 */
 .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */
+.domainOpenGraphicsFD = remoteDomainOpenGraphicsFD, /* 1.2.8 */
 .domainInjectNMI = remoteDomainInjectNMI, /* 0.9.2 */
 .domainMigrateBegin3 = remoteDomainMigrateBegin3, /* 0.9.2 */
 .domainMigratePrepare3 = remoteDomainMigratePrepare3, /* 0.9.2 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 5c316fb..6dc2d29 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2733,6 +2733,12 @@ struct remote_domain_open_graphics_args {
 unsigned int flags;
 };
 
+struct remote_domain_open_graphics_fd_args {
+remote_nonnull_domain dom;
+unsigned int idx;
+unsigned int flags;
+};
+
 struct remote_node_suspend_for_duration_args {
 unsigned int target;
 unsigned hyper duration;
@@ -5420,5 +5426,12 @@ enum remote_procedure {
  * @generate: both
  * @acl: connect:write
  */
-REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342
+REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342,
+
+/**
+ * @generate: none
+ * @acl: domain:open_graphics
+ */
+REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343
+
 };
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c
index 19b2d6c..5c57128 100644
--- a/src/rpc/virnetmessage.c
+++ b/src/rpc/virnetmessage.c
@@ -564,3 +564,29 @@ int virNetMessageDupFD(virNetMessagePtr msg,
 }
 return fd;
 }
+
+int 

Re: [libvirt] Plan for the next release

2014-08-25 Thread Ján Tomko
On 08/25/2014 02:34 PM, Peter Krempa wrote:
 On 08/20/14 13:26, Daniel Veillard wrote:
   We already have a number of commits in, over 150, I can't freeze next
 monday but I could do this next Tuesday (the 26) for a planned release
 around Sep 1st,

   So unless there is a good reason to change this, that's the plan :)
 
 Well there are a few new API designs either posted or in progress which
 we'd like to get into this release, so we'd appreciate a day or two
 extra to finish stuff up.
 
 

The bulk domain stats API would be cool to have in a release with such a round
number. :)

I have also posted an API proposal myself, but I don't keep my hopes up about
getting it into 1.2.8.

Jan



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

Re: [libvirt] [PATCH v2] spec: drop anything older than Fedora 13

2014-08-25 Thread Eric Blake
On 08/23/2014 09:45 AM, Daniel Veillard wrote:
  Haha, looks fine, ACK :-)
 
 We never built on RHEL-4 so that doesn't affect any RHEL, and
 Fedora before 14 really really should not be used anymore,

Perhaps we can further simplify the spec file all the way up to a
minimum of Fedora 18 while still keeping RHEL 5 working, but I wasn't
ready to do that.

At any rate, this patch is now pushed.

-- 
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

[libvirt] [PATCH] maint: drop spurious semicolons

2014-08-25 Thread Eric Blake
I noticed a line 'int nparams = 0;;' in remote_dispatch.h, and
tracked down where it was generated.  While at it, I found a
couple of other double semicolons.  Additionally, I noticed that
commit df0b57a95 left a stale reference to the file name
remote_dispatch_bodies.h.

* src/conf/numatune_conf.c (virDomainNumatuneNodeParseXML): Drop
empty statement.
* tests/virdbustest.c (testMessageStruct, testMessageSimple):
Likewise.
* src/rpc/gendispatch.pl (remote_dispatch_bodies.h): Likewise, and
update stale comments.

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

Pushing under the trivial rule.

Syntax check can't easily catch this, because we want to allow
'for (int i = 0;; i++)' on a loop that uses break to exit, so
this is just done by manual inspection.

 src/conf/numatune_conf.c | 2 +-
 src/rpc/gendispatch.pl   | 4 ++--
 tests/virdbustest.c  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index ff0f3eb..21d9a64 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -81,7 +81,7 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr 
*numatunePtr,
   xmlXPathContextPtr ctxt)
 {
 char *tmp = NULL;
-int n = 0;;
+int n = 0;
 int ret = -1;
 size_t i = 0;
 virDomainNumatunePtr numatune = *numatunePtr;
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index d820d0e..27093d2 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -380,7 +380,7 @@ if ($mode eq debug) {
 }
 }

-# Bodies for dispatch functions (remote_dispatch_bodies.h).
+# Bodies for dispatch functions (remote_dispatch.h).
 elsif ($mode eq server) {
 my %generate = map { $_ = 1 } @autogen;
 my @keys = sort (keys %calls);
@@ -537,7 +537,7 @@ elsif ($mode eq server) {
 push(@args_list, args-$1.$1_len);
 } elsif ($args_member =~ m/^remote_typed_param (\S+)(\S+);/) 
{
 push(@vars_list, virTypedParameterPtr $1 = NULL);
-push(@vars_list, int n$1 = 0;);
+push(@vars_list, int n$1 = 0);
 if ($call-{ProcName} eq NodeSetMemoryParameters) {
 push(@args_list, priv-conn);
 }
diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index a798fbe..0079b41 100644
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -62,7 +62,7 @@ static int testMessageSimple(const void *args 
ATTRIBUTE_UNUSED)
 unsigned int in_uint32 = 2, out_uint32 = 0;
 long long in_int64 = 1LL, out_int64 = 0;
 unsigned long long in_uint64 = 2LL, out_uint64 = 0;
-double in_double = 3.14159265359, out_double = 0;;
+double in_double = 3.14159265359, out_double = 0;
 const char *in_string = Hello World;
 char *out_string = NULL;
 const char *in_objectpath = /org/libvirt/test;
@@ -338,7 +338,7 @@ static int testMessageStruct(const void *args 
ATTRIBUTE_UNUSED)
 unsigned int in_uint32 = 2, out_uint32 = 0;
 long long in_int64 = -1LL, out_int64 = 0;
 unsigned long long in_uint64 = 2LL, out_uint64 = 0;
-double in_double = 3.14159265359, out_double = 0;;
+double in_double = 3.14159265359, out_double = 0;
 const char *in_string = Hello World;
 char *out_string = NULL;
 const char *in_objectpath = /org/libvirt/test;
-- 
1.9.3

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


[libvirt] [PATCH] libxl: fix memory corruption introduced by commit b55cc5f4e

2014-08-25 Thread Jim Fehlig
Commit b55cc5f4e did a shallow copy of libxl_{sdl,vnc}_info from the
domain config to the build info, which resulted in double-freeing
strings contained in the structures during cleanup, which later
resulted in a libvirtd crash.  Fix by performing a deep copy of the
structure, VIR_STRDUP'ing embedded strings instead of simply copying
their pointers.

Fixes the following issue reported on the libvirt dev list

https://www.redhat.com/archives/libvir-list/2014-August/msg01112.html
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_conf.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 1210500..1dbdd9c 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1130,10 +1130,24 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
 libxl_domain_build_info *b_info = d_config-b_info;
 libxl_device_vfb vfb = d_config-vfbs[0];
 
-if (libxl_defbool_val(vfb.vnc.enable))
-memcpy(b_info-u.hvm.vnc, vfb.vnc, sizeof(libxl_vnc_info));
-else if (libxl_defbool_val(vfb.sdl.enable))
-memcpy(b_info-u.hvm.sdl, vfb.sdl, sizeof(libxl_sdl_info));
+if (libxl_defbool_val(vfb.vnc.enable)) {
+libxl_defbool_set(b_info-u.hvm.vnc.enable, true);
+if (VIR_STRDUP(b_info-u.hvm.vnc.listen, vfb.vnc.listen)  0)
+goto error;
+if (VIR_STRDUP(b_info-u.hvm.vnc.passwd, vfb.vnc.passwd)  0)
+goto error;
+b_info-u.hvm.vnc.display = vfb.vnc.display;
+libxl_defbool_set(b_info-u.hvm.vnc.findunused,
+  libxl_defbool_val(vfb.vnc.findunused));
+} else if (libxl_defbool_val(vfb.sdl.enable)) {
+libxl_defbool_set(b_info-u.hvm.sdl.enable, true);
+libxl_defbool_set(b_info-u.hvm.sdl.opengl,
+  libxl_defbool_val(vfb.sdl.opengl));
+if (VIR_STRDUP(b_info-u.hvm.sdl.display, vfb.sdl.display)  0)
+goto error;
+if (VIR_STRDUP(b_info-u.hvm.sdl.xauthority, vfb.sdl.xauthority)  
0)
+goto error;
+}
 }
 
 return 0;
-- 
1.8.4.5

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


Re: [libvirt] [Libvirt] segfault with patch libxl: fix framebuffer port setting for HVM domains

2014-08-25 Thread Jim Fehlig
Chris wrote:
 Hello,

 I encountered segfaults with libvirt + libxl with this specific patch :
 http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b55cc5f4e31b488c4f9c3c8470c992c1f8f5d09c
   

Hi Chris,

Thanks for the report.  Took a bit to track this down, but in my
testing, the following patch fixes the issue

https://www.redhat.com/archives/libvir-list/2014-August/msg01151.html

Regards,
Jim

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


Re: [libvirt] [PATCH] libxl: fix memory corruption introduced by commit b55cc5f4e

2014-08-25 Thread Eric Blake
On 08/25/2014 05:01 PM, Jim Fehlig wrote:
 Commit b55cc5f4e did a shallow copy of libxl_{sdl,vnc}_info from the
 domain config to the build info, which resulted in double-freeing
 strings contained in the structures during cleanup, which later
 resulted in a libvirtd crash.  Fix by performing a deep copy of the
 structure, VIR_STRDUP'ing embedded strings instead of simply copying
 their pointers.
 
 Fixes the following issue reported on the libvirt dev list
 
 https://www.redhat.com/archives/libvir-list/2014-August/msg01112.html
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_conf.c | 22 ++
  1 file changed, 18 insertions(+), 4 deletions(-)

ACK.

-- 
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] [PATCH] libxl: fix memory corruption introduced by commit b55cc5f4e

2014-08-25 Thread Jim Fehlig
Eric Blake wrote:
 On 08/25/2014 05:01 PM, Jim Fehlig wrote:
   
 Commit b55cc5f4e did a shallow copy of libxl_{sdl,vnc}_info from the
 domain config to the build info, which resulted in double-freeing
 strings contained in the structures during cleanup, which later
 resulted in a libvirtd crash.  Fix by performing a deep copy of the
 structure, VIR_STRDUP'ing embedded strings instead of simply copying
 their pointers.

 Fixes the following issue reported on the libvirt dev list

 https://www.redhat.com/archives/libvir-list/2014-August/msg01112.html
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_conf.c | 22 ++
  1 file changed, 18 insertions(+), 4 deletions(-)
 

 ACK.
   

Thanks; Pushed.

Regards,
Jim

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


[libvirt] [PATCH 0/4] Introduce support for virtio-blk-pci iothreads

2014-08-25 Thread John Ferlan
Introduce iothreads support to libvirt.  These will be used to facilitate
adding an iothread attribute to a support disk which will enable having
a dedicated event loop thread for the disk.  IOThreads are a QEMU feature
recently added (2.1) as a replacement for the virtio-blk data plane
functionality that's been in tech preview since 1.4.

Followup patches will add API's in order to list the IOThreads and eventually
be able to assign IOThreads to specific CPU's if so desired. This set of
patches should cover at least the bare minimum in order to allow modifying
domain XML in order to use the feature.


John Ferlan (4):
  domain_conf: Introduce iothreads XML
  qemu: Add support for iothreads
  domain_conf: Add support for iothreads in disk definition
  qemu: Allow use of iothreads for disk definitions

 docs/formatdomain.html.in  | 34 ++
 docs/schemas/domaincommon.rng  | 25 +++
 src/conf/domain_conf.c | 52 +-
 src/conf/domain_conf.h |  4 ++
 src/qemu/qemu_capabilities.c   |  2 +
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 48 
 src/qemu/qemu_command.h|  5 +++
 src/qemu/qemu_hotplug.c| 11 +
 .../qemuxml2argv-iothreads-disk.args   | 17 +++
 .../qemuxml2argv-iothreads-disk.xml| 40 +
 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args |  8 
 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml  | 29 
 tests/qemuxml2argvtest.c   |  4 ++
 tests/qemuxml2xmltest.c|  2 +
 15 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml

-- 
1.9.3

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


[libvirt] [PATCH 1/4] domain_conf: Introduce iothreads XML

2014-08-25 Thread John Ferlan
Introduce XML to allowing adding iothreads to the domain. These can be
used by virtio-blk-pci devices in order to assign a specific thread to
handle the workload for the device.  The iothreads are the official
implementation of the virtio-blk Data Plane that's been in tech preview
for QEMU.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in | 26 ++
 docs/schemas/domaincommon.rng | 12 
 src/conf/domain_conf.c| 28 
 src/conf/domain_conf.h|  2 ++
 4 files changed, 68 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 59127bb..0fe10f4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -470,6 +470,32 @@
   /dd
 /dl
 
+h3a name=elementsIOThreadsAllocationIOThreads Allocation/a/h3
+  p
+IOThreads are a QEMU feature that will allow supported disk
+devices to be configured to use a dedicated event loop thread
+to handle block I/O requests.
+span class=sinceSince 1.2.8 (QEMU only)/span
+  /p
+
+pre
+lt;domaingt;
+  ...
+  lt;iothreadsgt;4lt;/iothreadsgt;
+  ...
+lt;/domaingt;
+/pre
+
+dl
+  dtcodeiothreads/code/dt
+  dd
+The content of this optional element defines the number
+of IOThreads to be assigned to the domain for use by
+virtio-blk-pci target storage devices. There should be
+only 1 or 2 IOThreads per host CPU and 1 IOThread per
+supported device.
+  /dd
+/dl
 
 h3a name=elementsCPUTuningCPU Tuning/a/h3
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9a89dd8..b4ac483 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -632,6 +632,12 @@
   /optional
 
   optional
+element name=iothreads
+  ref name=countIOThreads/
+/element
+  /optional
+
+  optional
 ref name=blkiotune/
   /optional
 
@@ -4747,6 +4753,12 @@
   param name=minInclusive1/param
 /data
   /define
+  define name=countIOThreads
+data type=unsignedInt
+  param name=pattern[0-9]+/param
+  param name=minInclusive0/param
+/data
+  /define
   define name=vcpuid
 data type=unsignedShort
   param name=pattern[0-9]+/param
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 22a7f7e..671c41c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11953,6 +11953,23 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 }
 
+/* Optional - iothreads */
+n = virXPathULong(string(./iothreads[1]), ctxt, count);
+if (n == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(iothreads count must be an integer));
+goto error;
+} else if (n  0) {
+def-iothreads = 0;
+} else {
+if ((unsigned int) count != count) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(invalid iothreads count '%lu'), count);
+goto error;
+}
+def-iothreads = count;
+}
+
 /* Extract cpu tunables. */
 if ((n = virXPathULong(string(./cputune/shares[1]), ctxt,
def-cputune.shares))  -1) {
@@ -14461,6 +14478,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 goto error;
 }
 
+if (src-iothreads != dst-iothreads) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Target domain iothreads count %u does not 
+ match source %u),
+   dst-iothreads, src-iothreads);
+goto error;
+}
+
 if (STRNEQ(src-os.type, dst-os.type)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Target domain OS type %s does not match source %s),
@@ -17874,6 +17899,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAsprintf(buf,  current='%u', def-vcpus);
 virBufferAsprintf(buf, %u/vcpu\n, def-maxvcpus);
 
+if (def-iothreads  0)
+virBufferAsprintf(buf, iothreads%u/iothreads\n, def-iothreads);
+
 if (def-cputune.sharesSpecified ||
 (def-cputune.nvcpupin  !virDomainIsAllVcpupinInherited(def)) ||
 def-cputune.period || def-cputune.quota ||
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 36ccf10..705ce32 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1908,6 +1908,8 @@ struct _virDomainDef {
 int placement_mode;
 virBitmapPtr cpumask;
 
+unsigned int iothreads;
+
 struct {
 unsigned long shares;
 bool sharesSpecified;
-- 
1.9.3

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


[libvirt] [PATCH 2/4] qemu: Add support for iothreads

2014-08-25 Thread John Ferlan
Add a new capability to ensure the iothreads feature exists for the qemu
emulator being run - requires the query-iothreads QMP command. Using the
domain XML add correspoding command argument in order to generate the
threads. The iothreads will use a name space libvirtIothread# where, the
future patch to add support for using an iothread to a disk definition to
merely define which of the available threads to use.

Add tests to ensure the xml/argv processing is correct.  Note that no
change was made to qemuargv2xmltest.c as processing the -object element
would require knowing more than just iothreads.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_capabilities.c   |  2 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 13 ++
 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args |  8 ++
 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml  | 29 ++
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 56 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 410086b..b331be7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -268,6 +268,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   rtc-reset-reinjection,
 
   splash-timeout, /* 175 */
+  query-iothreads,
 );
 
 
@@ -1430,6 +1431,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { nbd-server-start, QEMU_CAPS_NBD_SERVER },
 { change-backing-file, QEMU_CAPS_CHANGE_BACKING_FILE },
 { rtc-reset-reinjection, QEMU_CAPS_RTC_RESET_REINJECTION },
+{ query-iothreads, QEMU_CAPS_OBJECT_IOTHREAD},
 };
 
 struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 48a4eaa..0980c00 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -215,6 +215,7 @@ typedef enum {
 QEMU_CAPS_OBJECT_USB_AUDIO   = 173, /* usb-audio device support */
 QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor 
command */
 QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
+QEMU_CAPS_OBJECT_IOTHREAD= 176, /* -object iothread */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6dac9d3..287a3f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7510,6 +7510,19 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, smp);
 VIR_FREE(smp);
 
+if (def-iothreads  0 
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+/* Create named iothread objects starting with 1. These may be used
+ * by a disk definition which will associate to an iothread by
+ * supplying a value of 1 up to the number of iothreads available
+ * (since 0 would indicate to not use the feature).
+ */
+for (i = 1; i = def-iothreads; i++) {
+virCommandAddArg(cmd, -object);
+virCommandAddArgFormat(cmd, iothread,id=libvirtIothread%zu, i);
+}
+}
+
 if (def-cpu  def-cpu-ncells)
 if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps)  0)
 goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args 
b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
new file mode 100644
index 000..ca0c174
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M \
+pc -m 214 -smp 2 \
+-object iothread,id=libvirtIothread1 \
+-object iothread,id=libvirtIothread2 \
+-nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
+/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
new file mode 100644
index 000..12a92e7
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
@@ -0,0 +1,29 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'2/vcpu
+  iothreads2/iothreads
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  driver name='qemu' type='raw'/
+  source 

[libvirt] [PATCH 3/4] domain_conf: Add support for iothreads in disk definition

2014-08-25 Thread John Ferlan
Add a new disk driver attribute iothread to be parsed as the thread
number for the disk to use. In order to more easily facilitate the usage
and configuration of the iothread, a zero for the attribute indicates
iothreads are not supported for the device and a positive value indicates
the specific thread to try and use.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in |  8 
 docs/schemas/domaincommon.rng | 13 +
 src/conf/domain_conf.c| 24 +++-
 src/conf/domain_conf.h|  2 ++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0fe10f4..ea0ca83 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2141,6 +2141,14 @@
 (ignore the discard request).
 span class='since'Since 1.0.6 (QEMU and KVM only)/span
   /li
+  li
+The optional codeiothread/code attribute will assign the
+disk to an IOThread as defined by the range for the domain
+a href=#elementsIOThreadsAllocationcodeiothreads/code/a
+value. Each device must use a unique IOThread and threads will
+be numbered from 1 to the domain iothreads value.
+span class='since'Since 1.2.8 (QEMU only)/span
+  /li
 /ul
   /dd
   dtcodeboot/code/dt
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b4ac483..40891e4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1572,6 +1572,9 @@
   optional
 ref name=discard/
   /optional
+  optional
+ref name=driverIOThread/
+  /optional
   empty/
 /element
   /define
@@ -1659,6 +1662,11 @@
   /choice
 /attribute
   /define
+  define name=driverIOThread
+attribute name='iothread'
+  ref name=iothreadsid/
+/attribute
+  /define
   define name=controller
 element name=controller
   attribute name=index
@@ -4759,6 +4767,11 @@
   param name=minInclusive0/param
 /data
   /define
+  define name=iothreadsid
+data type=unsignedInt
+  param name=pattern[0-9]+/param
+/data
+  /define
   define name=vcpuid
 data type=unsignedShort
   param name=pattern[0-9]+/param
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 671c41c..b15f279 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5399,6 +5399,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *ioeventfd = NULL;
 char *event_idx = NULL;
 char *copy_on_read = NULL;
+char *driverIOThread = NULL;
 char *devaddr = NULL;
 virStorageEncryptionPtr encryption = NULL;
 char *serial = NULL;
@@ -5547,6 +5548,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 event_idx = virXMLPropString(cur, event_idx);
 copy_on_read = virXMLPropString(cur, copy_on_read);
 discard = virXMLPropString(cur, discard);
+driverIOThread = virXMLPropString(cur, iothread);
 } else if (!def-mirror 
xmlStrEqual(cur-name, BAD_CAST mirror) 
!(flags  VIR_DOMAIN_XML_INACTIVE)) {
@@ -6080,6 +6082,15 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
+if (driverIOThread) {
+if (virStrToLong_uip(driverIOThread, NULL, 10, def-iothread)  0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(Invalid iothread attribute in disk driver 
+ element: %s), driverIOThread);
+goto error;
+}
+}
+
 if (devaddr) {
 if (virDomainParseLegacyDeviceAddress(devaddr,
   def-info.addr.pci)  0) {
@@ -6180,6 +6191,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 VIR_FREE(event_idx);
 VIR_FREE(copy_on_read);
 VIR_FREE(discard);
+VIR_FREE(driverIOThread);
 VIR_FREE(devaddr);
 VIR_FREE(serial);
 virStorageEncryptionFree(encryption);
@@ -11968,6 +11980,14 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 def-iothreads = count;
+
+/* Create a bitmap for inuse threads - noting that entries are
+ * numbered 1..def-iothreads since 0 (zero) iothreads means
+ * nothing and assigning a disk to an IOThread requires at least a
+ * thread#  0 since a zero would indicate no IOThread for the disk
+ */
+if (!(def-iothreadmap = virBitmapNew(def-iothreads+1)))
+goto error;
 }
 
 /* Extract cpu tunables. */
@@ -15538,7 +15558,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
 if (def-src-driverName || def-src-format  0 || def-cachemode ||
 def-error_policy || def-rerror_policy || def-iomode ||
 def-ioeventfd || def-event_idx || def-copy_on_read ||
-def-discard) {
+

[libvirt] [PATCH 4/4] qemu: Allow use of iothreads for disk definitions

2014-08-25 Thread John Ferlan
For virtio-blk-pci disks with the disk iothread attribute that are
running the correct emulator, add the iothread=libvirtIothread# to
the -device command line in order to enable iothreads for the disk.

This code will check both the start and hotplug paths for the capability,
whether the iothreadsmap has been defined, and whether there's an available
iothread to be used for the disk.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_command.c| 35 +++
 src/qemu/qemu_command.h|  5 +++
 src/qemu/qemu_hotplug.c| 11 ++
 .../qemuxml2argv-iothreads-disk.args   | 17 +
 .../qemuxml2argv-iothreads-disk.xml| 40 ++
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 111 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 287a3f3..740b6ec 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3862,6 +3862,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
 virBufferAddLit(opt, virtio-blk-device);
 } else {
 virBufferAddLit(opt, virtio-blk-pci);
+if (disk-iothread  0)
+virBufferAsprintf(opt, ,iothread=libvirtIothread%u,
+  disk-iothread);
 }
 qemuBuildIoEventFdStr(opt, disk-ioeventfd, qemuCaps);
 if (disk-event_idx 
@@ -6614,6 +6617,31 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 
 
+bool
+qemuDomainIothreadsAvailable(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainDiskDefPtr disk)
+{
+bool inuse;
+const char *src = virDomainDiskGetSource(disk);
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD) ||
+!def-iothreadmap) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(IOThreads not supported for this QEMU 
+ for disk '%s'), src);
+return false;
+}
+if (virBitmapGetBit(def-iothreadmap, disk-iothread, inuse) || inuse) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(IOThread '%u' for disk '%s' is already 
+ being used), disk-iothread, src);
+return false;
+}
+return true;
+}
+
+
 static int
 qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
 virCommandPtr cmd,
@@ -8055,6 +8083,13 @@ qemuBuildCommandLine(virConnectPtr conn,
disk-src-driverName, disk-src-path);
 goto error;
 }
+
+/* Check iothreads relationship */
+if (disk-iothread  0) {
+if (!qemuDomainIothreadsAvailable(def, qemuCaps, disk))
+goto error;
+ignore_value(virBitmapSetBit(def-iothreadmap, disk-iothread));
+}
 }
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 633ff71..bec6c14 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -81,6 +81,11 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
bool forXMLToArgv)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);
 
+bool
+qemuDomainIothreadsAvailable(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainDiskDefPtr disk);
+
 /* Generate '-device' string for chardev device */
 int
 qemuBuildChrDeviceStr(char **deviceStr,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a364c52..720220d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -335,6 +335,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 }
 }
 
+if (disk-iothread  0) {
+if (!qemuDomainIothreadsAvailable(vm-def, priv-qemuCaps, disk))
+goto cleanup;
+}
+
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false)  0)
 goto cleanup;
 
@@ -396,6 +401,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 if (ret  0)
 goto error;
 
+if (disk-iothread  vm-def-iothreadmap)
+ignore_value(virBitmapSetBit(vm-def-iothreadmap, disk-iothread));
+
 virDomainDiskInsertPreAlloced(vm-def, disk);
 
  cleanup:
@@ -2539,6 +2547,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
 }
 }
 
+if (disk-iothread  vm-def-iothreadmap)
+ignore_value(virBitmapClearBit(vm-def-iothreadmap, disk-iothread));
+
 qemuDomainReleaseDeviceAddress(vm, disk-info, src);
 
 if (virSecurityManagerRestoreDiskLabel(driver-securityManager,
diff --git 

Re: [libvirt] [PATCH 1/2] lib: Introduce API for retrieving bulk domain stats

2014-08-25 Thread Li Wei


On 08/26/2014 01:05 AM, Peter Krempa wrote:
 The motivation for this API is that management layers that use libvirt
 usually poll for statistics using various split up APIs we currently
 provide. To get all the necessary stuff, the app needs to issue a lot of
 calls and agregate the results.
 
 The APIs I'm introducing here:
 1) Returns data in a format that we can expand in the future and is
 (pseudo) hierarchical. The data is returned as typed parameters where
 the fields are constructed as dot-separated strings containing names and
 other stuff in a list of typed params.
 
 2) Stats for multiple (all) domains can be queried at once and are
 returned in one call. This will allow to decrease the overhead necessary
 to issue multiple calls per domain multiplied by the count of domains.
 
 3) Selectable (bit mask) fields in the returned format. This will allow
 to retrieve only specific stats according to the apps need.
 
 The stats groups will be enabled using a bit field @stats passed as the
 function argument. A few sample stats groups that this API will support:
 
 VIR_DOMAIN_STATS_STATE
 VIR_DOMAIN_STATS_CPU
 VIR_DOMAIN_STATS_BLOCK
 VIR_DOMAIN_STATS_INTERFACE
 
 the returned typed params will use the following scheme
 
 state.state = running
 state.reason = started
 cpu.count = 8
 cpu.0.state = running
 cpu.0.time = 1234
 ---
  include/libvirt/libvirt.h.in |  26 +++
  src/driver.h |   9 +++
  src/libvirt.c| 179 
 +++
  src/libvirt_public.syms  |   7 ++
  4 files changed, 221 insertions(+)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 47ea695..962f740 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2501,6 +2501,32 @@ int virDomainDetachDeviceFlags(virDomainPtr domain,
  int virDomainUpdateDeviceFlags(virDomainPtr domain,
 const char *xml, unsigned int flags);
 
 +typedef struct _virDomainStatsRecord virDomainStatsRecord;
 +typedef virDomainStatsRecord *virDomainStatsRecordPtr;
 +struct _virDomainStatsRecord {
 +virDomainPtr dom;
 +unsigned int nparams;
 +virTypedParameterPtr params;
 +};
 +
 +typedef enum {
 +VIR_DOMAIN_STATS_ALL = (1  0), /* return all stats fields
 +   implemented in the daemon */

Why not define VIR_DOMAIN_STATS_ALL as the bitwise or of each other
individual VIR_DOMAIN_STATS_XXX so we no need to make a special path
for VIR_DOMAIN_STATS_ALL in the implementation?


 +VIR_DOMAIN_STATS_STATE = (1  1), /* return domain state */
 +} virDomainStatsTypes;
 +
 +int virConnectGetAllDomainStats(virConnectPtr conn,
 +unsigned int stats,
 +virDomainStatsRecordPtr **retStats,
 +unsigned int flags);
 +
 +int virDomainListGetStats(virDomainPtr *doms,
 +  unsigned int stats,
 +  virDomainStatsRecordPtr **retStats,
 +  unsigned int flags);
 +
 +void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
 +
  /*
   * BlockJob API
   */
 diff --git a/src/driver.h b/src/driver.h
 index ba7c1fc..d5596ab 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -1191,6 +1191,14 @@ typedef int
unsigned int flags);
 
 
 +typedef int
 +(*virDrvDomainListGetStats)(virConnectPtr conn,
 +virDomainPtr *doms,
 +unsigned int ndoms,
 +unsigned int stats,
 +virDomainStatsRecordPtr **retStats,
 +unsigned int flags);
 +
  typedef struct _virDriver virDriver;
  typedef virDriver *virDriverPtr;
 
 @@ -1411,6 +1419,7 @@ struct _virDriver {
  virDrvDomainSetTime domainSetTime;
  virDrvNodeGetFreePages nodeGetFreePages;
  virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
 +virDrvDomainListGetStats domainListGetStats;
  };
 
 
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 8349261..bbbc023 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -21341,3 +21341,182 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  virDispatchError(conn);
  return NULL;
  }
 +
 +
 +/**
 + * virConnectGetAllDomainStats:
 + * @conn: pointer to the hypervisor connection
 + * @stats: stats to return, binary-OR of virDomainStatsTypes
 + * @retStats: Pointer that will be filled with the array of returned stats.
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * Query statistics for all domains on a given connection.
 + *
 + * Report statistics of various parameters for a running VM according to 
 @stats
 + * field. The statistics are returned as an array of structures for each 
 queried
 + * domain. The structure contains an array of typed parameters containing the
 + * individual statistics. The typed 

Re: [libvirt] [PATCH] maint: drop spurious semicolons

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 04:31:07PM -0600, Eric Blake wrote:

I noticed a line 'int nparams = 0;;' in remote_dispatch.h, and
tracked down where it was generated.  While at it, I found a
couple of other double semicolons.  Additionally, I noticed that
commit df0b57a95 left a stale reference to the file name
remote_dispatch_bodies.h.

* src/conf/numatune_conf.c (virDomainNumatuneNodeParseXML): Drop
empty statement.
* tests/virdbustest.c (testMessageStruct, testMessageSimple):
Likewise.
* src/rpc/gendispatch.pl (remote_dispatch_bodies.h): Likewise, and
update stale comments.

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

Pushing under the trivial rule.

Syntax check can't easily catch this, because we want to allow
'for (int i = 0;; i++)' on a loop that uses break to exit, so
this is just done by manual inspection.



But trying ';;$' it looks like it can.  And trying it out it seems
there's still one place left.

Before your patch:

 git ls-files | grep '\.[chx]' | xargs grep ';;$'
src/conf/numatune_conf.c:int n = 0;;
src/xenconfig/xen_common.c:return -1;;
tests/virdbustest.c:double in_double = 3.14159265359, out_double = 0;;
tests/virdbustest.c:double in_double = 3.14159265359, out_double = 0;;

And after your patch:
 git ls-files | grep '\.[chx]' | xargs grep ';;$'
src/xenconfig/xen_common.c:return -1;;

Martin


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

Re: [libvirt] [PATCH 1/2] lib: Introduce API for retrieving bulk domain stats

2014-08-25 Thread Li Wei


On 08/26/2014 01:05 AM, Peter Krempa wrote:
 The motivation for this API is that management layers that use libvirt
 usually poll for statistics using various split up APIs we currently
 provide. To get all the necessary stuff, the app needs to issue a lot of
 calls and agregate the results.
 
 The APIs I'm introducing here:
 1) Returns data in a format that we can expand in the future and is
 (pseudo) hierarchical. The data is returned as typed parameters where
 the fields are constructed as dot-separated strings containing names and
 other stuff in a list of typed params.
 
 2) Stats for multiple (all) domains can be queried at once and are
 returned in one call. This will allow to decrease the overhead necessary
 to issue multiple calls per domain multiplied by the count of domains.
 
 3) Selectable (bit mask) fields in the returned format. This will allow
 to retrieve only specific stats according to the apps need.
 
 The stats groups will be enabled using a bit field @stats passed as the
 function argument. A few sample stats groups that this API will support:
 
 VIR_DOMAIN_STATS_STATE
 VIR_DOMAIN_STATS_CPU
 VIR_DOMAIN_STATS_BLOCK
 VIR_DOMAIN_STATS_INTERFACE
 
 the returned typed params will use the following scheme
 
 state.state = running
 state.reason = started
 cpu.count = 8
 cpu.0.state = running
 cpu.0.time = 1234
 ---
  include/libvirt/libvirt.h.in |  26 +++
  src/driver.h |   9 +++
  src/libvirt.c| 179 
 +++
  src/libvirt_public.syms  |   7 ++
  4 files changed, 221 insertions(+)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 47ea695..962f740 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2501,6 +2501,32 @@ int virDomainDetachDeviceFlags(virDomainPtr domain,
  int virDomainUpdateDeviceFlags(virDomainPtr domain,
 const char *xml, unsigned int flags);
 
 +typedef struct _virDomainStatsRecord virDomainStatsRecord;
 +typedef virDomainStatsRecord *virDomainStatsRecordPtr;
 +struct _virDomainStatsRecord {
 +virDomainPtr dom;
 +unsigned int nparams;
 +virTypedParameterPtr params;
 +};
 +
 +typedef enum {
 +VIR_DOMAIN_STATS_ALL = (1  0), /* return all stats fields
 +   implemented in the daemon */
 +VIR_DOMAIN_STATS_STATE = (1  1), /* return domain state */
 +} virDomainStatsTypes;
 +
 +int virConnectGetAllDomainStats(virConnectPtr conn,
 +unsigned int stats,
 +virDomainStatsRecordPtr **retStats,
 +unsigned int flags);
 +
 +int virDomainListGetStats(virDomainPtr *doms,
 +  unsigned int stats,
 +  virDomainStatsRecordPtr **retStats,
 +  unsigned int flags);
 +
 +void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
 +
  /*
   * BlockJob API
   */
 diff --git a/src/driver.h b/src/driver.h
 index ba7c1fc..d5596ab 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -1191,6 +1191,14 @@ typedef int
unsigned int flags);
 
 
 +typedef int
 +(*virDrvDomainListGetStats)(virConnectPtr conn,
 +virDomainPtr *doms,
 +unsigned int ndoms,
 +unsigned int stats,
 +virDomainStatsRecordPtr **retStats,
 +unsigned int flags);
 +
  typedef struct _virDriver virDriver;
  typedef virDriver *virDriverPtr;
 
 @@ -1411,6 +1419,7 @@ struct _virDriver {
  virDrvDomainSetTime domainSetTime;
  virDrvNodeGetFreePages nodeGetFreePages;
  virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
 +virDrvDomainListGetStats domainListGetStats;
  };
 
 
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 8349261..bbbc023 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -21341,3 +21341,182 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  virDispatchError(conn);
  return NULL;
  }
 +
 +
 +/**
 + * virConnectGetAllDomainStats:
 + * @conn: pointer to the hypervisor connection
 + * @stats: stats to return, binary-OR of virDomainStatsTypes
 + * @retStats: Pointer that will be filled with the array of returned stats.
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * Query statistics for all domains on a given connection.
 + *
 + * Report statistics of various parameters for a running VM according to 
 @stats
 + * field. The statistics are returned as an array of structures for each 
 queried
 + * domain. The structure contains an array of typed parameters containing the
 + * individual statistics. The typed parameter name for each statistic field
 + * consists of a dot-separated string containing name of the requested group
 + * followed by a group specific description of the statistic value.

Re: [libvirt] [PATCH] maint: drop spurious semicolons

2014-08-25 Thread Eric Blake
On 08/25/2014 08:52 PM, Martin Kletzander wrote:
 On Mon, Aug 25, 2014 at 04:31:07PM -0600, Eric Blake wrote:
 I noticed a line 'int nparams = 0;;' in remote_dispatch.h, and
 tracked down where it was generated.  While at it, I found a
 couple of other double semicolons.  Additionally, I noticed that
 commit df0b57a95 left a stale reference to the file name
 remote_dispatch_bodies.h.



 Syntax check can't easily catch this, because we want to allow
 'for (int i = 0;; i++)' on a loop that uses break to exit, so
 this is just done by manual inspection.

 
 But trying ';;$' it looks like it can.  And trying it out it seems
 there's still one place left.

Oh cool, looks like I have a followup patch to write, then.


 And after your patch:
  git ls-files | grep '\.[chx]' | xargs grep ';;$'
 src/xenconfig/xen_common.c:return -1;;
 
 Martin

-- 
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] [PATCH 3/4] domain_conf: Add support for iothreads in disk definition

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 08:38:08PM -0400, John Ferlan wrote:

Add a new disk driver attribute iothread to be parsed as the thread
number for the disk to use. In order to more easily facilitate the usage
and configuration of the iothread, a zero for the attribute indicates
iothreads are not supported for the device and a positive value indicates
the specific thread to try and use.

Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in |  8 
docs/schemas/domaincommon.rng | 13 +
src/conf/domain_conf.c| 24 +++-
src/conf/domain_conf.h|  2 ++
4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b4ac483..40891e4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1572,6 +1572,9 @@
  optional
ref name=discard/
  /optional
+  optional
+ref name=driverIOThread/
+  /optional
  empty/
/element
  /define
@@ -1659,6 +1662,11 @@
  /choice
/attribute
  /define
+  define name=driverIOThread
+attribute name='iothread'
+  ref name=iothreadsid/


Again, what's the difference to just ref name=unsignedInt?


+/attribute
+  /define
  define name=controller
element name=controller
  attribute name=index
@@ -4759,6 +4767,11 @@
  param name=minInclusive0/param
/data
  /define
+  define name=iothreadsid
+data type=unsignedInt
+  param name=pattern[0-9]+/param
+/data
+  /define
  define name=vcpuid
data type=unsignedShort
  param name=pattern[0-9]+/param
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 671c41c..b15f279 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c

[...]

@@ -11968,6 +11980,14 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
def-iothreads = count;
+
+/* Create a bitmap for inuse threads - noting that entries are
+ * numbered 1..def-iothreads since 0 (zero) iothreads means
+ * nothing and assigning a disk to an IOThread requires at least a
+ * thread#  0 since a zero would indicate no IOThread for the disk
+ */
+if (!(def-iothreadmap = virBitmapNew(def-iothreads+1)))
+goto error;


virBitmapFree(def-iothreadmap) is missing in virDomainDefFree().

Martin


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

Re: [libvirt] [PATCH 2/4] qemu: Add support for iothreads

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 08:38:07PM -0400, John Ferlan wrote:

Add a new capability to ensure the iothreads feature exists for the qemu
emulator being run - requires the query-iothreads QMP command. Using the
domain XML add correspoding command argument in order to generate the
threads. The iothreads will use a name space libvirtIothread# where, the
future patch to add support for using an iothread to a disk definition to
merely define which of the available threads to use.

Add tests to ensure the xml/argv processing is correct.  Note that no
change was made to qemuargv2xmltest.c as processing the -object element
would require knowing more than just iothreads.

Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_capabilities.c   |  2 ++
src/qemu/qemu_capabilities.h   |  1 +
src/qemu/qemu_command.c| 13 ++
tests/qemuxml2argvdata/qemuxml2argv-iothreads.args |  8 ++
tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml  | 29 ++
tests/qemuxml2argvtest.c   |  2 ++
tests/qemuxml2xmltest.c|  1 +
7 files changed, 56 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 410086b..b331be7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -268,6 +268,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
  rtc-reset-reinjection,

  splash-timeout, /* 175 */
+  query-iothreads,


Why query- when the capability is _OBJECT_?  Or is this just a typo?
/bikeshed


);


@@ -1430,6 +1431,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ nbd-server-start, QEMU_CAPS_NBD_SERVER },
{ change-backing-file, QEMU_CAPS_CHANGE_BACKING_FILE },
{ rtc-reset-reinjection, QEMU_CAPS_RTC_RESET_REINJECTION },
+{ query-iothreads, QEMU_CAPS_OBJECT_IOTHREAD},


We have virQEMUCapsObjectTypes[] where you can just stick the name of
the object you want to test, and not rely on a related command to
probe for it.


};

struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 48a4eaa..0980c00 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -215,6 +215,7 @@ typedef enum {
QEMU_CAPS_OBJECT_USB_AUDIO   = 173, /* usb-audio device support */
QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor 
command */
QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
+QEMU_CAPS_OBJECT_IOTHREAD= 176, /* -object iothread */

QEMU_CAPS_LAST,   /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6dac9d3..287a3f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7510,6 +7510,19 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, smp);
VIR_FREE(smp);

+if (def-iothreads  0 
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+/* Create named iothread objects starting with 1. These may be used
+ * by a disk definition which will associate to an iothread by
+ * supplying a value of 1 up to the number of iothreads available
+ * (since 0 would indicate to not use the feature).
+ */
+for (i = 1; i = def-iothreads; i++) {
+virCommandAddArg(cmd, -object);
+virCommandAddArgFormat(cmd, iothread,id=libvirtIothread%zu, i);


I don't see we would use 'libvirt*' naming for any other IDs,
'iothread%zu' would be enough, I guess (and the command-line wouldn't
be so long as well).

Martin


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

Re: [libvirt] [PATCH 1/4] domain_conf: Introduce iothreads XML

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 08:38:06PM -0400, John Ferlan wrote:

Introduce XML to allowing adding iothreads to the domain. These can be
used by virtio-blk-pci devices in order to assign a specific thread to
handle the workload for the device.  The iothreads are the official
implementation of the virtio-blk Data Plane that's been in tech preview
for QEMU.

Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in | 26 ++
docs/schemas/domaincommon.rng | 12 
src/conf/domain_conf.c| 28 
src/conf/domain_conf.h|  2 ++
4 files changed, 68 insertions(+)


[...]

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9a89dd8..b4ac483 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -632,6 +632,12 @@
  /optional

  optional
+element name=iothreads
+  ref name=countIOThreads/


What's the difference between this and using ref name=unsignedInt
directly?


+/element
+  /optional
+
+  optional
ref name=blkiotune/
  /optional

@@ -4747,6 +4753,12 @@
  param name=minInclusive1/param
/data
  /define
+  define name=countIOThreads
+data type=unsignedInt
+  param name=pattern[0-9]+/param
+  param name=minInclusive0/param
+/data
+  /define
  define name=vcpuid
data type=unsignedShort
  param name=pattern[0-9]+/param
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 22a7f7e..671c41c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11953,6 +11953,23 @@ virDomainDefParseXML(xmlDocPtr xml,
}
}

+/* Optional - iothreads */
+n = virXPathULong(string(./iothreads[1]), ctxt, count);
+if (n == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(iothreads count must be an integer));
+goto error;
+} else if (n  0) {
+def-iothreads = 0;
+} else {
+if ((unsigned int) count != count) {


Instead of this machinery, it would be more straightforward to just do
(example written by hand, not tested):

tmp = virXPathString(string(./iothreads[1]), ctxt);
if (tmp  virStrToLong_uip(tmp, NULL, 0, def-iothreads)  0)
virReportError(VIR_ERR_XML_ERROR, _(invalid iothreads count '%s'), tmp);

Martin


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

Re: [libvirt] [PATCH 4/4] qemu: Allow use of iothreads for disk definitions

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 08:38:09PM -0400, John Ferlan wrote:

For virtio-blk-pci disks with the disk iothread attribute that are
running the correct emulator, add the iothread=libvirtIothread# to
the -device command line in order to enable iothreads for the disk.

This code will check both the start and hotplug paths for the capability,
whether the iothreadsmap has been defined, and whether there's an available
iothread to be used for the disk.

Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_command.c| 35 +++
src/qemu/qemu_command.h|  5 +++
src/qemu/qemu_hotplug.c| 11 ++
.../qemuxml2argv-iothreads-disk.args   | 17 +
.../qemuxml2argv-iothreads-disk.xml| 40 ++
tests/qemuxml2argvtest.c   |  2 ++
tests/qemuxml2xmltest.c|  1 +
7 files changed, 111 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 287a3f3..740b6ec 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3862,6 +3862,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
virBufferAddLit(opt, virtio-blk-device);
} else {
virBufferAddLit(opt, virtio-blk-pci);
+if (disk-iothread  0)
+virBufferAsprintf(opt, ,iothread=libvirtIothread%u,
+  disk-iothread);


You are using the def-iothread only to format it with virtio disks,
but ... [1]


}
qemuBuildIoEventFdStr(opt, disk-ioeventfd, qemuCaps);
if (disk-event_idx 
@@ -6614,6 +6617,31 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
}


+bool
+qemuDomainIothreadsAvailable(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainDiskDefPtr disk)
+{
+bool inuse;
+const char *src = virDomainDiskGetSource(disk);
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD) ||
+!def-iothreadmap) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(IOThreads not supported for this QEMU 
+ for disk '%s'), src);
+return false;
+}
+if (virBitmapGetBit(def-iothreadmap, disk-iothread, inuse) || inuse) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(IOThread '%u' for disk '%s' is already 
+ being used), disk-iothread, src);


Some disks may not have source set (empty cdrom drive), so it should
probably be NULLSTR(src), but I think IOThread '%u' used multiple
times is more than enough.

This leads me to a code simplification idea.  If you checked
QEMU_CAPS_OBJECT_IOTHREAD when creating the iothread objects it's
enough to make virBitmapGetBit() handle NULL bitmap carefully and then
just virBitmapGetBit() the bit the same way.  And if the message does
not output the disk name (src), this whole function can be thrown away
and instead of doing:

if (!qemuDomainIothreadsAvailable(def, qemuCaps, disk))
goto error;

you'd do:

if (virBitmapGetBit(def-iothreadmap, disk-iothread, tmp) || tmp)
virReportError(...);

Just an idea, though, not a requirement ;)  If you keep it in a
separate function, though, I'd suggest checking the range and
reporting specific error, because when testing I got for example this
error:

IOThread '3' for disk '(null)' is already being used

even though it clearly wasn't; I just had only 1 or 2 I/O threads
created.


+return false;
+}
+return true;
+}
+
+
static int
qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
virCommandPtr cmd,
@@ -8055,6 +8083,13 @@ qemuBuildCommandLine(virConnectPtr conn,
   disk-src-driverName, disk-src-path);
goto error;
}
+
+/* Check iothreads relationship */
+if (disk-iothread  0) {
+if (!qemuDomainIothreadsAvailable(def, qemuCaps, disk))
+goto error;
+ignore_value(virBitmapSetBit(def-iothreadmap, disk-iothread));
+}
}



[1] ... here you check it for all disks.  And it's kept in the domain
definition for all disks as well.  Maybe removing it from unsupported
disks and checking(+building) the iothreadmap could be done in
qemuDomainDefPostParse().


if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 633ff71..bec6c14 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -81,6 +81,11 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
   bool forXMLToArgv)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);

+bool

Re: [libvirt] [PATCH] blockcopy: allow block device destination

2014-08-25 Thread Martin Kletzander

On Mon, Aug 25, 2014 at 09:55:28AM -0400, Adam Litke wrote:

On 22/08/14 14:26 -0600, Eric Blake wrote:

Ping


Sorry, I thought I'd responded to this.


On 08/13/2014 02:00 PM, Eric Blake wrote:

To date, anyone performing a block copy and pivot ends up with
the destination being treated as disk type='file'.  While this
works for data access for a block device, it has at least one
noticeable shortcoming: virDomainGetBlockInfo() reports allocation
differently for block devices visited as files (the size of the
device) than for block devices visited as disk type='block'
(the maximum sector used, as reported by qemu); and this difference
is significant when trying to manage qcow2 format on block devices
that can be grown as needed.

I still plan to add a virDomainBlockCopy() API that takes the
destination disk as XML, allowing full expressive capability
to copy to a network disk.  But a new API can't be backported,
while a new flag to an existing API can.  So this patch enhances
blockcopy to let the user flag when the resulting XML after the
copy must list the device as type='block'.


Is there any situation where you would not want this behavior?  The
only thing I can think of is that we need to keep the current behavior
for backwards compatibility.  If this is the case, then I'd say the
patch looks reasonable to me.


One more question... What happens if this flag is used with a drive of
type file?  Can we raise an error in that case?



I understand it as it makes sense to do that too, so no error should
be reported.  That's because the flag depends on the *target* disk,
not the source one and libvirt can't (easily and reliably) say what
type that should be.

I haven't checked whether it works as it should, but the patch looks
good to me, so ACK from me if my above understanding is correct.

Martin


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