Re: [libvirt] [PATCH 0/3] storage: zfs: implement download and upload
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
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
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
- 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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
- 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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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