[libvirt] Release of libvirt-1.2.14

2015-04-02 Thread Daniel Veillard
  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

2015-04-02 Thread Luyao Huang
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

2015-04-02 Thread Luyao Huang
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

2015-04-02 Thread Luyao Huang
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread Jiri Denemark
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

2015-04-02 Thread Michael Mueller
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread Peter Krempa
---

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

2015-04-02 Thread Peter Krempa
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

2015-04-02 Thread Peter Krempa
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

2015-04-02 Thread Daniel Veillard
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

2015-04-02 Thread Chen, Hanxiao


 -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

2015-04-02 Thread Peter Krempa
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread Prerna Saxena

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

2015-04-02 Thread Martin Kletzander

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

2015-04-02 Thread Erik Skultety


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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread Erik Skultety
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Erik Skultety
 
 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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Jim Fehlig
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

2015-04-02 Thread Ján Tomko
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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Martin Kletzander

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

2015-04-02 Thread Huanle Han
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 Thread Huanle Han
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

2015-04-02 Thread Eduardo Habkost
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

2015-04-02 Thread Jim Fehlig
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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Martin Kletzander

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

2015-04-02 Thread John Ferlan


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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Ján Tomko
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)

2015-04-02 Thread Martin Kletzander
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

2015-04-02 Thread Martin Kletzander

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)

2015-04-02 Thread Marc-André Lureau
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread Marc-André Lureau
---
 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

2015-04-02 Thread Marc-André Lureau
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Marc-André Lureau
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

2015-04-02 Thread Marc-André Lureau
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Michal Privoznik
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

2015-04-02 Thread Guido Günther
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

2015-04-02 Thread Marc-André Lureau
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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

2015-04-02 Thread John Ferlan
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