[libvirt] Release of libvirt-1.2.14
It's out ! The release is tagged in git, signed tarballs and packages are available from the usual place: ftp://libvirt.org/libvirt/ I also tagged and pushed a libvirt-python-1.2.14 release at: ftp://libvirt.org/libvirt/python/ This is a rather large release, around 350 commits, in a balanced set: a few new features, a reasonable set of fixes and a bunch of improvements :-) Features: - qemu: Implement memory device hotplug (Peter Krempa) - Implement public API for virDomainPinIOThread (John Ferlan) - Implement public API for virDomainGetIOThreadsInfo (John Ferlan) - SRIOV NIC offload feature discovery (James Chapman) Documentation: - virCgroupNewPartition: Fix comment (Michal Privoznik) - route element must specify network address (Chen Fan) - no 'via' attribute in route element (Chen Fan) - Document that USB hostdevs do not need nodeDettach (Ján Tomko) - Document behavior of compat when creating qcow2 volumes (Ján Tomko) - Fix typo in error message (Ján Tomko) - Fix common misspellings (Martin Kletzander) - Fix doc for backingStore (Deepak Shetty) - schema and docs for the midonet virtualport type (Antoni Segura Puimedon) - add a note that spice channel is usable only with spice graphics (Pavel Hrdina) - net-define: update or unify documentation (Pavel Hrdina) - pool-define: update and unify documentation (Pavel Hrdina) - iface-define: update and unify documentation (Pavel Hrdina) - Fix syntax for vcpupin description (John Ferlan) - virsh.pod: Add information regarding LXC for setmem, memtune, and dominfo (John Ferlan) - add a note that attr 'managed' is only used by PCI devices (Erik Skultety) Portability: - build: avoid variable named 'interface', for mingw (Eric Blake) - vircgroup: Fix build issue mingw cross compile (John Ferlan) - vircgroup: Fix build issue on mingw cross compile (John Ferlan) - virnetdev: fix build with old kernel (Pavel Hrdina) - Fix build on mingw (Ján Tomko) Bug fixes: - qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT (Peter Krempa) - qemu: processBlockJob: Don't unlock @vm twice (Peter Krempa) - qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase (Peter Krempa) - virsh: blockCopy: Add missing jump on error path (Luyao Huang) - qemuDomainGetNumaParameters: Check for the correct CGroup controller (Michal Privoznik) - virCgroupController: Check the enum fits into 'int' (Michal Privoznik) - virnetlink: fix build error (Pavel Hrdina) - qemu: end the job when try to blockcopy to non-file destination (Shanzhi Yu) - relaxng: allow : in /dev/disk/by-path names (Eric Blake) - libxl: Fix memory leak if pthread_create fails. (Konrad Rzeszutek Wilk) - util: use netlink to delete bridge devices (Laine Stump) - qemu: command: Fix property name for start address of a pc-dimm module (Luyao Huang) - qemu: command: Check for empty network source when formatting drive cmd (Peter Krempa) - qemu: command: Report error when formatting network source with protocol _NONE (Peter Krempa) - build: fix race when creating the cpu_map.xml symlink (Amy Fong) - Don't validata filesystem target type (Guido Günther) - rpc: Don't unref identity object while callbacks still can be executed (Peter Krempa) - util: identity: Harden virIdentitySetCurrent() (Peter Krempa) - qemu: domain: Don't leak device alias list (Peter Krempa) - rpc: serverclient: Clear pointer with NULL instead of 0 (Peter Krempa) - qemu: fix set vcpus on host without NUMA (Pavel Hrdina) - qemu: monitor: Don't leak @props with non-JSON in qemuMonitorAddObject (Peter Krempa) - qemu: Don't return memory device config on error in qemuBuildMemoryBackendStr (Peter Krempa) - Fix underlinking of libvirt_driver_interface.so (Natanael Copa) - networkStateInitialize: Don't lock network driver (Michal Privoznik) - qemu: Migrate memory on numatune change (Martin Kletzander) - parallels: fix libvirt crash if parallelsNetworkOpen fails (Maxim Nestratov) - qemu: do not overwrite the error in qemuDomainObjExitMonitor (Luyao Huang) - libxl: Don't overwrite errors from xenconfig (Jim Fehlig) - qemu: Fix two issues in qemuDomainSetVcpus error handling (John Ferlan) - qemu: track 'cancelling' migration state (Eric Blake) - parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE (Maxim Nestratov) - parallels: switch off offline management feature (Maxim Nestratov) - parallels: set correct network adapter link state (Maxim Nestratov) - parallels: fix parallelsLoadNetworks (Maxim Nestratov) - network: Resolve Coverity FORWARD_NULL (John Ferlan) - qemuGetDHCPInterfaces: Don't leak @network (Michal Privoznik) - cmdDomIfAddr: Free @ip_addr_str (Michal Privoznik) - qemu: read backing chain names from qemu (Eric Blake) - qemu: driver: Fix cold-update of removable storage devices (Peter Krempa) - parallels: fix home directory for VMs (Maxim Nestratov) - parallels: don't forget to unlock domain if unregister fails (Maxim Nestratov) - parallels: set cpu mode when applying xml configuration
[libvirt] [PATCH 0/2] rework the check for the numa cpus
We introduce a check for numa cpu total count in 5f7b71, But seems this check cannot work well. There are some scenarioes: 1. one of cpu id is out of maxvcpus, can set success(cpu count = 5 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-3,100' memory='512000' unit='KiB'/ the cpus '100' exceed maxvcpus, this setting is not valid (although qemu do not output error). 2. use the same cpu in 2 cell, can set success(cpu count = 8 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-3' memory='512000' unit='KiB'/ cell id='1' cpus='0-3' memory='512000' unit='KiB'/ I guess nobody will use this setting if he really want use numa in his vm. (qemu not output error, but we can find some interesting things in vm, all cpus have bind in one numa node) 3. use the same cpu in 2 cell, cannot set success(cpu count = 11 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-6' memory='512000' unit='KiB'/ cell id='1' cpus='0-3' memory='512000' unit='KiB'/ No need forbid this scenario if scenario 2 is a 'valid' setting. However add new check during parse xml will make vm has broken settings disappear after update libvirtd, so possible solutions: 1. add new check when parse xml, and find a way to avoid vm evaporate. I chose this way and i don't think just drop the invalid settings is a good idea for numa cpus, so i add a new function. 2. add new check when start vm. I think this is a configuration issue, so no need report it so late. 3. just remove this check and do not add new check... :) Luyao Huang (2): conf: introduce a new function to add check avoid losing track conf: rework the cpu check for vm numa settings src/bhyve/bhyve_driver.c | 4 ++-- src/conf/domain_conf.c | 21 ++--- src/conf/domain_conf.h | 1 + src/conf/numa_conf.c | 37 ++--- src/conf/numa_conf.h | 2 +- src/esx/esx_driver.c | 2 +- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_driver.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/parallels/parallels_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- src/uml/uml_driver.c | 4 ++-- src/vbox/vbox_common.c | 2 +- src/vmware/vmware_driver.c | 4 ++-- src/xen/xen_driver.c | 4 ++-- src/xenapi/xenapi_driver.c | 4 ++-- 18 files changed, 70 insertions(+), 39 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] conf: rework the cpu check for vm numa settings
https://bugzilla.redhat.com/show_bug.cgi?id=1176020 We had a check for the vcpu count total number in numa before, however this check is not good enough. There are some examples: 1. one of cpu id is out of maxvcpus, can set success(cpu count = 5 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-3,100' memory='512000' unit='KiB'/ 2. use the same cpu in 2 cell, can set success(cpu count = 8 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-3' memory='512000' unit='KiB'/ cell id='1' cpus='0-3' memory='512000' unit='KiB'/ 3. use the same cpu in 2 cell, cannot set success(cpu count = 11 10): vcpu placement='static'10/vcpu cell id='0' cpus='0-6' memory='512000' unit='KiB'/ cell id='1' cpus='0-3' memory='512000' unit='KiB'/ Use a new check for numa cpus, check if use a cpu exceeds maxvcpus and if duplicate use one cpu in more than one cell. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 12 src/conf/numa_conf.c | 37 ++--- src/conf/numa_conf.h | 2 +- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 831f033..3a5eb1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13510,7 +13510,10 @@ virDomainThreadSchedParse(xmlNodePtr node, static int virDomainDefNewCheck(virDomainDefPtr def) { -/*TO DO*/ +if (def-numa +(virDomainNumaCheckCPU(def-numa, def-maxvcpus) 0)) +return -1; + return 0; } @@ -14141,13 +14144,6 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainNumaDefCPUParseXML(def-numa, ctxt) 0) goto error; -if (virDomainNumaGetCPUCountTotal(def-numa) def-maxvcpus) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Number of CPUs in numa exceeds the - vcpu count)); -goto error; -} - if (virDomainNumatuneParseXML(def-numa, def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 8a0f686..0191a22 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -790,16 +790,39 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, } -unsigned int -virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa) +int +virDomainNumaCheckCPU(virDomainNumaPtr numa, + unsigned short maxvcpus) { -size_t i; -unsigned int ret = 0; +size_t i,j; -for (i = 0; i numa-nmem_nodes; i++) -ret += virBitmapCountBits(virDomainNumaGetNodeCpumask(numa, i)); +for (i = 0; i numa-nmem_nodes; i++) { +virBitmapPtr nodeset = NULL; +ssize_t bit = -1; + +nodeset = virDomainNumaGetNodeCpumask(numa, i); +for (j = 0; j i; j++) { +if (virBitmapOverlaps(virDomainNumaGetNodeCpumask(numa, j), + nodeset)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Cannot binding one vCPU in 2 NUMA cell + %d and %d), (int) i, (int) j); +return -1; +} +} -return ret; +while ((bit = virBitmapNextSetBit(nodeset, bit)) = 0) { +if (bit = maxvcpus-1) +continue; + +virReportError(VIR_ERR_INTERNAL_ERROR, + _(vcpu '%d' in numa cell '%d' exceeds the maxvcpus), + (int) bit, (int) i); +return -1; +} +} + +return 0; } diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index ded6e01..d7713c8 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -149,7 +149,7 @@ bool virDomainNumatuneNodeSpecified(virDomainNumaPtr numatune, int virDomainNumaDefCPUParseXML(virDomainNumaPtr def, xmlXPathContextPtr ctxt); int virDomainNumaDefCPUFormat(virBufferPtr buf, virDomainNumaPtr def); -unsigned int virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa); +int virDomainNumaCheckCPU(virDomainNumaPtr numa, unsigned short maxvcpus); #endif /* __NUMA_CONF_H__ */ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: introduce a new function to add check avoid losing track
Add new check in the new function will avoid some vm disappear after restart libvirtd, and the new check will be called when define or create a vm. And after some version, maybe we can move these check to the right place. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/bhyve/bhyve_driver.c | 4 ++-- src/conf/domain_conf.c | 11 +++ src/conf/domain_conf.h | 1 + src/esx/esx_driver.c | 2 +- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_driver.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/parallels/parallels_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- src/uml/uml_driver.c | 4 ++-- src/vbox/vbox_common.c | 2 +- src/vmware/vmware_driver.c | 4 ++-- src/xen/xen_driver.c | 4 ++-- src/xenapi/xenapi_driver.c | 4 ++-- 16 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 21db277..3cea3d6 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -495,7 +495,7 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virDomainObjPtr vm = NULL; virObjectEventPtr event = NULL; virCapsPtr caps = NULL; -unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_NEW_CHECK; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -895,7 +895,7 @@ bhyveDomainCreateXML(virConnectPtr conn, virObjectEventPtr event = NULL; virCapsPtr caps = NULL; unsigned int start_flags = 0; -unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_NEW_CHECK; virCheckFlags(VIR_DOMAIN_START_AUTODESTROY | VIR_DOMAIN_START_VALIDATE, NULL); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cd6ee22..831f033 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13507,6 +13507,13 @@ virDomainThreadSchedParse(xmlNodePtr node, return -1; } +static int +virDomainDefNewCheck(virDomainDefPtr def) +{ +/*TO DO*/ +return 0; +} + static virDomainDefPtr virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr root, @@ -15398,6 +15405,10 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainDefPostParse(def, caps, xmlopt) 0) goto error; +if (flags VIR_DOMAIN_DEF_PARSE_NEW_CHECK +virDomainDefNewCheck(def) 0) +goto error; + /* Auto-add any implied controllers which aren't present */ if (virDomainDefAddImplicitControllers(def) 0) goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 84e880a..21341f7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2528,6 +2528,7 @@ typedef enum { /* parse only source half of disk */ VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 7, VIR_DOMAIN_DEF_PARSE_VALIDATE= 1 8, +VIR_DOMAIN_DEF_PARSE_NEW_CHECK = 1 9, } virDomainDefParseFlags; typedef enum { diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 179f44c..db0a309 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3042,7 +3042,7 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; const char *src; -unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_NEW_CHECK; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 05f6eb1..23eb7e4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -862,7 +862,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); -unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_NEW_CHECK; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_VALIDATE, NULL); @@ -2600,7 +2600,7 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virDomainPtr dom = NULL; virObjectEventPtr event = NULL; virDomainDefPtr oldDef = NULL; -unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_NEW_CHECK; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 245000d..38b4206 100644 --- a/src/lxc/lxc_driver.c +++
Re: [libvirt] [PATCH 1/6] Remove unused macros
On Wed, Apr 01, 2015 at 07:30:22PM +0200, Peter Krempa wrote: On Wed, Apr 01, 2015 at 17:32:28 +0200, Ján Tomko wrote: In the order of appearance: * MAX_LISTEN - never used added by 23ad665c (qemud) and addec57 (lock daemon) * NEXT_FREE_CLASS_ID - never used, added by 07d1b6b * virLockError - never used, added by eb8268a4 * OPENVZ_MAX_ARG, CMDBUF_LEN, CMDOP_LEN unused since the removal of ADD_ARG_LIT in d8b31306 * QEMU_NB_PER_CPU_STAT_PARAM - unused since 897808e * QEMU_CMD_PROMPT, QEMU_PASSWD_PROMPT - unused since 1dc10a7 * TEST_MODEL_WORDSIZE - unused since c25c18f7 * TEMPDIR - never used, added by 714bef5 * NSIG - workaround around old headers added by commit 60ed1d2 unused since virExec was moved by commit 02e8691 * DO_TEST_PARSE - never used, added by 9afa006 * DIFF_MSEC, GETTIMEOFDAY - unused since eee6eb6 Wow, you bothered with electronic archeology? :) --- daemon/libvirtd.c | 1 - src/conf/network_conf.c | 1 - src/locking/lock_daemon.c | 1 - src/locking/lock_driver_lockd.c | 4 src/openvz/openvz_driver.c | 4 src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor_text.c| 3 --- src/test/test_driver.c | 1 - src/uml/uml_driver.c| 3 --- src/util/virutil.c | 4 tests/sockettest.c | 10 -- tests/testutils.c | 5 - 12 files changed, 39 deletions(-) ACK (after the release) Do we have to explictly state that in the freeze? I pushed the series now, thanks for the review. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python PATCH] Post-release version bump to 1.2.15
Signed-off-by: Jiri Denemark jdene...@redhat.com --- Pushed as trivial. setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 00e6d75..79b048f 100755 --- a/setup.py +++ b/setup.py @@ -309,7 +309,7 @@ class my_clean(clean): _c_modules, _py_modules = get_module_lists() setup(name = 'libvirt-python', - version = '1.2.14', + version = '1.2.15', url = 'http://www.libvirt.org', maintainer = 'Libvirt Maintainers', maintainer_email = 'libvir-list@redhat.com', -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On Wed, 1 Apr 2015 20:05:24 -0300 Eduardo Habkost ehabk...@redhat.com wrote: If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a stable-abi-safe field on CpuDefinitionInfo? Exactly that fulfills the name field for s390 already in my view. And cpu model none just means that QEMU does not manage the cpu model. That's also the reason why I initially returned an empty [] model and not none. This somewhat convinces me to go back to this approach... I understand the reasons for your approach and it seems to work for s390, but the only problem I see is that you are adding an additional (undocumented?) s390-specific constraint to the semantics of query-cpu-models: that the model name will appear on the list only if it can be safely migratable. This may prevent us from unifying CPU model code into generic code later. I agree that an aliases is something different compared with the CPU model none as there is a CPU class representing it. And thus, when implicitly or explicitly selected, shall be presented in the CPU definition list as well. If I would set runnable to false as it now (bad), it would be sorted out by the considered for migration test but it would be misleading as it is always runnable. Though an additional field like migrate-able could express that characteristic. But if we add a simple stable-abi-safe field to the list (even if s390 set it to to true for all models and omit aliases and none in this first version), we will have clearer semantics that can still be honoured by other architectures (and by generic code) later. To be honest I currently don't right get the idea that you follow with that stable-abi-save field... But eventually yes (I wrote this before the section above) The stable-abi-save field means: Take me into account for whatever kind of CPU model related comparison you perform between two running QEMU instances as I represent a well defined aspect. Thus CPU model none will be { name: none, runnable: true, stable-abi-save: false } and the aliases can be represented as { name: alias, runnable: true|false, stable-abi-save: false } in the s390 case, right? Michael -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: remove stale comment
Copied from the vcpupin command, which has two modes of operation. --- Pushed as trivial. tools/virsh-domain.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index eee441f..e7b5eeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6961,7 +6961,6 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) goto cleanup; cpumaplen = VIR_CPU_MAPLEN(maxcpu); -/* Pin mode: pinning specified vcpu to specified physical cpus*/ if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen))) goto cleanup; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Bump version to 1.2.15 for new dev cycle
--- Pushed as trivial. configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index b1193a6..38fbbad 100644 --- a/configure.ac +++ b/configure.ac @@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General Public dnl License along with this library. If not, see dnl http://www.gnu.org/licenses/. -AC_INIT([libvirt], [1.2.14], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt], [1.2.15], [libvir-list@redhat.com], [], [http://libvirt.org]) AC_CONFIG_SRCDIR([src/libvirt.c]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] conf: Rename virDomainHasDiskMirror and detect block jobs properly
On Mon, Mar 30, 2015 at 14:33:09 -0600, Eric Blake wrote: On 03/30/2015 12:50 PM, Peter Krempa wrote: From: Shanzhi Yu s...@redhat.com virDomainHasDiskMirror() currently detects only jobs that add the mirror elements. Since some operations like migration are interlocked by existing block jobs on the given domain the check needs to be instrumented to check regular jobs too. This patch renames virDomainHasDiskMirror to virDomainHasDiskBlockjob and adds an argument that allows to select that it returns true only for block copy jobs as those interlock making the domain persistent. Other two uses trigger on any block job type. Signed-off-by: Shanzhi Yu s...@redhat.com Signed-off-by: Peter Krempa pkre...@redhat.com --- src/conf/domain_conf.c| 25 - src/conf/domain_conf.h| 3 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c| 6 +++--- src/qemu/qemu_migration.c | 2 +- 5 files changed, 27 insertions(+), 11 deletions(-) ACK; series should be safe for freeze as a bug fix. I've missed the rc-2 deadline so I've pushed them now that the release is out. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Fix cgroups regresion when default cpuset is specified
On Tue, Mar 31, 2015 at 16:14:53 +0200, Ján Tomko wrote: On Fri, Mar 27, 2015 at 02:12:04PM +0100, Peter Krempa wrote: Since commit a39f69d2b libvirt would fail to start a VM if the default cpu set was specified and individual vcpus were pinned to cpus outside of that cpuset. Peter Krempa (8): qemu: cgroup: Store auto cpuset instead of re-creating it on demand qemu: cgroup: Refactor setup for IOThread cgroups qemu: cgroup: Properly set up vcpu pinning qemu: cgroup: Use priv-autoCpuset instead of using qemuPrepareCpumap() qemu: cgroup: Rename qemuSetupCgroupEmulatorPin to qemuSetupCgroupCpusetCpus qemu: cgroup: Kill qemuSetupCgroupIOThreadsPin() qemu: cgroup: Kill qemuSetupCgroupVcpuPin() qemu: Copy bitmap in a sane way src/qemu/qemu_cgroup.c | 150 +++- src/qemu/qemu_cgroup.h | 13 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 21 +++ src/qemu/qemu_process.c | 93 +- src/qemu/qemu_process.h | 2 - 7 files changed, 85 insertions(+), 198 deletions(-) ACK series. Thanks. Now that the release is out I've pushed the patches. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt-1.2.14 candidate release 2
On Thu, Apr 02, 2015 at 06:51:35PM +0200, Guido Günther wrote: Hi, On Tue, Mar 31, 2015 at 09:01:33AM +0800, Daniel Veillard wrote: So I have just tagged in git and pushed the 1.2.14 candidate release 2, it is available as usual as signed tarballs and rpms from: ftp://libvirt.org/libvirt/ I did push the 3 patches from Peter because what is the point of a candidate release if it doesn't include the potentially controversial bits that we intend for the final release ? That doesn't prevent someone else than Eric to give the 2nd ACK for patch 3 of the series :-) So please git it a try, if needed we will just do an rc3 ! BTW is there regression testing done from oVirt using libvirt upstream on an automated basis ? If not that's something I could help pushing for assuming we have public reg tests for oVirt. At least it seems to work in my minimal own tests, but others should check, Looks good on Debian's Buildds: https://buildd.debian.org/status/package.php?p=libvirtsuite=experimental thanks for the feedback, but release is out, might want to push the 1.2.14 final not rc2 :) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] fail out if enable userns but disable netns
-Original Message- From: Chen, Hanxiao/陈 晗霄 Sent: Thursday, March 26, 2015 10:49 AM To: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com Subject: RE: [libvirt] [PATCH 0/2] fail out if enable userns but disable netns -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Monday, March 23, 2015 11:46 AM To: libvir-list@redhat.com Subject: [libvirt] [PATCH 0/2] fail out if enable userns but disable netns Chen Hanxiao (2): Revert LXC: create a bind mount for sysfs when enable userns but disable netns LXC: make sure netns been enabled when trying to enable userns src/lxc/lxc_container.c | 45 - 1 file changed, 16 insertions(+), 29 deletions(-) ping ping Regards, - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] conf: Introduce virStoragePoolSaveXML
On Thu, Apr 02, 2015 at 12:10:36 +0200, Erik Skultety wrote: Make XML definition saving more generic by moving the common code into virStoragePoolSaveXML and leave case specific code to PoolSave{Status,Config,...} functions. --- src/conf/storage_conf.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a8e9876..73b937e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1913,11 +1913,25 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, return ret; } + +static int virStoragePoolSaveXML(const char *path, + virStoragePoolDefPtr def, + const char *xml) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; +int ret = -1; + +virUUIDFormat(def-uuid, uuidstr); +ret = virXMLSaveFile(path, + virXMLPickShellSafeComment(def-name, uuidstr), + pool-edit, xml); + +return ret; +} int Missing empty lines between functions. virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def) { -char uuidstr[VIR_UUID_STRING_BUFLEN]; char *xml; int ret = -1; @@ -1927,12 +1941,12 @@ virStoragePoolSaveConfig(const char *configFile, return -1; } -virUUIDFormat(def-uuid, uuidstr); -ret = virXMLSaveFile(configFile, - virXMLPickShellSafeComment(def-name, uuidstr), - pool-edit, xml); -VIR_FREE(xml); +if (virStoragePoolSaveXML(configFile, def, xml)) +goto cleanup; +ret = 0; + cleanup: +VIR_FREE(xml); return ret; } ACK if you add at least one new line between virStoragePoolSaveConfig and virStoragePoolSaveXML Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 6/6] iscsi: Add checks for non standard stable target.path
On Wed, Apr 01, 2015 at 01:29:11PM -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1171933 If a non stable path is provided for the pool's target path, check to see if the directory exists before allowing pool startup; otherwise, The only non-stable path that can possibly result in a non-empty pool is '/dev/'. later in the processLU calls to find LUN's all that happens is the volume target.path will get the strdup'd value of the pool target.path (which doesn't exist), so attempts to find the LU are unsuccessful resulting in a started pool with no devices listed even though the block devices for the iSCSI LU's do exist. Additionally if the non stable path does exist and it's determined no targets are found, then force failure in the refresh path. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_iscsi.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index fba037f..b5a15b1 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -149,6 +149,15 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, if (virStorageBackendSCSIFindLUs(pool, host) 0) goto cleanup; +if (pool-volumes.count == 0 This is always true if the path does not start with /dev. https://www.redhat.com/archives/libvir-list/2015-March/msg01578.html +!STRPREFIX(pool-def-target.path, /dev)) { +virReportError(VIR_ERR_INVALID_ARG, + _(Non stable target path '%s' for pool '%s' + found no target volumes), + pool-def-target.path, pool-def-name); +return -1; +} + retval = 0; cleanup: @@ -393,6 +402,15 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, return -1; } +if (!STRPREFIX(pool-def-target.path, /dev) +!virFileExists(pool-def-target.path)) { If you remove the virFileExists condition, you don't need the error check in the first hunk. Jan +virReportError(VIR_ERR_INVALID_ARG, + _(Non stable target path '%s' not found for pool '%s'), + pool-def-target.path, pool-def-name); +return -1; +} + + if ((session = virStorageBackendISCSISession(pool, true)) == NULL) { if ((portal = virStorageBackendISCSIPortal(pool-def-source)) == NULL) goto cleanup; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] scsi: Adjust return value for virStorageBackendSCSINewLun
On 04/02/2015 07:31 AM, Ján Tomko wrote: On Wed, Apr 01, 2015 at 01:29:08PM -0400, John Ferlan wrote: Add a return -2 to differentiate that the failure was a result of a non stable device path found or some other real error which would be messaged in some manner. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 58e7e6d..6def373 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev) } +/* + * Attempt to create a new LUN + * + * Returns: + * + * 0 = Success + * -1 = Failure due to some sort of OOM or other fatal issue found when + *attempting to get/update information about a found volume + * -2 = Failure to find a stable path + */ static int virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, uint32_t host ATTRIBUTE_UNUSED, @@ -202,7 +212,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, VIR_DEBUG(No stable path found for '%s' in '%s', devpath, pool-def-target.path); -retval = -1; +retval = -2; This mixes two different errors: * virStorageBackendStablePath short-circuited based on pool target path that's just as fatal as OOM and the attempt to find other volumes will also fail * virStorageBackendStablePath fell through to ret_strdup - the directory exists, but a symlink to the device did not show up This means the device could appear later, so it makes sense to retry later. And it doesn't handle the opendir failure in virStorageBackendStablePath, which could also mean that the device will appear later. Given that this check here has to negate the !STREQ /dev checks in virStorageBackendStablePath, maybe virStorageBackendStablePath should be split up to two functions - one that returns an error when the device path can't be stabilized and the other that would strdup the original path? True - the real bugger is virStorageBackendStablePath - since it's shared code, my goal here was to minimize impact to the [i]SCSI paths. In any case, I've pushed the 3 ACK'd patches and will see what I can come up with for StablePath checking. Tks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState
These functions operate exactly the same as their network equivalents virNetworkLoadAllState, virNetworkLoadState. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 --- src/conf/storage_conf.c | 94 src/conf/storage_conf.h | 7 src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 13 ++ 4 files changed, 115 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ee564f2..e7d6e6b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1863,6 +1863,100 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, } +virStoragePoolObjPtr +virStoragePoolLoadState(virStoragePoolObjListPtr pools, +const char *stateDir, +const char *name) +{ +char *stateFile = NULL; +virStoragePoolDefPtr def = NULL; +virStoragePoolObjPtr pool = NULL; +xmlDocPtr xml = NULL; +xmlXPathContextPtr ctxt = NULL; +xmlNodePtr node = NULL; + +if (!(stateFile = virFileBuildPath(stateDir, name, .xml))) +goto error; + +if (!(xml = virXMLParseCtxt(stateFile, NULL, _((pool state)), ctxt))) +goto error; + +if (!(node = virXPathNode(//pool, ctxt))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Could not find any 'pool' element in state file)); +goto error; +} + +ctxt-node = node; +if (!(def = virStoragePoolDefParseXML(ctxt))) +goto error; + +if (!STREQ(name, def-name)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Storage pool state file '%s' does not match + pool name '%s'), + stateFile, def-name); +goto error; +} + +/* create the object */ +if (!(pool = virStoragePoolObjAssignDef(pools, def))) +goto error; + +/* XXX: future handling of some additional usefull status data, + * for now, if a status file for a pool exists, the pool will be marked + * as active + */ + +pool-active = 1; + + cleanup: +VIR_FREE(stateFile); +xmlFree(xml); +xmlXPathFreeContext(ctxt); +return pool; + + error: +virStoragePoolDefFree(def); +goto cleanup; +} + + +int +virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, + const char *stateDir) +{ +DIR *dir; +struct dirent *entry; +int ret = -1; + +if (!(dir = opendir(stateDir))) { +if (errno == ENOENT) +return 0; + +virReportSystemError(errno, _(Failed to open dir '%s'), stateDir); +return -1; +} + +while ((ret = virDirRead(dir, entry, stateDir)) 0) { +virStoragePoolObjPtr pool; + +if (entry-d_name[0] == '.') +continue; + +if (!virFileStripSuffix(entry-d_name, .xml)) +continue; + +if (!(pool = virStoragePoolLoadState(pools, stateDir, entry-d_name))) +continue; +virStoragePoolObjUnlock(pool); +} + +closedir(dir); +return ret; +} + + int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, const char *configDir, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 8d43019..7471006 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -318,6 +318,13 @@ int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, const char *configDir, const char *autostartDir); +int virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, + const char *stateDir); + +virStoragePoolObjPtr +virStoragePoolLoadState(virStoragePoolObjListPtr pools, +const char *stateDir, +const char *name); virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 56acb01..6b95dea 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -800,6 +800,7 @@ virStoragePoolFormatFileSystemNetTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolGetVhbaSCSIHostParent; virStoragePoolLoadAllConfigs; +virStoragePoolLoadAllState; virStoragePoolObjAssignDef; virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 0180fd7..36c05b3 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -199,6 +199,17 @@ storageStateInitialize(bool privileged, } driver-privileged = privileged; +if (virFileMakePath(driver-stateDir) 0) { +virReportError(errno, + _(cannot create directory %s), + driver-stateDir); +goto error; +} + +if (virStoragePoolLoadAllState(driver-pools, +
[libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
Properly format storage pool state XML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 --- src/conf/storage_conf.c | 35 +++ src/conf/storage_conf.h | 4 +++- src/libvirt_private.syms | 1 + 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 73b937e..ee564f2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path, return ret; } + + +int virStoragePoolSaveState(const char *stateFile, +virStoragePoolDefPtr def) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; +int ret = -1; +char *xml; + +virBufferAddLit(buf, poolstatus\n); +virBufferAdjustIndent(buf, 2); + +if (virStoragePoolDefFormatBuf(buf, def) 0) +goto error; + +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /poolstatus\n); + +if (virBufferCheckError(buf) 0) +goto error; + +if (!(xml = virBufferContentAndReset(buf))) +goto error; + +if (virStoragePoolSaveXML(stateFile, def, xml)) +goto error; + +ret = 0; + + error: +VIR_FREE(xml); +return ret; +} + + int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 4584075..da378b7 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -371,7 +371,9 @@ virStoragePoolObjPtr virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); -int virStoragePoolSaveConfig(const char *configDir, +int virStoragePoolSaveState(const char *stateFile, +virStoragePoolDefPtr def); +int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def); int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, virStoragePoolObjPtr pool, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e71b1a..56acb01 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -813,6 +813,7 @@ virStoragePoolObjRemove; virStoragePoolObjSaveDef; virStoragePoolObjUnlock; virStoragePoolSaveConfig; +virStoragePoolSaveState; virStoragePoolSourceAdapterTypeFromString; virStoragePoolSourceAdapterTypeToString; virStoragePoolSourceClear; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] storage: Add support for storage pool state XML
This patch introduces new virStorageDriverState element stateDir. Also adds necessary changes to storageStateInitialize, so that directories initialization becomes more generic. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 --- src/conf/storage_conf.h | 1 + src/storage/storage_driver.c | 109 +++ 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index da378b7..8d43019 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -293,6 +293,7 @@ struct _virStorageDriverState { char *configDir; char *autostartDir; +char *stateDir; bool privileged; }; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 64ea770..0180fd7 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -78,6 +78,7 @@ static void storageDriverAutostart(void) { size_t i; +char *stateFile = NULL; virConnectPtr conn = NULL; /* XXX Remove hardcoding of QEMU URI */ @@ -136,7 +137,14 @@ storageDriverAutostart(void) virStoragePoolObjUnlock(pool); continue; } + +if (!(stateFile = virFileBuildPath(driver-stateDir, + pool-def-name, .xml))) +continue; + +ignore_value(virStoragePoolSaveState(stateFile, pool-def)); pool-active = 1; +VIR_FREE(stateFile); } virStoragePoolObjUnlock(pool); } @@ -147,61 +155,67 @@ storageDriverAutostart(void) /** * virStorageStartup: * - * Initialization function for the QEmu daemon + * Initialization function for the Storage Driver */ static int storageStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { -char *base = NULL; +int ret = -1; +char *configdir = NULL; +char *rundir = NULL; if (VIR_ALLOC(driver) 0) -return -1; +return ret; if (virMutexInit(driver-lock) 0) { VIR_FREE(driver); -return -1; +return ret; } storageDriverLock(); if (privileged) { -if (VIR_STRDUP(base, SYSCONFDIR /libvirt) 0) +if (VIR_STRDUP(driver-configDir, + SYSCONFDIR /libvirt/storage) 0 || +VIR_STRDUP(driver-autostartDir, + SYSCONFDIR /libvirt/storage/autostart) 0 || +VIR_STRDUP(driver-stateDir, + LOCALSTATEDIR /run/libvirt/storage) 0) goto error; } else { -base = virGetUserConfigDirectory(); -if (!base) +configdir = virGetUserConfigDirectory(); +rundir = virGetUserRuntimeDirectory(); +if (!(configdir rundir)) +goto error; + +if ((virAsprintf(driver-configDir, +%s/storage, configdir) 0) || +(virAsprintf(driver-autostartDir, +%s/storage, configdir) 0) || +(virAsprintf(driver-stateDir, + %s/storage/run, rundir) 0)) goto error; } driver-privileged = privileged; -/* - * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/... - * (session) or /etc/libvirt/storage/... (system). - */ -if (virAsprintf(driver-configDir, -%s/storage, base) == -1) -goto error; - -if (virAsprintf(driver-autostartDir, -%s/storage/autostart, base) == -1) -goto error; - -VIR_FREE(base); - if (virStoragePoolLoadAllConfigs(driver-pools, driver-configDir, driver-autostartDir) 0) goto error; storageDriverUnlock(); -return 0; + +ret = 0; + cleanup: +VIR_FREE(configdir); +VIR_FREE(rundir); +return ret; error: -VIR_FREE(base); storageDriverUnlock(); storageStateCleanup(); -return -1; +goto cleanup; } /** @@ -261,6 +275,7 @@ storageStateCleanup(void) VIR_FREE(driver-configDir); VIR_FREE(driver-autostartDir); +VIR_FREE(driver-stateDir); storageDriverUnlock(); virMutexDestroy(driver-lock); VIR_FREE(driver); @@ -579,6 +594,7 @@ storagePoolCreateXML(virConnectPtr conn, virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; virStorageBackendPtr backend; +char *stateFile; virCheckFlags(0, NULL); @@ -609,7 +625,12 @@ storagePoolCreateXML(virConnectPtr conn, goto cleanup; } -if (backend-refreshPool(conn, pool) 0) { +if (!(stateFile = virFileBuildPath(driver-stateDir, + pool-def-name, .xml))) +goto cleanup; + +if (virStoragePoolSaveState(stateFile, pool-def) 0 || +
[libvirt] [PATCH 6/6] storage: Introduce storagePoolUpdateAllState function
The 'checkPool' callback was originally part of the storageDriverAutostart function, but the pools need to be checked earlier during initialization phase, otherwise we can't start a domain which mountsa volume after daemon's restarted. This is because qemuProcessReconnect is called earlier than storageDriverAutostart. Therefore the 'checkPool' logic has been moved to storagePoolUpdateAllState which is called inside storageDriverInitialize. We also need a valid 'conn' reference to be able to execute 'refreshPool' during initialization phase. Though it isn't available until storageDriverAutostart all of our storage backends do ignore 'conn' pointer, except for RBD, but RBD doesn't support 'checkPool' callback, so it's safe to pass conn = NULL in this case. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 --- src/storage/storage_driver.c | 74 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 36c05b3..12e94ad 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -75,6 +75,64 @@ static void storageDriverUnlock(void) } static void +storagePoolUpdateAllState(void) +{ +size_t i; +bool active = false; + +for (i = 0; i driver-pools.count; i++) { +virStoragePoolObjPtr pool = driver-pools.objs[i]; +virStorageBackendPtr backend; + +virStoragePoolObjLock(pool); +if (!virStoragePoolObjIsActive(pool)) { +virStoragePoolObjUnlock(pool); +continue; +} + +if ((backend = virStorageBackendForType(pool-def-type)) == NULL) { +VIR_ERROR(_(Missing backend %d), pool-def-type); +virStoragePoolObjUnlock(pool); +continue; +} + +/* Backends which do not support 'checkPool' are considered + * inactive by default. + */ +if (backend-checkPool +backend-checkPool(pool, active) 0) { +virErrorPtr err = virGetLastError(); +VIR_ERROR(_(Failed to initialize storage pool '%s': %s), + pool-def-name, err ? err-message : + _(no error message found)); +virStoragePoolObjUnlock(pool); +continue; +} + +/* We can pass NULL as connection, most backends do not use + * it anyway, but if they do and fail, we want to log error and + * continue with other pools. + */ +if (active) { +virStoragePoolObjClearVols(pool); +if (backend-refreshPool(NULL, pool) 0) { +virErrorPtr err = virGetLastError(); +if (backend-stopPool) +backend-stopPool(NULL, pool); +VIR_ERROR(_(Failed to restart storage pool '%s': %s), + pool-def-name, err ? err-message : + _(no error message found)); +virStoragePoolObjUnlock(pool); +continue; +} +} + +pool-active = active; +virStoragePoolObjUnlock(pool); +} +} + +static void storageDriverAutostart(void) { size_t i; @@ -95,23 +153,11 @@ storageDriverAutostart(void) virStoragePoolObjLock(pool); if ((backend = virStorageBackendForType(pool-def-type)) == NULL) { -VIR_ERROR(_(Missing backend %d), pool-def-type); virStoragePoolObjUnlock(pool); continue; } -if (backend-checkPool -backend-checkPool(pool, started) 0) { -virErrorPtr err = virGetLastError(); -VIR_ERROR(_(Failed to initialize storage pool '%s': %s), - pool-def-name, err ? err-message : - _(no error message found)); -virStoragePoolObjUnlock(pool); -continue; -} - -if (!started -pool-autostart +if (pool-autostart !virStoragePoolObjIsActive(pool)) { if (backend-startPool backend-startPool(conn, pool) 0) { @@ -215,6 +261,8 @@ storageStateInitialize(bool privileged, driver-autostartDir) 0) goto error; +storagePoolUpdateAllState(); + storageDriverUnlock(); ret = 0; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] conf: Introduce virStoragePoolSaveXML
Make XML definition saving more generic by moving the common code into virStoragePoolSaveXML and leave case specific code to PoolSave{Status,Config,...} functions. --- src/conf/storage_conf.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a8e9876..73b937e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1913,11 +1913,25 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, return ret; } + +static int virStoragePoolSaveXML(const char *path, + virStoragePoolDefPtr def, + const char *xml) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; +int ret = -1; + +virUUIDFormat(def-uuid, uuidstr); +ret = virXMLSaveFile(path, + virXMLPickShellSafeComment(def-name, uuidstr), + pool-edit, xml); + +return ret; +} int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def) { -char uuidstr[VIR_UUID_STRING_BUFLEN]; char *xml; int ret = -1; @@ -1927,12 +1941,12 @@ virStoragePoolSaveConfig(const char *configFile, return -1; } -virUUIDFormat(def-uuid, uuidstr); -ret = virXMLSaveFile(configFile, - virXMLPickShellSafeComment(def-name, uuidstr), - pool-edit, xml); -VIR_FREE(xml); +if (virStoragePoolSaveXML(configFile, def, xml)) +goto cleanup; +ret = 0; + cleanup: +VIR_FREE(xml); return ret; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/rpc/virnetsocket.c | 46 ++ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..8d8b6e0 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -545,8 +545,10 @@ int virNetSocketNewConnectUNIX(const char *path, { char *binname = NULL; char *pidpath = NULL; +char *lockpath = NULL; int fd, passfd = -1; int pidfd = -1; +int lockfd = -1; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +563,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } +if (spawnDaemon) { +if (virPidFileConstructPath(false, NULL, libvirt-lock, lockpath) 0) +goto error; + +if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) 0 || +virSetCloseExec(lockfd) 0) { +virReportSystemError(errno, _(Unable to create lock '%s'), lockpath); +goto error; +} + +if (virFileLock(lockfd, false, 0, 1, true) 0) { +virReportSystemError(errno, _(Unable to lock '%s'), lockpath); +goto error; +} +} + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); goto error; @@ -595,17 +613,8 @@ int virNetSocketNewConnectUNIX(const char *path, if (virPidFileConstructPath(false, NULL, binname, pidpath) 0) goto error; -pidfd = virPidFileAcquirePath(pidpath, false, getpid()); -if (pidfd 0) { -/* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ -virResetLastError(); -spawnDaemon = false; -goto retry; -} +if ((pidfd = virPidFileAcquirePath(pidpath, false, getpid())) 0) +goto error; if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); @@ -668,16 +677,17 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; } -/* - * Do we need to eliminate the super-rare race here any more? It would - * need incorporating the following VIR_FORCE_CLOSE() into a - * virCommandHook inside a virNetSocketForkDaemon(). - */ VIR_FORCE_CLOSE(pidfd); if (virNetSocketForkDaemon(binary, passfd) 0) goto error; } +if (lockfd) { +unlink(lockpath); +VIR_FORCE_CLOSE(lockfd); +VIR_FREE(lockpath); +} + localAddr.len = sizeof(localAddr.data); if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) { virReportSystemError(errno, %s, _(Unable to get local socket name)); @@ -694,10 +704,14 @@ int virNetSocketNewConnectUNIX(const char *path, error: if (pidfd = 0) virPidFileDeletePath(pidpath); +if (lockfd) +unlink(lockpath); VIR_FREE(pidpath); +VIR_FREE(lockpath); VIR_FORCE_CLOSE(fd); VIR_FORCE_CLOSE(passfd); VIR_FORCE_CLOSE(pidfd); +VIR_FORCE_CLOSE(lockfd); if (spawnDaemon) unlink(path); return -1; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/6] scsi: Remove unused 'type_path' in processLU
On Wed, Apr 01, 2015 at 01:29:09PM -0400, John Ferlan wrote: Seems to be a remnant that was never cleaned up from original submit... Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 3 --- 1 file changed, 3 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 00/10] Add support for memory hotplug
Hi Peter, While playing around with memory hotplug implementation, I found that the guest XML isnt updated after a successful hotplug operation : [root@kop2 test-libvirt]# ./bin/virsh attach-device rhel71-be /home/prerna/mem-hp.xml Device attached successfully [root@kop2 test-libvirt]# ./bin/virsh qemu-monitor-command rhel71-be --hmp info memdev memory backend: 0 size: 1073741824 merge: true dump: true prealloc: false policy: default host nodes: [root@kop2 test-libvirt]# ./bin/virsh edit rhel71-be Does not reflect an updated memory value or a separate dimm device I have observed this on PowerKVM. Wondering what is a good place to observe the hot-added memory : Should this reflect as an updated value for current memory in domain XML, or should this show up as a memory device of type: memory model='dimm' source pagesize unit='KiB'4096/pagesize nodemask1-3/nodemask /source target size unit='KiB'524287/size node1/node /target /memory I would be happy to work on a patch to fix this. Pls provide me your thoughts. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] hostdev: fix net config restore error
On Wed, Apr 01, 2015 at 01:35:36PM -0400, Laine Stump wrote: On 04/01/2015 12:15 PM, Huanle Han wrote: On 2015年03月31日 17:46, Martin Kletzander wrote: On Thu, Mar 26, 2015 at 10:23:47PM +0800, Huanle Han wrote: Fix for such a case: 1. Domain A and B xml contain the same SRIOV net hostdev(interface type='hostdev' / with same pci address). 2. virsh start A (Successfully, and configure the SRIOV net with custom mac) 3. virsh start B (Fail because of the hostdev used by domain A or other reason.) In step 3, 'virHostdevNetConfigRestore' is called for the hostdev which is still used by domain A. It makes the mac/vlan of the SRIOV net change. Makes perfect sense, thanks for finding that out. Someone else encountered this last month too, and posted a patch: https://www.redhat.com/archives/libvir-list/2015-February/msg00394.html The patch was NACKed because I didn't think the fix was correct (although it did work) https://www.redhat.com/archives/libvir-list/2015-February/msg00976.html I don't have the time to do a close analysis of this patch right now, but from the comments it sounds like it addresses the concern that I had with the previous patch (that it wouldn't be messing with any devices that were currently not in use by any domain, and hadn't yet been reached in the setup part). I wanted to point that out now so that whoever looks at the next version of this patch checks for that. I must have missed that. It looks like this series does not have the issue of the one you've mentioned. Code Change in this fix: 1. As the pci used by other domain have been removed from 'pcidevs' in previous loop, we only restore the nic config for the hostdev still in 'pcidevs'(used by this domain) 2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the hostdev is a pci net device or not. 3. update the comments to make it more clear Signed-off-by: Huanle Han hanxue...@gmail.com mailto:hanxue...@gmail.com --- src/util/virhostdev.c | 52 --- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f567d..b04bae2 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, return ret; } +static int +virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) +{ +return hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS +hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI +hostdev-parent.type == VIR_DOMAIN_DEVICE_NET +hostdev-parent.data.net http://parent.data.net; +} static int virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, @@ -481,10 +489,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, /* This is only needed for PCI devices that have been defined * using interface type='hostdev'. For all others, it is a NOP. */ -if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || -hostdev-parent.type != VIR_DOMAIN_DEVICE_NET || -!hostdev-parent.data.net http://parent.data.net) +if (!virHostdevIsPCINetDevice(hostdev)) return 0; isvf = virHostdevIsVirtualFunction(hostdev); @@ -781,19 +786,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, goto cleanup; } -/* Again 4 loops; mark all devices as inactive before reset +/* Here are 4 loops; mark all devices as inactive before reset * them and reset all the devices before re-attach. * Attach mac and port profile parameters to devices */ + +/* Loop 1: delete the copy of the dev from pcidevs if it's used by + * other domain. Or delete it from activePCIHostDevs if it had + * been used by this domain. + */ i = 0; while (i virPCIDeviceListCount(pcidevs)) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); virPCIDevicePtr activeDev = NULL; -/* delete the copy of the dev from pcidevs if it's used by - * other domain. Or delete it from activePCIHostDevs if it had - * been used by this domain. - */ activeDev = virPCIDeviceListFind(hostdev_mgr-activePCIHostdevs, dev); if (activeDev) { const char *usedby_drvname; @@ -814,14 +820,27 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, * pcidevs, but has been removed from activePCIHostdevs. */ -/* - * For SRIOV net host devices, unset mac and port profile before - * reset and reattach device +/* Loop 2: before reset and reattach device, + * unset mac and port profile for SRIOV net host devices used by this domain */ The comments were formatted as a sentence, it would be nice to keep it that way. Is comment as below OK? /* * Loop 2: For SRIOV net host devices used by this domain, unset mac * and port
Re: [libvirt] [PATCH 1/7] storage: Remove unused attribute conn from 'checkPool' callback
On 03/26/2015 02:51 PM, Peter Krempa wrote: On Wed, Mar 25, 2015 at 12:19:18 -0400, John Ferlan wrote: On 03/24/2015 06:06 AM, Erik Skultety wrote: The checkPool callback should be used when updating states of all pools during storageStateInitialize, not storageDriverAutostart, otherwise we can't start a domain which mounts a volume after daemons restarted. This is because qemuProcessReconnect is called earlier than storageDriverAutostart which marks the pool as active, so that the domain can mount a volume from this pool successfully. In order to fix this, conn attribute has to be discarded from the callback, because storageStateInitialize doesn't have a connection yet. (it's not used anyway) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 This patch definitely does not resolve this^^. I'm not convinced we can remove 'conn'. Something about backend or backward compatibility. Since it's not used by any backend anyway, there's (more or less) no harm in keeping it. The storage driver backend APIs are internal so it's okay to change Additionally I recall that it will simplify some of the further patches. ACK Peter Thanks, I removed the BZ link and reworded the message a bit and pushed, v2 of the remaining patches coming in a second. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Add support for storage pool state XML
Patches add necessary changes to add support for storage pool state XMLs and thus resolve https://bugzilla.redhat.com/show_bug.cgi?id=1177733 Erik Skultety (6): conf: Introduce virStoragePoolDefFormatBuf conf: Introduce virStoragePoolSaveXML conf: Introduce virStoragePoolSaveState storage: Add support for storage pool state XML conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState storage: Introduce storagePoolUpdateAllState function src/conf/storage_conf.c | 233 --- src/conf/storage_conf.h | 12 ++- src/libvirt_private.syms | 2 + src/storage/storage_driver.c | 188 +++--- 4 files changed, 358 insertions(+), 77 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] conf: Introduce virStoragePoolDefFormatBuf
When modifying config/status XML, it might be handy to include some additional XML elements (e.g. poolstatus). In order to do so, introduce new formatting function virStoragePoolDefFormatBuf and make virStoragePoolDefFormat call it. --- src/conf/storage_conf.c | 78 + 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index b070448..a8e9876 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf, } -char * -virStoragePoolDefFormat(virStoragePoolDefPtr def) +static int +virStoragePoolDefFormatBuf(virBufferPtr buf, + virStoragePoolDefPtr def) { virStoragePoolOptionsPtr options; -virBuffer buf = VIR_BUFFER_INITIALIZER; -const char *type; char uuid[VIR_UUID_STRING_BUFLEN]; +const char *type; options = virStoragePoolOptionsForPoolType(def-type); if (options == NULL) -return NULL; +goto error; type = virStoragePoolTypeToString(def-type); if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unexpected pool type)); -goto cleanup; +goto error; } -virBufferAsprintf(buf, pool type='%s'\n, type); -virBufferAdjustIndent(buf, 2); -virBufferEscapeString(buf, name%s/name\n, def-name); +virBufferAsprintf(buf, pool type='%s'\n, type); +virBufferAdjustIndent(buf, 2); +virBufferEscapeString(buf, name%s/name\n, def-name); virUUIDFormat(def-uuid, uuid); -virBufferAsprintf(buf, uuid%s/uuid\n, uuid); +virBufferAsprintf(buf, uuid%s/uuid\n, uuid); -virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n, +virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n, def-capacity); -virBufferAsprintf(buf, allocation unit='bytes'%llu/allocation\n, +virBufferAsprintf(buf, allocation unit='bytes'%llu/allocation\n, def-allocation); -virBufferAsprintf(buf, available unit='bytes'%llu/available\n, +virBufferAsprintf(buf, available unit='bytes'%llu/available\n, def-available); -if (virStoragePoolSourceFormat(buf, options, def-source) 0) -goto cleanup; +if (virStoragePoolSourceFormat(buf, options, def-source) 0) +goto error; /* RBD, Sheepdog, and Gluster devices are not local block devs nor * files, so they don't have a target */ if (def-type != VIR_STORAGE_POOL_RBD def-type != VIR_STORAGE_POOL_SHEEPDOG def-type != VIR_STORAGE_POOL_GLUSTER) { -virBufferAddLit(buf, target\n); -virBufferAdjustIndent(buf, 2); +virBufferAddLit(buf, target\n); +virBufferAdjustIndent(buf, 2); -virBufferEscapeString(buf, path%s/path\n, def-target.path); +virBufferEscapeString(buf, path%s/path\n, def-target.path); -virBufferAddLit(buf, permissions\n); -virBufferAdjustIndent(buf, 2); -virBufferAsprintf(buf, mode0%o/mode\n, +virBufferAddLit(buf, permissions\n); +virBufferAdjustIndent(buf, 2); +virBufferAsprintf(buf, mode0%o/mode\n, def-target.perms.mode); -virBufferAsprintf(buf, owner%d/owner\n, +virBufferAsprintf(buf, owner%d/owner\n, (int) def-target.perms.uid); -virBufferAsprintf(buf, group%d/group\n, +virBufferAsprintf(buf, group%d/group\n, (int) def-target.perms.gid); -virBufferEscapeString(buf, label%s/label\n, +virBufferEscapeString(buf, label%s/label\n, def-target.perms.label); -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, /permissions\n); -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, /target\n); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /permissions\n); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /target\n); } -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, /pool\n); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /pool\n); + +return 0; + + error: +return -1; +} + +char * +virStoragePoolDefFormat(virStoragePoolDefPtr def) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +if (virStoragePoolDefFormatBuf(buf, def) 0) +goto error; if (virBufferCheckError(buf) 0) -goto cleanup; +goto error; return virBufferContentAndReset(buf); - cleanup: + error: virBufferFreeAndReset(buf); return NULL; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/6] iscsi: Use error message from virStorageBackendSCSIFindLUs
On Wed, Apr 01, 2015 at 01:29:06PM -0400, John Ferlan wrote: Don't supercede the error message virStorageBackendSCSIFindLUs as the message such as error: Failed to find LUs on host 60: ... is not overly clear as to what the real problem might be. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_iscsi.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/6] iscsi: Fix exit path for virStorageBackendISCSIFindLUs failure
On Wed, Apr 01, 2015 at 01:29:07PM -0400, John Ferlan wrote: If the call to virStorageBackendISCSIGetHostNumber failed, we set retval = -1, but yet still called virStorageBackendSCSIFindLUs. Need to add a goto cleanup - while at it, adjust the logic to initialize retval to -1 and only changed to 0 (zero) on success. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_iscsi.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Add support for storage pool state XML
On 04/02/2015 12:10 PM, Erik Skultety wrote: Patches add necessary changes to add support for storage pool state XMLs and thus resolve https://bugzilla.redhat.com/show_bug.cgi?id=1177733 Erik Skultety (6): conf: Introduce virStoragePoolDefFormatBuf conf: Introduce virStoragePoolSaveXML conf: Introduce virStoragePoolSaveState storage: Add support for storage pool state XML conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState storage: Introduce storagePoolUpdateAllState function src/conf/storage_conf.c | 233 --- src/conf/storage_conf.h | 12 ++- src/libvirt_private.syms | 2 + src/storage/storage_driver.c | 188 +++--- 4 files changed, 358 insertions(+), 77 deletions(-) These are v2 patches, forgot the --subject-prefix option with format-patch. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] scsi: Adjust return value for virStorageBackendSCSINewLun
On Wed, Apr 01, 2015 at 01:29:08PM -0400, John Ferlan wrote: Add a return -2 to differentiate that the failure was a result of a non stable device path found or some other real error which would be messaged in some manner. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 58e7e6d..6def373 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev) } +/* + * Attempt to create a new LUN + * + * Returns: + * + * 0 = Success + * -1 = Failure due to some sort of OOM or other fatal issue found when + *attempting to get/update information about a found volume + * -2 = Failure to find a stable path + */ static int virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, uint32_t host ATTRIBUTE_UNUSED, @@ -202,7 +212,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, VIR_DEBUG(No stable path found for '%s' in '%s', devpath, pool-def-target.path); -retval = -1; +retval = -2; This mixes two different errors: * virStorageBackendStablePath short-circuited based on pool target path that's just as fatal as OOM and the attempt to find other volumes will also fail * virStorageBackendStablePath fell through to ret_strdup - the directory exists, but a symlink to the device did not show up This means the device could appear later, so it makes sense to retry later. And it doesn't handle the opendir failure in virStorageBackendStablePath, which could also mean that the device will appear later. Given that this check here has to negate the !STREQ /dev checks in virStorageBackendStablePath, maybe virStorageBackendStablePath should be split up to two functions - one that returns an error when the device path can't be stabilized and the other that would strdup the original path? Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/6] Allocate virtio-serial addresses
On 03/24/2015 01:15 PM, Ján Tomko wrote: Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1076708 https://bugzilla.redhat.com/show_bug.cgi?id=890606 New in v3: * preserve the behavior of honoring the controller if a partial address is specified: address type='virtio-serial' controller='2'/ * automatically add a controller if we're out of free ports and add a test for it Ján Tomko (6): Add test for virtio serial port assignment Add functions to track virtio-serial addresses Allocate virtio-serial addresses when starting a domain Expand the address set when attaching a virtio-serial controller Assign an address when hotplugging a virtio-serial device Auto add virtio-serial controllers src/conf/domain_addr.c | 435 + src/conf/domain_addr.h | 61 +++ src/conf/domain_conf.c | 48 +-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 11 + src/qemu/qemu_command.c| 63 +++ src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c| 32 +- src/qemu/qemu_process.c| 2 + tests/qemuhotplugtest.c| 2 +- .../qemuxml2argv-channel-virtio-auto.args | 2 +- .../qemuxml2argv-channel-virtio-autoadd.args | 33 ++ .../qemuxml2argv-channel-virtio-autoadd.xml| 45 +++ .../qemuxml2argv-channel-virtio-autoassign.args| 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 +++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +- 18 files changed, 777 insertions(+), 43 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml ACK series John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] conf: Introduce virStoragePoolSaveXML
ACK if you add at least one new line between virStoragePoolSaveConfig and virStoragePoolSaveXML Peter Added empty lines and pushed, thank you Peter. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] storage: Introduce storagePoolUpdateAllState function
On 04/02/2015 06:10 AM, Erik Skultety wrote: The 'checkPool' callback was originally part of the storageDriverAutostart function, but the pools need to be checked earlier during initialization phase, otherwise we can't start a domain which mountsa volume after daemon's s/mountsa/mounts a/ s/after daemon's/after the libvirtd daemon/ restarted. This is because qemuProcessReconnect is called earlier than storageDriverAutostart. Therefore the 'checkPool' logic has been moved to storagePoolUpdateAllState which is called inside storageDriverInitialize. We also need a valid 'conn' reference to be able to execute 'refreshPool' during initialization phase. Though it isn't available until storageDriverAutostart all of our storage backends do ignore 'conn' pointer, except for RBD, but RBD doesn't support 'checkPool' callback, so it's safe to pass conn = NULL in this case. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 yes, this does belong in this one Also, as I'm going through my list of backlog bugzillas, I think that https://bugzilla.redhat.com/show_bug.cgi?id=1125805 can be duplicated to this bz. --- src/storage/storage_driver.c | 74 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 36c05b3..12e94ad 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -75,6 +75,64 @@ static void storageDriverUnlock(void) } static void +storagePoolUpdateAllState(void) +{ +size_t i; +bool active = false; Instead of initializing it here - do it in the code... + +for (i = 0; i driver-pools.count; i++) { +virStoragePoolObjPtr pool = driver-pools.objs[i]; +virStorageBackendPtr backend; + +virStoragePoolObjLock(pool); +if (!virStoragePoolObjIsActive(pool)) { +virStoragePoolObjUnlock(pool); +continue; +} + +if ((backend = virStorageBackendForType(pool-def-type)) == NULL) { +VIR_ERROR(_(Missing backend %d), pool-def-type); +virStoragePoolObjUnlock(pool); +continue; +} + +/* Backends which do not support 'checkPool' are considered + * inactive by default. + */ active = false; So here's my reasoning for resetting active each time through the pool... What happens if pool [1] has a check and is determined to be active... then pool [2] doesn't have a check function, but active is still true from [1] - thus the refreshPool will happen. ACK with that adjustment. John +if (backend-checkPool +backend-checkPool(pool, active) 0) { +virErrorPtr err = virGetLastError(); +VIR_ERROR(_(Failed to initialize storage pool '%s': %s), + pool-def-name, err ? err-message : + _(no error message found)); +virStoragePoolObjUnlock(pool); +continue; +} + +/* We can pass NULL as connection, most backends do not use + * it anyway, but if they do and fail, we want to log error and + * continue with other pools. + */ +if (active) { +virStoragePoolObjClearVols(pool); +if (backend-refreshPool(NULL, pool) 0) { +virErrorPtr err = virGetLastError(); +if (backend-stopPool) +backend-stopPool(NULL, pool); +VIR_ERROR(_(Failed to restart storage pool '%s': %s), + pool-def-name, err ? err-message : + _(no error message found)); +virStoragePoolObjUnlock(pool); +continue; +} +} + +pool-active = active; +virStoragePoolObjUnlock(pool); +} +} + +static void storageDriverAutostart(void) { size_t i; @@ -95,23 +153,11 @@ storageDriverAutostart(void) virStoragePoolObjLock(pool); if ((backend = virStorageBackendForType(pool-def-type)) == NULL) { -VIR_ERROR(_(Missing backend %d), pool-def-type); virStoragePoolObjUnlock(pool); continue; } -if (backend-checkPool -backend-checkPool(pool, started) 0) { -virErrorPtr err = virGetLastError(); -VIR_ERROR(_(Failed to initialize storage pool '%s': %s), - pool-def-name, err ? err-message : - _(no error message found)); -virStoragePoolObjUnlock(pool); -continue; -} - -if (!started -pool-autostart +if (pool-autostart !virStoragePoolObjIsActive(pool)) { if (backend-startPool backend-startPool(conn, pool) 0) { @@ -215,6 +261,8 @@ storageStateInitialize(bool
Re: [libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
On 02.04.2015 16:33, Martin Kletzander wrote: On Thu, Apr 02, 2015 at 03:07:11PM +0200, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/rpc/virnetsocket.c | 46 ++ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..8d8b6e0 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -545,8 +545,10 @@ int virNetSocketNewConnectUNIX(const char *path, { char *binname = NULL; char *pidpath = NULL; +char *lockpath = NULL; int fd, passfd = -1; int pidfd = -1; +int lockfd = -1; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +563,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } +if (spawnDaemon) { +if (virPidFileConstructPath(false, NULL, libvirt-lock, lockpath) 0) +goto error; + +if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) 0 || +virSetCloseExec(lockfd) 0) { +virReportSystemError(errno, _(Unable to create lock '%s'), lockpath); +goto error; +} + +if (virFileLock(lockfd, false, 0, 1, true) 0) { +virReportSystemError(errno, _(Unable to lock '%s'), lockpath); +goto error; +} +} + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); goto error; @@ -595,17 +613,8 @@ int virNetSocketNewConnectUNIX(const char *path, if (virPidFileConstructPath(false, NULL, binname, pidpath) 0) goto error; -pidfd = virPidFileAcquirePath(pidpath, false, getpid()); -if (pidfd 0) { -/* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ -virResetLastError(); -spawnDaemon = false; -goto retry; -} +if ((pidfd = virPidFileAcquirePath(pidpath, false, getpid())) 0) +goto error; What if someone starts the daemon by hand in the meantime? There should be 'goto retry;', I guess. Okay. if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); @@ -668,16 +677,17 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; } -/* - * Do we need to eliminate the super-rare race here any more? It would - * need incorporating the following VIR_FORCE_CLOSE() into a - * virCommandHook inside a virNetSocketForkDaemon(). - */ VIR_FORCE_CLOSE(pidfd); if (virNetSocketForkDaemon(binary, passfd) 0) goto error; There's still a problem here. If someone starts the daemon between the VIR_FORCE_CLOSE() and us starting the daemon, virNetSocketForkDaemon() won't fail as the process will start, but it will die after a while, realizing that there is another daemon running. And this thread will continue with socket connected nowhere; the socket that was passed to the daemon being started was unlinked by the daemon that was started manually. I know this is very, very rare case, but I thought the same about starting multiple virshes (what the heck is the plural of 'virsh' anyway) and look where that got us! I'm not against this approach, it's definitely an improvement for the current situation, but it's not a complete solution. What are your thoughts on that? I think the whole daemon spanning approach is fubar-ed. But let me see if we can rely here purely on the daemon acquiring the pidfile itself. It's strange that we allocate the PID file anyway. However, it's not
Re: [libvirt] [PATCH V3] libxl: fix dom0 balloon logic
Martin Kletzander wrote: On Wed, Apr 01, 2015 at 11:08:54AM -0600, Jim Fehlig wrote: Recent testing on large memory systems revealed a bug in the Xen xl tool's freemem() function. When autoballooning is enabled, freemem() is used to ensure enough memory is available to start a domain, ballooning dom0 if necessary. When ballooning large amounts of memory from dom0, freemem() would exceed its self-imposed wait time and return an error. Meanwhile, dom0 continued to balloon. Starting the domain later, after sufficient memory was ballooned from dom0, would succeed. The libvirt implementation in libxlDomainFreeMem() suffers the same bug since it is modeled after freemem(). In the end, the best place to fix the bug on the Xen side was to slightly change the behavior of libxl_wait_for_memory_target(). Instead of failing after caller-provided wait_sec, the function now blocks as long as dom0 memory ballooning is progressing. It will return failure only when more memory is needed to reach the target and wait_sec have expired with no progress being made. See xen.git commit fd3aa246. There was a dicussion on how this would affect other libxl apps like libvirt http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html If libvirt containing this patch was build against a Xen containing the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() will fail after 30 sec and domain creation will be terminated. Without this patch and with old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() does not succeed after 30 sec, but returns success anyway. Domain creation continues resulting in all sorts of fun stuff like cpu soft lockups in the guest OS. It was decided to properly fix libxl_wait_for_memory_target(), and if anything improve the default behavior of apps using the freemem reference impl in xl. xl was patched to accommodate the change in libxl_wait_for_memory_target() with xen.git commit 883b30a0. This patch does the same in the libxl driver. While at it, I changed the logic to essentially match freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner IMO and will make it easier to spot future, potentially interesting divergences. Signed-off-by: Jim Fehlig jfeh...@suse.com --- V3: Remove unneeded local variable 'ret' in libxlDomainFreeMem. ACK Thanks! I've pushed this now. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix xlconfigtest with older libxl
Commit cd5dc30 added this test, but it fails if LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is not defined: 6) Xen XM-2-XML Format fullvirt-multiusb ... libvirt: error : unsupported configuration: multiple USB devices not supported FAILED --- tests/xlconfigtest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 6d4aa6d..c992548 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -215,7 +215,10 @@ mymain(void) DO_TEST(new-disk, 3); DO_TEST(spice, 3); + +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST DO_TEST(fullvirt-multiusb, 3); +#endif virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
On 04/02/2015 06:10 AM, Erik Skultety wrote: Properly format storage pool state XML. Your previous patch 3/7 had a better commit message: Introduce virStoragePoolSaveStatus to properly format the status XML in the same manner as virStoragePoolDefFormat, except for adding a poolstatus ... /poolstatus around the definition. This is similar to virNetworkObjFormat used to save the live/active network information. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 By itself - no it doesn't solve the bug (same for 4 and 5) --- src/conf/storage_conf.c | 35 +++ src/conf/storage_conf.h | 4 +++- src/libvirt_private.syms | 1 + 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 73b937e..ee564f2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path, return ret; } + + +int virStoragePoolSaveState(const char *stateFile, +virStoragePoolDefPtr def) Again it's int virStorage... ACK with those adjustments. John FYI: Coverity is happy with all 6 patches... +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; +int ret = -1; +char *xml; + +virBufferAddLit(buf, poolstatus\n); +virBufferAdjustIndent(buf, 2); + +if (virStoragePoolDefFormatBuf(buf, def) 0) +goto error; + +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /poolstatus\n); + +if (virBufferCheckError(buf) 0) +goto error; + +if (!(xml = virBufferContentAndReset(buf))) +goto error; + +if (virStoragePoolSaveXML(stateFile, def, xml)) +goto error; + +ret = 0; + + error: +VIR_FREE(xml); +return ret; +} + + int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 4584075..da378b7 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -371,7 +371,9 @@ virStoragePoolObjPtr virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); -int virStoragePoolSaveConfig(const char *configDir, +int virStoragePoolSaveState(const char *stateFile, +virStoragePoolDefPtr def); +int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def); int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, virStoragePoolObjPtr pool, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e71b1a..56acb01 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -813,6 +813,7 @@ virStoragePoolObjRemove; virStoragePoolObjSaveDef; virStoragePoolObjUnlock; virStoragePoolSaveConfig; +virStoragePoolSaveState; virStoragePoolSourceAdapterTypeFromString; virStoragePoolSourceAdapterTypeToString; virStoragePoolSourceClear; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] hostdev: Fix index error in loop after remove an element
On Tue, Mar 31, 2015 at 10:34:57AM +0200, Martin Kletzander wrote: On Thu, Mar 26, 2015 at 10:23:46PM +0800, Huanle Han wrote: 'virPCIDeviceList' is actually an array. Removing one element makes the rest of the element move. Use while loop, increase index only when not virPCIDeviceListDel(pcidevs, dev) Signed-off-by: Huanle Han hanxue...@gmail.com --- src/util/virhostdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 23365a3..83f567d 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -785,7 +785,8 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, * them and reset all the devices before re-attach. * Attach mac and port profile parameters to devices */ -for (i = 0; i virPCIDeviceListCount(pcidevs); i++) { +i = 0; +while (i virPCIDeviceListCount(pcidevs)) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); virPCIDevicePtr activeDev = NULL; @@ -806,6 +807,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, } virPCIDeviceListDel(hostdev_mgr-activePCIHostdevs, dev); +i++; 'i--;' before that 'continue;' that's not in the index would be shorter, but I guess this is more readable. Or maybe: for (i = virPCIDeviceListCount(pcidevs); i 0; i--) { size_t dev_index = i - 1; would just deal with this. Anyway, this works just as it is, ACK if you don't want to change it. Pushed now as it's after release. pgppVr9WMj8xe.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] hostdev: fix loop index error when resetvfnetconfig
The variable 'last_processed_hostdev_vf' indicates index of the last successfully configed vf. When resetvfnetconfig because of failure, hostdevs[last_processed_hostdev_vf] should also be reset. Signed-off-by: Huanle Han hanxue...@gmail.com --- src/util/virhostdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f567d..b15db70 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -693,7 +693,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr, resetvfnetconfig: for (i = 0; - last_processed_hostdev_vf != -1 i last_processed_hostdev_vf; i++) + last_processed_hostdev_vf != -1 i = last_processed_hostdev_vf; i++) virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr-stateDir, NULL); reattachdevs: -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] hostdev: fix net config restore error
2015-04-02 1:35 GMT+08:00 Laine Stump la...@laine.org: On 04/01/2015 12:15 PM, Huanle Han wrote: On 2015年03月31日 17:46, Martin Kletzander wrote: On Thu, Mar 26, 2015 at 10:23:47PM +0800, Huanle Han wrote: Fix for such a case: 1. Domain A and B xml contain the same SRIOV net hostdev(interface type='hostdev' / with same pci address). 2. virsh start A (Successfully, and configure the SRIOV net with custom mac) 3. virsh start B (Fail because of the hostdev used by domain A or other reason.) In step 3, 'virHostdevNetConfigRestore' is called for the hostdev which is still used by domain A. It makes the mac/vlan of the SRIOV net change. Makes perfect sense, thanks for finding that out. Someone else encountered this last month too, and posted a patch: https://www.redhat.com/archives/libvir-list/2015-February/msg00394.html The patch was NACKed because I didn't think the fix was correct (although it did work) https://www.redhat.com/archives/libvir-list/2015-February/msg00976.html I don't have the time to do a close analysis of this patch right now, but from the comments it sounds like it addresses the concern that I had with the previous patch (that it wouldn't be messing with any devices that were currently not in use by any domain, and hadn't yet been reached in the setup part). I wanted to point that out now so that whoever looks at the next version of this patch checks for that. active virPCIDevice has member variable 'used_by_drvname' and 'used_by_domname' to mark its user. According to ’virHostdevPreparePCIDevices‘, only the pci is successfully prepared, will the member 'used_by_XXX' be assigned. The domain PCI state when reattach: 1. If qemuProcessStart() fails before pci setup(virHostdevPreparePCIDevices()), the pci will either be not active, or be active but used by others. 2. if qemuProcessStart() fails during pci setup , virHostdevPreparePCIDevices rollbacks the change. The result will be the same with 1. 3. if qemuProcessStart() fails after pci setup, the pci will be active and used by this domain. In fucntion virHostdevReAttachPCIDevices, 1. pcidevs is from virHostdevGetActivePCIHostDeviceList(). That is say, pci in pcidevs is active ( used/prepared by any domains). 2. In loop 1, the pci used by other domains is removed from the pcidevs. After loop, all pcis in pcidevs are used/prepared by this domain. 3. The following reattach operation is on these 'pcidevs', not the pci used by nobody or other domains. So I don't think the problem you said exists. This solution is OK. Am I right? Code Change in this fix: 1. As the pci used by other domain have been removed from 'pcidevs' in previous loop, we only restore the nic config for the hostdev still in 'pcidevs'(used by this domain) 2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the hostdev is a pci net device or not. 3. update the comments to make it more clear Signed-off-by: Huanle Han hanxue...@gmail.com hanxue...@gmail.com --- src/util/virhostdev.c | 52 --- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f567d..b04bae2 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, return ret; } +static int +virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) +{ +return hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS +hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI +hostdev-parent.type == VIR_DOMAIN_DEVICE_NET +hostdev-parent.data.net; +} static int virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, @@ -481,10 +489,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, /* This is only needed for PCI devices that have been defined * using interface type='hostdev'. For all others, it is a NOP. */ -if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || -hostdev-parent.type != VIR_DOMAIN_DEVICE_NET || -!hostdev-parent.data.net) +if (!virHostdevIsPCINetDevice(hostdev)) return 0; isvf = virHostdevIsVirtualFunction(hostdev); @@ -781,19 +786,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, goto cleanup; } -/* Again 4 loops; mark all devices as inactive before reset +/* Here are 4 loops; mark all devices as inactive before reset * them and reset all the devices before re-attach. * Attach mac and port profile parameters to devices */ + +/* Loop 1: delete the copy of the dev from pcidevs if it's used by + * other domain. Or delete it from activePCIHostDevs if it had + * been used by this domain. + */ i = 0; while (i virPCIDeviceListCount(pcidevs))
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On Thu, Apr 02, 2015 at 09:09:07AM +0200, Michael Mueller wrote: On Wed, 1 Apr 2015 20:05:24 -0300 Eduardo Habkost ehabk...@redhat.com wrote: If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a stable-abi-safe field on CpuDefinitionInfo? Exactly that fulfills the name field for s390 already in my view. And cpu model none just means that QEMU does not manage the cpu model. That's also the reason why I initially returned an empty [] model and not none. This somewhat convinces me to go back to this approach... I understand the reasons for your approach and it seems to work for s390, but the only problem I see is that you are adding an additional (undocumented?) s390-specific constraint to the semantics of query-cpu-models: that the model name will appear on the list only if it can be safely migratable. This may prevent us from unifying CPU model code into generic code later. I agree that an aliases is something different compared with the CPU model none as there is a CPU class representing it. And thus, when implicitly or explicitly selected, shall be presented in the CPU definition list as well. If I would set runnable to false as it now (bad), it would be sorted out by the considered for migration test but it would be misleading as it is always runnable. Though an additional field like migrate-able could express that characteristic. Exactly. But if we add a simple stable-abi-safe field to the list (even if s390 set it to to true for all models and omit aliases and none in this first version), we will have clearer semantics that can still be honoured by other architectures (and by generic code) later. To be honest I currently don't right get the idea that you follow with that stable-abi-save field... But eventually yes (I wrote this before the section above) The stable-abi-save field means: Take me into account for whatever kind of CPU model related comparison you perform between two running QEMU instances as I represent a well defined aspect. Yes. stable-abi-safe would mean that nothing guest-visible will change in the CPU model when running a different QEMU version or running in a different host, thus making it safe to live-migrate (as long as you keep the same machine+accelerator and don't change other guest-visible configuration in the QEMU command-line, of course). That's a constraint we already keep in the x86 CPU models, except for -cpu host. In other words, it means as long as the name matches the query-cpus output from the source host, it is guaranteed to be safe to live-migrate. Which is the constraint you need, right? (I am not 100% sure about the naming. Maybe we should call it live-migration-safe?) Thus CPU model none will be { name: none, runnable: true, stable-abi-save: false } and the aliases can be represented as { name: alias, runnable: true|false, stable-abi-save: false } in the s390 case, right? Exactly. We don't need to return them in the first version if you don't need to (althought I don't see a reason to not return them). It will just allow us to return them in the future. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix xlconfigtest with older libxl
Ján Tomko wrote: Commit cd5dc30 added this test, but it fails if LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is not defined: Opps, sorry. I tested back to Xen 4.3 where I thought this was undefined. But looking at that machine now, I see LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined. I guess your libxl is from Xen 4.2? 6) Xen XM-2-XML Format fullvirt-multiusb ... libvirt: error : unsupported configuration: multiple USB devices not supported FAILED --- tests/xlconfigtest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 6d4aa6d..c992548 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -215,7 +215,10 @@ mymain(void) DO_TEST(new-disk, 3); DO_TEST(spice, 3); + +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST DO_TEST(fullvirt-multiusb, 3); +#endif virObjectUnref(caps); virObjectUnref(xmlopt); ACK. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] storage: Add support for storage pool state XML
On 04/02/2015 06:10 AM, Erik Skultety wrote: This patch introduces new virStorageDriverState element stateDir. Also adds necessary changes to storageStateInitialize, so that directories initialization becomes more generic. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 ^^ don't forget to remove... --- src/conf/storage_conf.h | 1 + src/storage/storage_driver.c | 109 +++ 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index da378b7..8d43019 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -293,6 +293,7 @@ struct _virStorageDriverState { char *configDir; char *autostartDir; +char *stateDir; bool privileged; }; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 64ea770..0180fd7 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -78,6 +78,7 @@ static void storageDriverAutostart(void) { size_t i; +char *stateFile = NULL; virConnectPtr conn = NULL; /* XXX Remove hardcoding of QEMU URI */ @@ -136,7 +137,14 @@ storageDriverAutostart(void) virStoragePoolObjUnlock(pool); continue; } + +if (!(stateFile = virFileBuildPath(driver-stateDir, + pool-def-name, .xml))) +continue; + +ignore_value(virStoragePoolSaveState(stateFile, pool-def)); pool-active = 1; +VIR_FREE(stateFile); I didn't quite pick up on this the first time, but with everything together now it was more obvious... If we cannot build the stateFile path, then pool-active = 1 never happens, but the pool is started... So it seems we'll have to stop the pool here as well and tell why (rather than continue)... Perhaps follow how you did things in storagePoolCreate[XML] with the extra caveats noted below... } virStoragePoolObjUnlock(pool); } @@ -147,61 +155,67 @@ storageDriverAutostart(void) /** * virStorageStartup: * - * Initialization function for the QEmu daemon + * Initialization function for the Storage Driver Yay! thanks ;-) */ static int storageStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { -char *base = NULL; +int ret = -1; +char *configdir = NULL; +char *rundir = NULL; if (VIR_ALLOC(driver) 0) -return -1; +return ret; if (virMutexInit(driver-lock) 0) { VIR_FREE(driver); -return -1; +return ret; } storageDriverLock(); if (privileged) { -if (VIR_STRDUP(base, SYSCONFDIR /libvirt) 0) +if (VIR_STRDUP(driver-configDir, + SYSCONFDIR /libvirt/storage) 0 || +VIR_STRDUP(driver-autostartDir, + SYSCONFDIR /libvirt/storage/autostart) 0 || +VIR_STRDUP(driver-stateDir, + LOCALSTATEDIR /run/libvirt/storage) 0) goto error; } else { -base = virGetUserConfigDirectory(); -if (!base) +configdir = virGetUserConfigDirectory(); +rundir = virGetUserRuntimeDirectory(); +if (!(configdir rundir)) +goto error; + +if ((virAsprintf(driver-configDir, +%s/storage, configdir) 0) || +(virAsprintf(driver-autostartDir, +%s/storage, configdir) 0) || +(virAsprintf(driver-stateDir, + %s/storage/run, rundir) 0)) goto error; } driver-privileged = privileged; -/* - * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/... - * (session) or /etc/libvirt/storage/... (system). - */ -if (virAsprintf(driver-configDir, -%s/storage, base) == -1) -goto error; - -if (virAsprintf(driver-autostartDir, -%s/storage/autostart, base) == -1) -goto error; - -VIR_FREE(base); - if (virStoragePoolLoadAllConfigs(driver-pools, driver-configDir, driver-autostartDir) 0) goto error; storageDriverUnlock(); -return 0; + +ret = 0; + cleanup: +VIR_FREE(configdir); +VIR_FREE(rundir); +return ret; error: -VIR_FREE(base); storageDriverUnlock(); storageStateCleanup(); -return -1; +goto cleanup; } /** @@ -261,6 +275,7 @@ storageStateCleanup(void) VIR_FREE(driver-configDir); VIR_FREE(driver-autostartDir); +VIR_FREE(driver-stateDir); storageDriverUnlock();
Re: [libvirt] [PATCH V3] libxl: fix dom0 balloon logic
On Wed, Apr 01, 2015 at 11:08:54AM -0600, Jim Fehlig wrote: Recent testing on large memory systems revealed a bug in the Xen xl tool's freemem() function. When autoballooning is enabled, freemem() is used to ensure enough memory is available to start a domain, ballooning dom0 if necessary. When ballooning large amounts of memory from dom0, freemem() would exceed its self-imposed wait time and return an error. Meanwhile, dom0 continued to balloon. Starting the domain later, after sufficient memory was ballooned from dom0, would succeed. The libvirt implementation in libxlDomainFreeMem() suffers the same bug since it is modeled after freemem(). In the end, the best place to fix the bug on the Xen side was to slightly change the behavior of libxl_wait_for_memory_target(). Instead of failing after caller-provided wait_sec, the function now blocks as long as dom0 memory ballooning is progressing. It will return failure only when more memory is needed to reach the target and wait_sec have expired with no progress being made. See xen.git commit fd3aa246. There was a dicussion on how this would affect other libxl apps like libvirt http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html If libvirt containing this patch was build against a Xen containing the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() will fail after 30 sec and domain creation will be terminated. Without this patch and with old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() does not succeed after 30 sec, but returns success anyway. Domain creation continues resulting in all sorts of fun stuff like cpu soft lockups in the guest OS. It was decided to properly fix libxl_wait_for_memory_target(), and if anything improve the default behavior of apps using the freemem reference impl in xl. xl was patched to accommodate the change in libxl_wait_for_memory_target() with xen.git commit 883b30a0. This patch does the same in the libxl driver. While at it, I changed the logic to essentially match freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner IMO and will make it easier to spot future, potentially interesting divergences. Signed-off-by: Jim Fehlig jfeh...@suse.com --- V3: Remove unneeded local variable 'ret' in libxlDomainFreeMem. ACK pgpfysqK0AkQl.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState
On 04/02/2015 06:10 AM, Erik Skultety wrote: These functions operate exactly the same as their network equivalents virNetworkLoadAllState, virNetworkLoadState. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 ^^ Don't forget to remove... --- src/conf/storage_conf.c | 94 src/conf/storage_conf.h | 7 src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 13 ++ 4 files changed, 115 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ee564f2..e7d6e6b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1863,6 +1863,100 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, } +virStoragePoolObjPtr +virStoragePoolLoadState(virStoragePoolObjListPtr pools, +const char *stateDir, +const char *name) +{ +char *stateFile = NULL; +virStoragePoolDefPtr def = NULL; +virStoragePoolObjPtr pool = NULL; +xmlDocPtr xml = NULL; +xmlXPathContextPtr ctxt = NULL; +xmlNodePtr node = NULL; + +if (!(stateFile = virFileBuildPath(stateDir, name, .xml))) +goto error; + +if (!(xml = virXMLParseCtxt(stateFile, NULL, _((pool state)), ctxt))) +goto error; + +if (!(node = virXPathNode(//pool, ctxt))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Could not find any 'pool' element in state file)); +goto error; +} + +ctxt-node = node; +if (!(def = virStoragePoolDefParseXML(ctxt))) +goto error; + +if (!STREQ(name, def-name)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Storage pool state file '%s' does not match + pool name '%s'), + stateFile, def-name); +goto error; +} + +/* create the object */ +if (!(pool = virStoragePoolObjAssignDef(pools, def))) +goto error; + +/* XXX: future handling of some additional usefull status data, s/usefull/useful ACK with the slight adjustments. If you want to post a v3 inline/as a response to patch 4 - that's fine John + * for now, if a status file for a pool exists, the pool will be marked + * as active + */ + +pool-active = 1; + + cleanup: +VIR_FREE(stateFile); +xmlFree(xml); +xmlXPathFreeContext(ctxt); +return pool; + + error: +virStoragePoolDefFree(def); +goto cleanup; +} + + +int +virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, + const char *stateDir) +{ +DIR *dir; +struct dirent *entry; +int ret = -1; + +if (!(dir = opendir(stateDir))) { +if (errno == ENOENT) +return 0; + +virReportSystemError(errno, _(Failed to open dir '%s'), stateDir); +return -1; +} + +while ((ret = virDirRead(dir, entry, stateDir)) 0) { +virStoragePoolObjPtr pool; + +if (entry-d_name[0] == '.') +continue; + +if (!virFileStripSuffix(entry-d_name, .xml)) +continue; + +if (!(pool = virStoragePoolLoadState(pools, stateDir, entry-d_name))) +continue; +virStoragePoolObjUnlock(pool); +} + +closedir(dir); +return ret; +} + + int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, const char *configDir, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 8d43019..7471006 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -318,6 +318,13 @@ int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, const char *configDir, const char *autostartDir); +int virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, + const char *stateDir); + +virStoragePoolObjPtr +virStoragePoolLoadState(virStoragePoolObjListPtr pools, +const char *stateDir, +const char *name); virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 56acb01..6b95dea 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -800,6 +800,7 @@ virStoragePoolFormatFileSystemNetTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolGetVhbaSCSIHostParent; virStoragePoolLoadAllConfigs; +virStoragePoolLoadAllState; virStoragePoolObjAssignDef; virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 0180fd7..36c05b3 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@
Re: [libvirt] [PATCH] docs: Add Host sFlow into monitoring apps
On 25.03.2015 16:37, Martin Kletzander wrote: Reported-by: Peter Phaal peter.ph...@gmail.com Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/apps.html.in | 6 ++ 1 file changed, 6 insertions(+) diff --git a/docs/apps.html.in b/docs/apps.html.in index 79ee73a..fd45189 100644 --- a/docs/apps.html.in +++ b/docs/apps.html.in @@ -321,6 +321,12 @@ For a full description, please refer to the libvirt section in the collectd.conf(5) manual page. /dd + dta href=http://host-sflow.sourceforge.net/;Host sFlow/a/dt + dd +Host sFlow is a lightweight agent running on KVM hypervisors that +links to libvirt library and exports standardized cpu, memory, network +and disk metrics for all virtual machines. + /dd dta href=http://honk.sigxcpu.org/projects/libvirt/#munin;Munin/a/dt dd The plugins provided by Guido Guuml;nther allow to monitor various things ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/6] Allocate virtio-serial addresses
On Thu, Apr 02, 2015 at 07:58:32AM -0400, John Ferlan wrote: On 03/24/2015 01:15 PM, Ján Tomko wrote: Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1076708 https://bugzilla.redhat.com/show_bug.cgi?id=890606 New in v3: * preserve the behavior of honoring the controller if a partial address is specified: address type='virtio-serial' controller='2'/ * automatically add a controller if we're out of free ports and add a test for it Ján Tomko (6): Add test for virtio serial port assignment Add functions to track virtio-serial addresses Allocate virtio-serial addresses when starting a domain Expand the address set when attaching a virtio-serial controller Assign an address when hotplugging a virtio-serial device Auto add virtio-serial controllers ... ACK series Pushed now, thanks for the review. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Typos: Get rid of dependan(t|cies)
Dependant is flagged as wrong in US dictionary (only valid in UK dictionary, and even then, it has only the financial sense and not the inter-relatedness sense that we are more prone to be wanting throughout code). Signed-off-by: Martin Kletzander mklet...@redhat.com --- Pushed as trivial and already decided on in here: https://www.redhat.com/archives/libvir-list/2015-March/msg01320.html src/libvirt-domain.c | 4 ++-- src/libvirt-secret.c | 2 +- src/util/virkmod.c | 2 +- tools/virsh.pod | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index f1608dc..0d00af8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2642,7 +2642,7 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) * * Reads native configuration data describing a domain, and * generates libvirt domain XML. The format of the native - * data is hypervisor dependant. + * data is hypervisor dependent. * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. @@ -2692,7 +2692,7 @@ virConnectDomainXMLFromNative(virConnectPtr conn, * * Reads a domain XML configuration document, and generates * a native configuration file describing the domain. - * The format of the native data is hypervisor dependant. + * The format of the native data is hypervisor dependent. * * Returns a 0 terminated UTF-8 encoded native config datafile, or NULL in case of error. * the caller must free() the returned value. diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index 946d1b8..e77f223 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -452,7 +452,7 @@ virSecretGetUsageType(virSecretPtr secret) * * Get the unique identifier of the object with which this * secret is to be used. The format of the identifier is - * dependant on the usage type of the secret. For a secret + * dependent on the usage type of the secret. For a secret * with a usage type of VIR_SECRET_USAGE_TYPE_VOLUME the * identifier will be a fully qualfied path name. The * identifiers are intended to be unique within the set of diff --git a/src/util/virkmod.c b/src/util/virkmod.c index e0bcdfc..219fad6 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -121,7 +121,7 @@ virKModLoad(const char *module, bool useBlacklist) * Remove or unload a module. * * NB: Do not use 'modprobe -r' here as that code will recursively - * unload any modules that were dependancies of the one being removed + * unload any modules that were dependencies of the one being removed * even if things still require them. e.g. it'll see the 'bridge' * module has refcount of 0 and remove it, even if there are bridges * created on the host diff --git a/tools/virsh.pod b/tools/virsh.pod index 5d52761..79d50f9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3235,7 +3235,7 @@ BNote: A storage pool that relies on remote resources such as an times in order to have all the volumes detected (see Bpool-refresh). This is because the corresponding volume devices may not be present in the host's filesystem during the initial pool startup or the current -refresh attempt. The number of refresh retries is dependant upon the +refresh attempt. The number of refresh retries is dependent upon the network connection and the time the host takes to export the corresponding devices. -- 2.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
On Thu, Apr 02, 2015 at 03:07:11PM +0200, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/rpc/virnetsocket.c | 46 ++ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..8d8b6e0 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -545,8 +545,10 @@ int virNetSocketNewConnectUNIX(const char *path, { char *binname = NULL; char *pidpath = NULL; +char *lockpath = NULL; int fd, passfd = -1; int pidfd = -1; +int lockfd = -1; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +563,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } +if (spawnDaemon) { +if (virPidFileConstructPath(false, NULL, libvirt-lock, lockpath) 0) +goto error; + +if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) 0 || +virSetCloseExec(lockfd) 0) { +virReportSystemError(errno, _(Unable to create lock '%s'), lockpath); +goto error; +} + +if (virFileLock(lockfd, false, 0, 1, true) 0) { +virReportSystemError(errno, _(Unable to lock '%s'), lockpath); +goto error; +} +} + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); goto error; @@ -595,17 +613,8 @@ int virNetSocketNewConnectUNIX(const char *path, if (virPidFileConstructPath(false, NULL, binname, pidpath) 0) goto error; -pidfd = virPidFileAcquirePath(pidpath, false, getpid()); -if (pidfd 0) { -/* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ -virResetLastError(); -spawnDaemon = false; -goto retry; -} +if ((pidfd = virPidFileAcquirePath(pidpath, false, getpid())) 0) +goto error; What if someone starts the daemon by hand in the meantime? There should be 'goto retry;', I guess. if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); @@ -668,16 +677,17 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; } -/* - * Do we need to eliminate the super-rare race here any more? It would - * need incorporating the following VIR_FORCE_CLOSE() into a - * virCommandHook inside a virNetSocketForkDaemon(). - */ VIR_FORCE_CLOSE(pidfd); if (virNetSocketForkDaemon(binary, passfd) 0) goto error; There's still a problem here. If someone starts the daemon between the VIR_FORCE_CLOSE() and us starting the daemon, virNetSocketForkDaemon() won't fail as the process will start, but it will die after a while, realizing that there is another daemon running. And this thread will continue with socket connected nowhere; the socket that was passed to the daemon being started was unlinked by the daemon that was started manually. I know this is very, very rare case, but I thought the same about starting multiple virshes (what the heck is the plural of 'virsh' anyway) and look where that got us! I'm not against this approach, it's definitely an improvement for the current situation, but it's not a complete solution. What are your thoughts on that? } +if (lockfd) { +unlink(lockpath); +VIR_FORCE_CLOSE(lockfd); +VIR_FREE(lockpath); +} + localAddr.len = sizeof(localAddr.data); if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) { virReportSystemError(errno, %s, _(Unable to get local socket name)); @@ -694,10 +704,14 @@ int virNetSocketNewConnectUNIX(const
[libvirt] [PATCH 0/5] Add vmport feature (v3)
Hi, The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm. However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. The following patch series allows to configure the vmport feature. The first version sent used a domain attribute, Daniel Berrange suggested to use a feature instead. I sent a second version that didn't receive reviews. Hopefully this one will stand more chances! cheers Marc-André Lureau (5): docs: add domain vmport feature domain/conf: add VIR_DOMAIN_FEATURE_VMPORT qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT qemu: add machine vmport argument tests: add machine-vmport-opt qemu test docs/formatdomain.html.in | 6 + docs/schemas/domaincommon.rng | 9 +++ src/conf/domain_conf.c | 5 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 14 +++ .../qemuxml2argv-machine-vmport-opt.args | 6 + .../qemuxml2argv-machine-vmport-opt.xml| 28 ++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: qemuDomainHotplugVcpus - separate out the add cgroup
Future IOThread setting patches would copy the code anyway, so create and generalize the add the vcpu to a cgroup into its own API. Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_driver.c | 69 +++--- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..fa880b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4635,9 +4635,49 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +typedef int cgroupNewFunc(virCgroupPtr domain, + int id, + bool create, + virCgroupPtr *group); + +static virCgroupPtr +qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, + cgroupNewFunc func, + int index, + char *mem_mask, + pid_t pid) +{ +virCgroupPtr new_cgroup = NULL; +int rv = -1; + +/* Create cgroup */ +if (func(cgroup, index, true, new_cgroup) 0) +return NULL; + +if (mem_mask virCgroupSetCpusetMems(new_cgroup, mem_mask) 0) +goto error; + +/* Add pid/thread to the cgroup */ +rv = virCgroupAddTask(new_cgroup, pid); +if (rv 0) { +virReportSystemError(-rv, + _(unable to add id %d task %d to cgroup), + index, pid); +virCgroupRemove(new_cgroup); +goto error; +} + +return new_cgroup; + + error: +virCgroupFree(new_cgroup); +return NULL; +} + +static int +qemuDomainHotplugVcpus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm-privateData; size_t i; @@ -4726,25 +4766,12 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (nvcpus oldvcpus) { for (i = oldvcpus; i nvcpus; i++) { if (priv-cgroup) { -int rv = -1; -/* Create cgroup for the onlined vcpu */ -if (virCgroupNewVcpu(priv-cgroup, i, true, cgroup_vcpu) 0) +cgroup_vcpu = qemuDomainHotplugAddCgroup(priv-cgroup, + virCgroupNewVcpu, + i, mem_mask, + cpupids[i]); +if (!cgroup_vcpu) goto cleanup; - -if (mem_mask -virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) 0) -goto cleanup; - -/* Add vcpu thread to the cgroup */ -rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); -if (rv 0) { -virReportSystemError(-rv, - _(unable to add vcpu %zu task %d to cgroup), - i, cpupids[i]); -virCgroupRemove(cgroup_vcpu); -goto cleanup; -} - } /* Inherit def-cpuset */ -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin
Future IOThread setting patches would copy the code anyway, so create and generalize a delete cgroup and pindef for the vcpu into its own API. Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_driver.c | 40 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa880b7..5dc1ad4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4675,6 +4675,30 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, } static int +qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup, + cgroupNewFunc func, + int index, + virDomainPinDefPtr **pindef_list, + size_t *npin) +{ +virCgroupPtr new_cgroup = NULL; + +if (cgroup) { +if (func(cgroup, index, false, new_cgroup) 0) +return -1; + +/* Remove the offlined cgroup */ +virCgroupRemove(new_cgroup); +virCgroupFree(new_cgroup); +} + +/* Free pindef setting */ +virDomainPinDel(pindef_list, npin, index); + +return 0; +} + +static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int nvcpus) @@ -4827,19 +4851,11 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } } else { for (i = oldvcpus - 1; i = nvcpus; i--) { -if (priv-cgroup) { -if (virCgroupNewVcpu(priv-cgroup, i, false, cgroup_vcpu) 0) -goto cleanup; - -/* Remove cgroup for the offlined vcpu */ -virCgroupRemove(cgroup_vcpu); -virCgroupFree(cgroup_vcpu); -} +if (qemuDomainHotplugDelCgroupPin(priv-cgroup, virCgroupNewVcpu, i, + vm-def-cputune.vcpupin, + vm-def-cputune.nvcpupin) 0) +goto cleanup; -/* Free vcpupin setting */ -virDomainPinDel(vm-def-cputune.vcpupin, -vm-def-cputune.nvcpupin, -i); } } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] REWORK/REPOST: Refactor some qemuDomainHotplugVcpus code
Originally these were in the middle of the IOThreads Setting code that needs some rework. So rather than needing to have to continually merge changes in this space, I separated them out. These are (hopefully) pure code motion and will be used in some manner by changes that will come this release cycle for IOThread Add/Del Support. IOThreads's posting: http://www.redhat.com/archives/libvir-list/2015-March/msg01014.html and specifically in order: http://www.redhat.com/archives/libvir-list/2015-March/msg01017.html http://www.redhat.com/archives/libvir-list/2015-March/msg01018.html http://www.redhat.com/archives/libvir-list/2015-March/msg01019.html I did have to make some changes to the last one to account for other recent changes in the code John Ferlan (3): qemu: qemuDomainHotplugVcpus - separate out the add cgroup qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin qemu: qemuDomainHotplugVcpus - separate out pin adjustment code src/qemu/qemu_driver.c | 201 - 1 file changed, 130 insertions(+), 71 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: qemuDomainHotplugVcpus - separate out pin adjustment code
Future IOThread setting patches would copy the code anyway, so create and generalize the adding of pindef for the vcpu into its own API Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_driver.c | 92 +- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5dc1ad4..f047403 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4675,6 +4675,56 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, } static int +qemuDomainHotplugAddPin(virBitmapPtr cpumask, +int index, +pid_t pid, +virDomainPinDefPtr **pindef_list, +size_t *npin, +virCgroupPtr cgroup) +{ +int ret = -1; +virDomainPinDefPtr pindef = NULL; + +/* vm-def-cputune.* arrays can't be NULL if + * vm-def-cpumask is not NULL. + */ +if (VIR_ALLOC(pindef) 0) +goto cleanup; + +if (!(pindef-cpumask = virBitmapNewCopy(cpumask))) { +VIR_FREE(pindef); +goto cleanup; +} +pindef-id = index; +if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) 0) { +virBitmapFree(pindef-cpumask); +VIR_FREE(pindef); +goto cleanup; +} + +if (cgroup) { +if (qemuSetupCgroupCpusetCpus(cgroup, pindef-cpumask) 0) { +virReportError(VIR_ERR_OPERATION_INVALID, + _(failed to set cpuset.cpus in cgroup for id %d), + index); +goto cleanup; +} +} else { +if (virProcessSetAffinity(pid, pindef-cpumask) 0) { +virReportError(VIR_ERR_SYSTEM_ERROR, + _(failed to set cpu affinity for id %d), + index); +goto cleanup; +} +} + +ret = 0; + + cleanup: +return ret; +} + +static int qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup, cgroupNewFunc func, int index, @@ -4800,48 +4850,14 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, /* Inherit def-cpuset */ if (vm-def-cpumask) { -/* vm-def-cputune.vcpupin can't be NULL if - * vm-def-cpumask is not NULL. - */ -virDomainPinDefPtr vcpupin = NULL; - -if (VIR_ALLOC(vcpupin) 0) -goto cleanup; - -if (!(vcpupin-cpumask = virBitmapNewCopy(vm-def-cpumask))) { -VIR_FREE(vcpupin); -goto cleanup; -} -vcpupin-id = i; -if (VIR_APPEND_ELEMENT_COPY(vm-def-cputune.vcpupin, -vm-def-cputune.nvcpupin, vcpupin) 0) { -virBitmapFree(vcpupin-cpumask); -VIR_FREE(vcpupin); +if (qemuDomainHotplugAddPin(vm-def-cpumask, i, cpupids[i], +vm-def-cputune.vcpupin, +vm-def-cputune.nvcpupin, +cgroup_vcpu) 0) { ret = -1; goto cleanup; } - -if (cgroup_vcpu) { -if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, - vcpupin-cpumask) 0) { -virReportError(VIR_ERR_OPERATION_INVALID, - _(failed to set cpuset.cpus in cgroup - for vcpu %zu), i); -ret = -1; -goto cleanup; -} -} else { -if (virProcessSetAffinity(cpupids[i], - vcpupin-cpumask) 0) { -virReportError(VIR_ERR_SYSTEM_ERROR, - _(failed to set cpu affinity for vcpu %zu), - i); -ret = -1; -goto cleanup; -} -} } - virCgroupFree(cgroup_vcpu); if (qemuProcessSetSchedParams(i, cpupids[i], -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] domain/conf: add VIR_DOMAIN_FEATURE_VMPORT
--- src/conf/domain_conf.c | 5 - src/conf/domain_conf.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1763305..27f15b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, kvm, pvspinlock, capabilities, - pmu) + pmu, + vmport) VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, default, @@ -14237,6 +14238,7 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: +case VIR_DOMAIN_FEATURE_VMPORT: node = ctxt-node; ctxt-node = nodes[i]; if ((tmp = virXPathString(string(./@state), ctxt))) { @@ -20853,6 +20855,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: +case VIR_DOMAIN_FEATURE_VMPORT: switch ((virTristateSwitch) def-features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0b2f1c9..edd488b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1634,6 +1634,7 @@ typedef enum { VIR_DOMAIN_FEATURE_PVSPINLOCK, VIR_DOMAIN_FEATURE_CAPABILITIES, VIR_DOMAIN_FEATURE_PMU, +VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] docs: add domain vmport feature
A new feature that can be turned on or off. The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm. However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. --- docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 9 + 2 files changed, 15 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..be588c8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1500,6 +1500,12 @@ performance monitoring unit for the guest. span class=sinceSince 1.2.12/span /dd + dtcodevmport/code/dt + ddDepending on the codestate/code attribute (values codeon/code, +codeoff/code, default codeon/code) enable or disable the +the emulation of VMWare IO port, for vmmouse etc. +span class=sinceSince 1.2.15/span + /dd /dl h3a name=elementsTimeTime keeping/a/h3 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..b8e06b3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4149,6 +4149,15 @@ optional ref name=pmu/ /optional + optional +element name=vmport + optional +attribute name=state + ref name=virOnOff/ +/attribute + /optional +/element + /optional /interleave /element /optional -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Honour sparse files on migration
When the storage is pre-created on the destination, it might have been allocated as sparse on the source. Let's keep it that way if we can. Michal Privoznik (3): qemuMigrationPrecreateStorage: Fix debug message qemuMigrationCookieNBD: Turn some items into their own struct qemuMigrationPrecreateDisk: Preserve sparse files src/qemu/qemu_migration.c | 49 +++ 1 file changed, 37 insertions(+), 12 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemuMigrationPrecreateDisk: Preserve sparse files
https://bugzilla.redhat.com/show_bug.cgi?id=817700 When pre-creating a disk on the destination, a volume XML is constructed. The XML is then passed to virStorageVolCreateXML() which does the work. But, since there's no allocation/ in the XML, the disk are fully allocated. This possibly breaks sparse allocation user has on the migration source. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 31 ++- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3adb949..a2f68ed 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -144,6 +144,7 @@ typedef qemuMigrationCookieNBDDisk *qemuMigrationCookieNBDDiskPtr; struct _qemuMigrationCookieNBDDisk { char *target; /* Disk target */ unsigned long long capacity;/* And its capacity */ +unsigned long long allocation; /* And its allocation */ }; typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD; @@ -591,6 +592,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, disk-dst) 0) goto cleanup; mig-nbd-disks[mig-nbd-ndisks].capacity = entry-capacity; +mig-nbd-disks[mig-nbd-ndisks].allocation = entry-wr_highest_offset; mig-nbd-ndisks++; } @@ -831,8 +833,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, for (i = 0; i mig-nbd-ndisks; i++) { virBufferEscapeString(buf, disk target='%s', mig-nbd-disks[i].target); -virBufferAsprintf(buf, capacity='%llu'/\n, - mig-nbd-disks[i].capacity); +virBufferAsprintf(buf, capacity='%llu' allocation='%llu'/\n, + mig-nbd-disks[i].capacity, + mig-nbd-disks[i].allocation); } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, /nbd\n); @@ -970,7 +973,7 @@ static qemuMigrationCookieNBDPtr qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) { qemuMigrationCookieNBDPtr ret = NULL; -char *port = NULL, *capacity = NULL; +char *port = NULL, *capacity = NULL, *allocation = NULL; size_t i; int n; xmlNodePtr *disks = NULL; @@ -998,6 +1001,7 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) for (i = 0; i n; i++) { ctxt-node = disks[i]; VIR_FREE(capacity); +VIR_FREE(allocation); if (!(ret-disks[i].target = virXPathString(string(./@target), ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -1014,12 +1018,26 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) NULLSTR(capacity)); goto error; } + +allocation = virXPathString(string(./@allocation), ctxt); +if (allocation) { +if (virStrToLong_ull(allocation, NULL, 10, + ret-disks[i].allocation) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Malformed disk allocation: '%s'), + allocation); +goto error; +} +} else { +ret-disks[i].allocation = -1; +} } } cleanup: VIR_FREE(port); VIR_FREE(capacity); +VIR_FREE(allocation); VIR_FREE(disks); ctxt-node = save_ctxt; return ret; @@ -1539,6 +1557,8 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, name%s/name\n, volName); virBufferAsprintf(buf, capacity%llu/capacity\n, nbd-capacity); +if (nbd-allocation != -1) +virBufferAsprintf(buf, allocation%llu/allocation\n, nbd-allocation); virBufferAddLit(buf, target\n); virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, format type='%s'/\n, format); @@ -1583,8 +1603,9 @@ qemuMigrationPrecreateStorage(virConnectPtr conn, int indx; const char *diskSrcPath; -VIR_DEBUG(Looking up disk target '%s' (capacity=%llu), - nbd-disks[i].target, nbd-disks[i].capacity); +VIR_DEBUG(Looking up disk target '%s' (capacity=%llu allocation=%llu), + nbd-disks[i].target, nbd-disks[i].capacity, + nbd-disks[i].allocation); if ((indx = virDomainDiskIndexByName(vm-def, nbd-disks[i].target, false)) 0) { -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: add machine vmport argument
Fill qemu command line vmport argument as required. --- src/qemu/qemu_command.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e94caea..22df0ad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7311,6 +7311,7 @@ qemuBuildMachineArgStr(virCommandPtr cmd, obsoleteAccel = true; } else { virBuffer buf = VIR_BUFFER_INITIALIZER; +virTristateSwitch vmport = def-features[VIR_DOMAIN_FEATURE_VMPORT]; virCommandAddArg(cmd, -machine); virBufferAdd(buf, def-os.machine, -1); @@ -7328,6 +7329,19 @@ qemuBuildMachineArgStr(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT)) virBufferAddLit(buf, ,usb=off); +if (vmport) { +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(vmport is not available + with this QEMU binary)); +virBufferFreeAndReset(buf); +return -1; +} + +virBufferAsprintf(buf, ,vmport=%s, + virTristateSwitchTypeToString(vmport)); +} + if (def-mem.dump_core) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT
vmport machine argument is only supported since qemu 2.2.0 --- src/qemu/qemu_capabilities.c | 6 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce6767c..8782613 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, qxl.vgamem_mb, qxl-vga.vgamem_mb, pc-dimm, + + machine-vmport-opt, /* 185 */ ); @@ -3239,6 +3241,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT); +/* vmport option is supported v2.2.0 onwards */ +if (qemuCaps-version = 2002000) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); + /* WebSockets were introduced between 1.3.0 and 1.3.1 */ if (qemuCaps-version = 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c7b1ac7..48c8f96 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -224,6 +224,7 @@ typedef enum { QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ +QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v1: -hopefully this one is race free src/rpc/virnetsocket.c | 148 - 1 file changed, 36 insertions(+), 112 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..2b891ae 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -123,7 +123,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket) #ifndef WIN32 -static int virNetSocketForkDaemon(const char *binary, int passfd) +static int virNetSocketForkDaemon(const char *binary) { int ret; virCommandPtr cmd = virCommandNewArgList(binary, @@ -136,10 +136,6 @@ static int virNetSocketForkDaemon(const char *binary, int passfd) virCommandAddEnvPassBlockSUID(cmd, XDG_RUNTIME_DIR, NULL); virCommandClearCaps(cmd); virCommandDaemonize(cmd); -if (passfd) { -virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); -virCommandPassListenFDs(cmd); -} ret = virCommandRun(cmd, NULL); virCommandFree(cmd); return ret; @@ -543,10 +539,10 @@ int virNetSocketNewConnectUNIX(const char *path, const char *binary, virNetSocketPtr *retsock) { -char *binname = NULL; -char *pidpath = NULL; -int fd, passfd = -1; -int pidfd = -1; +char *lockpath = NULL; +int lockfd = -1; +int fd = -1; +int retries = 100; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +557,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } +if (spawnDaemon) { +if (virPidFileConstructPath(false, NULL, libvirt-lock, lockpath) 0) +goto error; + +if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) 0 || +virSetCloseExec(lockfd) 0) { +virReportSystemError(errno, _(Unable to create lock '%s'), lockpath); +goto error; +} + +if (virFileLock(lockfd, false, 0, 1, true) 0) { +virReportSystemError(errno, _(Unable to lock '%s'), lockpath); +goto error; +} +} + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { virReportSystemError(errno, %s, _(Failed to create socket)); goto error; @@ -574,108 +586,25 @@ int virNetSocketNewConnectUNIX(const char *path, if (remoteAddr.data.un.sun_path[0] == '@') remoteAddr.data.un.sun_path[0] = '\0'; - retry: -if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) { -int status = 0; -pid_t pid = 0; - -if (!spawnDaemon) { +while (retries + connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) { +if (!(spawnDaemon errno == ENOENT)) { virReportSystemError(errno, _(Failed to connect socket to '%s'), path); goto error; } -if (!(binname = last_component(binary)) || binname[0] == '\0') { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Cannot determine basename for binary '%s'), - binary); -goto error; -} - -if (virPidFileConstructPath(false, NULL, binname, pidpath) 0) -goto error; - -pidfd = virPidFileAcquirePath(pidpath, false, getpid()); -if (pidfd 0) { -/* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ -virResetLastError(); -spawnDaemon = false; -goto retry; -} - -if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) { -virReportSystemError(errno, %s, _(Failed to create socket)); +if (virNetSocketForkDaemon(binary) 0) goto error; -} -/* - * We
[libvirt] [PATCH 1/3] qemuMigrationPrecreateStorage: Fix debug message
When pre-creating storage for domains, we need to find corresponding disk in the XML on the destination (domain XML may differ there, e.g. disk is accessible under different path). For better debugging, I'm printing all info I received on a disk. But there was a typo when printing the disk capacity: %lluu instead of %llu. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8c45415..b20ede8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1579,7 +1579,7 @@ qemuMigrationPrecreateStorage(virConnectPtr conn, int indx; const char *diskSrcPath; -VIR_DEBUG(Looking up disk target '%s' (capacity=%lluu), +VIR_DEBUG(Looking up disk target '%s' (capacity=%llu), nbd-disks[i].target, nbd-disks[i].capacity); if ((indx = virDomainDiskIndexByName(vm-def, -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemuMigrationCookieNBD: Turn some items into their own struct
It will be more future proof if we just pass a pointer to a single NBD disk representation to the qemuMigrationPrecreateDisk() instead of copying every single item onto the stack. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b20ede8..3adb949 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -139,16 +139,20 @@ struct _qemuMigrationCookieNetwork { qemuMigrationCookieNetDataPtr net; }; +typedef struct _qemuMigrationCookieNBDDisk qemuMigrationCookieNBDDisk; +typedef qemuMigrationCookieNBDDisk *qemuMigrationCookieNBDDiskPtr; +struct _qemuMigrationCookieNBDDisk { +char *target; /* Disk target */ +unsigned long long capacity;/* And its capacity */ +}; + typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD; typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr; struct _qemuMigrationCookieNBD { int port; /* on which port does NBD server listen for incoming data */ size_t ndisks; /* Number of items in @disk array */ -struct { -char *target; /* Disk target */ -unsigned long long capacity;/* And its capacity */ -} *disks; +qemuMigrationCookieNBDDiskPtr disks; }; typedef struct _qemuMigrationCookie qemuMigrationCookie; @@ -1458,7 +1462,7 @@ qemuMigrationRestoreDomainState(virConnectPtr conn, virDomainObjPtr vm) static int qemuMigrationPrecreateDisk(virConnectPtr conn, virDomainDiskDefPtr disk, - unsigned long long capacity) + qemuMigrationCookieNBDDiskPtr nbd) { int ret = -1; virStoragePoolPtr pool = NULL; @@ -1534,7 +1538,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, virBufferAddLit(buf, volume\n); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, name%s/name\n, volName); -virBufferAsprintf(buf, capacity%llu/capacity\n, capacity); +virBufferAsprintf(buf, capacity%llu/capacity\n, nbd-capacity); virBufferAddLit(buf, target\n); virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, format type='%s'/\n, format); @@ -1601,7 +1605,7 @@ qemuMigrationPrecreateStorage(virConnectPtr conn, VIR_DEBUG(Proceeding with disk source %s, NULLSTR(diskSrcPath)); -if (qemuMigrationPrecreateDisk(conn, disk, nbd-disks[i].capacity) 0) +if (qemuMigrationPrecreateDisk(conn, disk, nbd-disks + i) 0) goto cleanup; } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt-1.2.14 candidate release 2
Hi, On Tue, Mar 31, 2015 at 09:01:33AM +0800, Daniel Veillard wrote: So I have just tagged in git and pushed the 1.2.14 candidate release 2, it is available as usual as signed tarballs and rpms from: ftp://libvirt.org/libvirt/ I did push the 3 patches from Peter because what is the point of a candidate release if it doesn't include the potentially controversial bits that we intend for the final release ? That doesn't prevent someone else than Eric to give the 2nd ACK for patch 3 of the series :-) So please git it a try, if needed we will just do an rc3 ! BTW is there regression testing done from oVirt using libvirt upstream on an automated basis ? If not that's something I could help pushing for assuming we have public reg tests for oVirt. At least it seems to work in my minimal own tests, but others should check, Looks good on Debian's Buildds: https://buildd.debian.org/status/package.php?p=libvirtsuite=experimental Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] tests: add machine-vmport-opt qemu test
Check that the vmport feature is correctly used in qemu commande line. --- .../qemuxml2argv-machine-vmport-opt.args | 6 + .../qemuxml2argv-machine-vmport-opt.xml| 28 ++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 36 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args new file mode 100644 index 000..ea1a11f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-S -machine pc,accel=tcg,vmport=off -m 214 -smp 1 -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-machine-vmport-opt.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml new file mode 100644 index 000..671d3a9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + features +vmport state='off'/ + /features + 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' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c02555d..45c9015 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -608,6 +608,8 @@ mymain(void) DO_TEST_FAILURE(machine-core-on, QEMU_CAPS_MACHINE_OPT); DO_TEST(machine-usb-opt, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_USB_OPT); +DO_TEST(machine-vmport-opt, QEMU_CAPS_MACHINE_OPT, +QEMU_CAPS_MACHINE_VMPORT_OPT); DO_TEST(kvm, QEMU_CAPS_MACHINE_OPT); DO_TEST(boot-cdrom, NONE); DO_TEST(boot-network, NONE); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Duplicate storage pool source refactoring and checks
A precursor to some changes I'm working on - I figured it was better not to hoard the patches and then drop a 15 patch series... Anyway, patches 1-4 refactor the iSCSI source duplicate checks into a separate subroutine. While I was at it, I realized the Host checks are duplicate of the NETFS check - so I further separated things out. As I was looking at other possible defs I realized not all of the proposed storage pool defs go through duplicate checks because the switch statement had the 'default:' case - so I removed that, added all the options with the hope that new storage pool types will add their own checking. This left me with a bit of sleuthing for cases I added. I was able to determine that Sheepdog and Gluster both have the possibility of a single source host, so I added that check as it doesn't seem like it would be a good idea to have two storage pools looking at the same gluster/sheepdog source For the zfs pool it seems that it's like the DISK, LOGICAL, and FS pools with regard to not having duplicate devices in the pool For mpath there doesn't seem to be any need, but I could be wrong For rbd it wasn't completely clear what to check since multiple hosts are allowed for a single pool - I suppose a proposed definition should check that none of it's definitions match, but then again I'm not clear so I left it alone John Ferlan (9): storage: Refactor iSCSI Source matching storage: Refactor matchISCSISource storage: Invert logic for matchISCSISource storage: Create matchPoolSourceHost storage: Use matchPoolSourceHost for NETFS storage: Remove default from switch in virStoragePoolSourceFindDuplicate storage: Add duplicate host check for Sheepdog pool def storage: Add duplicate host check for Gluster pool def storage: Add duplicate devices check for zfs pool def src/conf/storage_conf.c | 59 +++-- 1 file changed, 43 insertions(+), 16 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] storage: Use matchPoolSourceHost for NETFS
Rather than have duplicate code doing the same check, have the netfs matching processing code use the new matchPoolSourceHost. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index b3e930b..6ed0aa9 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2348,9 +2348,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, matchpool = pool; break; case VIR_STORAGE_POOL_NETFS: -if ((STREQ(pool-def-source.dir, def-source.dir)) \ - (pool-def-source.nhost == 1 def-source.nhost == 1) \ - (STREQ(pool-def-source.hosts[0].name, def-source.hosts[0].name))) +if (STREQ(pool-def-source.dir, def-source.dir) +matchPoolSourceHost(pool-def-source, def-source)) matchpool = pool; break; case VIR_STORAGE_POOL_SCSI: -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] storage: Refactor matchISCSISource
Continuing on the same reduction them, foobar'd Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4a38416..fa7a7f9 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2295,13 +2295,12 @@ static bool matchISCSISource(virStoragePoolObjPtr matchpool, virStoragePoolDefPtr def) { -if (matchpool-def-source.nhost == 1 def-source.nhost == 1) { -if (STREQ(matchpool-def-source.hosts[0].name, - def-source.hosts[0].name)) { -if ((matchpool-def-source.initiator.iqn) -(def-source.initiator.iqn)) { -if (STREQ(matchpool-def-source.initiator.iqn, - def-source.initiator.iqn)) +virStoragePoolSourcePtr poolsrc = matchpool-def-source; +virStoragePoolSourcePtr defsrc = def-source; +if (poolsrc-nhost == 1 defsrc-nhost == 1) { +if (STREQ(poolsrc-hosts[0].name, defsrc-hosts[0].name)) { +if (poolsrc-initiator.iqn defsrc-initiator.iqn) { +if (STREQ(poolsrc-initiator.iqn, defsrc-initiator.iqn)) return true; else return false; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/9] storage: Invert logic for matchISCSISource
Invert the logic for better readability/flow and futher separation Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index fa7a7f9..e4cb54b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2297,18 +2297,19 @@ matchISCSISource(virStoragePoolObjPtr matchpool, { virStoragePoolSourcePtr poolsrc = matchpool-def-source; virStoragePoolSourcePtr defsrc = def-source; -if (poolsrc-nhost == 1 defsrc-nhost == 1) { -if (STREQ(poolsrc-hosts[0].name, defsrc-hosts[0].name)) { -if (poolsrc-initiator.iqn defsrc-initiator.iqn) { -if (STREQ(poolsrc-initiator.iqn, defsrc-initiator.iqn)) -return true; -else -return false; -} -return true; -} -} -return false; + + +/* NB: nhost cannot be 1 */ +if (poolsrc-nhost == 0 || defsrc-nhost == 0) +return false; + +if (STRNEQ(poolsrc-hosts[0].name, defsrc-hosts[0].name)) +return false; + +if (STRNEQ_NULLABLE(poolsrc-initiator.iqn, defsrc-initiator.iqn)) +return false; + +return true; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/9] storage: Create matchPoolSourceHost
Split out the nhost == 1 and hosts[0].name logic into a separate routine Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e4cb54b..b3e930b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2290,6 +2290,17 @@ matchSCSIAdapterParent(virStoragePoolObjPtr pool, return false; } +static bool +matchPoolSourceHost(virStoragePoolSourcePtr poolsrc, +virStoragePoolSourcePtr defsrc) +{ +/* NB: nhost cannot be 1 */ +if (poolsrc-nhost == 0 || defsrc-nhost == 0) +return false; + +return STREQ(poolsrc-hosts[0].name, defsrc-hosts[0].name); +} + static bool matchISCSISource(virStoragePoolObjPtr matchpool, @@ -2299,11 +2310,7 @@ matchISCSISource(virStoragePoolObjPtr matchpool, virStoragePoolSourcePtr defsrc = def-source; -/* NB: nhost cannot be 1 */ -if (poolsrc-nhost == 0 || defsrc-nhost == 0) -return false; - -if (STRNEQ(poolsrc-hosts[0].name, defsrc-hosts[0].name)) +if (!matchPoolSourceHost(poolsrc, defsrc)) return false; if (STRNEQ_NULLABLE(poolsrc-initiator.iqn, defsrc-initiator.iqn)) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] storage: Remove default from switch in virStoragePoolSourceFindDuplicate
So that we can cover all the cases. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6ed0aa9..5f1c151 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2342,7 +2342,7 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjLock(pool); -switch (pool-def-type) { +switch ((virStoragePoolType)pool-def-type) { case VIR_STORAGE_POOL_DIR: if (STREQ(pool-def-target.path, def-target.path)) matchpool = pool; @@ -2427,7 +2427,12 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, case VIR_STORAGE_POOL_DISK: matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); break; -default: +case VIR_STORAGE_POOL_MPATH: +case VIR_STORAGE_POOL_RBD: +case VIR_STORAGE_POOL_SHEEPDOG: +case VIR_STORAGE_POOL_GLUSTER: +case VIR_STORAGE_POOL_ZFS: +case VIR_STORAGE_POOL_LAST: break; } virStoragePoolObjUnlock(pool); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] storage: Refactor iSCSI Source matching
Create a separate iSCSI Source matching subroutine. Makes the calling code a bit cleaner as well as sets up for future patches which need to do better source hosts[0].name processing/checking Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b1898b..4a38416 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2291,6 +2291,28 @@ matchSCSIAdapterParent(virStoragePoolObjPtr pool, } +static bool +matchISCSISource(virStoragePoolObjPtr matchpool, + virStoragePoolDefPtr def) +{ +if (matchpool-def-source.nhost == 1 def-source.nhost == 1) { +if (STREQ(matchpool-def-source.hosts[0].name, + def-source.hosts[0].name)) { +if ((matchpool-def-source.initiator.iqn) +(def-source.initiator.iqn)) { +if (STREQ(matchpool-def-source.initiator.iqn, + def-source.initiator.iqn)) +return true; +else +return false; +} +return true; +} +} +return false; +} + + int virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, @@ -2390,17 +2412,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, case VIR_STORAGE_POOL_ISCSI: matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); if (matchpool) { -if (matchpool-def-source.nhost == 1 def-source.nhost == 1) { -if (STREQ(matchpool-def-source.hosts[0].name, def-source.hosts[0].name)) { -if ((matchpool-def-source.initiator.iqn) (def-source.initiator.iqn)) { -if (STREQ(matchpool-def-source.initiator.iqn, def-source.initiator.iqn)) -break; -matchpool = NULL; -} -break; -} -} -matchpool = NULL; +if (!matchISCSISource(matchpool, def)) +matchpool = NULL; } break; case VIR_STORAGE_POOL_FS: -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] storage: Add duplicate host check for Gluster pool def
Check proposed pool definitions to ensure they aren't trying to use the same host as currently defined definitions - disallow the duplicate Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5db7478..e1599a4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2428,12 +2428,12 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); break; case VIR_STORAGE_POOL_SHEEPDOG: +case VIR_STORAGE_POOL_GLUSTER: if (matchPoolSourceHost(pool-def-source, def-source)) matchpool = pool; break; case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: -case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_ZFS: case VIR_STORAGE_POOL_LAST: break; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] storage: Add duplicate host check for Sheepdog pool def
Check proposed pool definitions to ensure they aren't trying to use the same host as currently defined definitions - disallow the duplicate Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5f1c151..5db7478 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2427,9 +2427,12 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, case VIR_STORAGE_POOL_DISK: matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); break; +case VIR_STORAGE_POOL_SHEEPDOG: +if (matchPoolSourceHost(pool-def-source, def-source)) +matchpool = pool; +break; case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: -case VIR_STORAGE_POOL_SHEEPDOG: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_ZFS: case VIR_STORAGE_POOL_LAST: -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] storage: Add duplicate devices check for zfs pool def
Check proposed pool definitions to ensure they aren't trying to use the same devices as currently defined definitions - disallow the duplicate Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e1599a4..b90af3a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2425,6 +2425,7 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, case VIR_STORAGE_POOL_FS: case VIR_STORAGE_POOL_LOGICAL: case VIR_STORAGE_POOL_DISK: +case VIR_STORAGE_POOL_ZFS: matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); break; case VIR_STORAGE_POOL_SHEEPDOG: @@ -2434,7 +2435,6 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, break; case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: -case VIR_STORAGE_POOL_ZFS: case VIR_STORAGE_POOL_LAST: break; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list