[libvirt] [PATCH 1/4] parallels: cpumask support

[Maxim Nestratov]

Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
 src/parallels/parallels_driver.c |4 +---
 src/parallels/parallels_sdk.c|   31 ---
 src/parallels/parallels_utils.h  |1 -
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 650b790..09d1cca 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -777,7 +777,6 @@ parallelsDomainGetVcpus(virDomainPtr domain,
 int maplen)
 {
 parallelsConnPtr privconn = domain-conn-privateData;
-parallelsDomObjPtr privdomdata = NULL;
 virDomainObjPtr privdom = NULL;
 size_t i;
 int v, maxcpu, hostcpus;
@@ -799,7 +798,6 @@ parallelsDomainGetVcpus(virDomainPtr domain,
 goto cleanup;
 }
 
-privdomdata = privdom-privateData;
 if ((hostcpus = nodeGetCPUCount())  0)
 goto cleanup;
 
@@ -820,7 +818,7 @@ parallelsDomainGetVcpus(virDomainPtr domain,
 int tmpmapLen = 0;
 
 memset(cpumaps, 0, maplen * maxinfo);
-virBitmapToData(privdomdata-cpumask, tmpmap, tmpmapLen);
+virBitmapToData(privdom-def-cpumask, tmpmap, tmpmapLen);
 if (tmpmapLen  maplen)
 tmpmapLen = maplen;
 
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fec145d..5a3969e 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -412,7 +412,6 @@ prlsdkDomObjFreePrivate(void *p)
 return;
 
 PrlHandle_Free(pdom-sdkdom);
-virBitmapFree(pdom-cpumask);
 VIR_FREE(pdom-uuid);
 VIR_FREE(pdom-home);
 VIR_FREE(p);
@@ -1053,8 +1052,7 @@ prlsdkConvertDomainState(VIRTUAL_MACHINE_STATE 
domainState,
 
 static int
 prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
- virDomainDefPtr def,
- parallelsDomObjPtr pdom)
+ virDomainDefPtr def)
 {
 char *buf;
 PRL_UINT32 buflen = 0;
@@ -1085,11 +1083,11 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
 pret = PrlVmCfg_GetCpuMask(sdkdom, buf, buflen);
 
 if (strlen(buf) == 0) {
-if (!(pdom-cpumask = virBitmapNew(hostcpus)))
+if (!(def-cpumask = virBitmapNew(hostcpus)))
 goto cleanup;
-virBitmapSetAll(pdom-cpumask);
+virBitmapSetAll(def-cpumask);
 } else {
-if (virBitmapParse(buf, 0, pdom-cpumask, hostcpus)  0)
+if (virBitmapParse(buf, 0, def-cpumask, hostcpus)  0)
 goto cleanup;
 }
 
@@ -1217,7 +1215,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
  convert to Kbytes */
 def-mem.cur_balloon = def-mem.max_balloon;
 
-if (prlsdkConvertCpuInfo(sdkdom, def, pdom)  0)
+if (prlsdkConvertCpuInfo(sdkdom, def)  0)
 goto error;
 
 if (prlsdkConvertCpuMode(sdkdom, def)  0)
@@ -1807,13 +1805,6 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 return -1;
 }
 
-if (def-cpumask != NULL) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(changing cpu mask is not supported 
- by parallels driver));
-return -1;
-}
-
 if (def-cputune.shares ||
 def-cputune.sharesSpecified ||
 def-cputune.period ||
@@ -2842,6 +2833,7 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
 size_t i;
 char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
 bool needBoot = true;
+char *mask = NULL;
 
 if (prlsdkCheckUnsupportedParams(sdkdom, def)  0)
 return -1;
@@ -2869,6 +2861,13 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
 pret = PrlVmCfg_SetCpuCount(sdkdom, def-vcpus);
 prlsdkCheckRetGoto(pret, error);
 
+if (!(mask = virBitmapFormat(def-cpumask)))
+goto error;
+
+pret = PrlVmCfg_SetCpuMask(sdkdom, mask);
+prlsdkCheckRetGoto(pret, error);
+VIR_FREE(mask);
+
 if (prlsdkClearDevices(sdkdom)  0)
 goto error;
 
@@ -2912,7 +2911,9 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
 return 0;
 
  error:
-return -1;
+VIR_FREE(mask);
+
+   return -1;
 }
 
 int
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index ead5586..394548a 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -69,7 +69,6 @@ struct parallelsDomObj {
 int id;
 char *uuid;
 char *home;
-virBitmapPtr cpumask;
 PRL_HANDLE sdkdom;
 };
 
-- 
1.7.1

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

[libvirt] [PATCH 2/4] parallels: don't forget to unlock domain in parallelsDomainHasManagedSaveImage

[Maxim Nestratov]

Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
 src/parallels/parallels_driver.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 09d1cca..bbe9450 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -973,6 +973,9 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, 
unsigned int flags)
 return -1;
 }
 
+   if (dom)
+virObjectUnlock(dom);
+
 return 0;
 }
 
-- 
1.7.1

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

[libvirt] [PATCH 4/4] parallels: prevent domain define only if vcpupin is specified

[Maxim Nestratov]

and their settings differ from common cpumask

Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
 src/parallels/parallels_sdk.c |   14 --
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 4ec9161..fde908b 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1808,14 +1808,24 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 if (def-cputune.shares ||
 def-cputune.sharesSpecified ||
 def-cputune.period ||
-def-cputune.quota ||
-def-cputune.nvcpupin) {
+def-cputune.quota) {
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(cputune is not supported by parallels driver));
 return -1;
 }
 
+if (def-cputune.vcpupin) {
+   for (i = 0; i  def-vcpus; i++) {
+if (!virBitmapEqual(def-cpumask,
+def-cputune.vcpupin[i]-cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(vcpupin cpumask differs from default 
cpumask));
+return -1;
+}
+}
+}
+
 
 /*
  * Though we don't support NUMA configuration at the moment
-- 
1.7.1

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

[libvirt] [PATCH 0/4] parallels: fix different domain define parameters

[Maxim Nestratov]

Actually our checking for unsupported parameters is too strict about
cpumask, vcpupin and NUMA parameters and in general wrong.
First of all, we do support cpumask and consequently we don't have to prevent
domain definition in case cpumask is specified. Also we should allow vcpupin
parameters if they don't contradict with common cpumask. NUMA checking was also
wrong because def-numa is always allocated. Here we check that its content is
not specified. 

Maxim Nestratov (4):
  parallels: cpumask support
  parallels: don't forget to unlock domain in
parallelsDomainHasManagedSaveImage
  parallels: prevent domain define only if NUMA is really specified
  parallels: prevent domain define only if vcpupin is specified

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

Re: [libvirt] [PATCH v5 4/5] qemu: Add support to pin IOThreads to specific CPU

[John Ferlan]

On 03/10/2015 12:37 PM, Ján Tomko wrote:
 On Tue, Mar 10, 2015 at 11:20:43AM -0400, John Ferlan wrote:


 On 03/10/2015 09:51 AM, Ján Tomko wrote:
 On Fri, Mar 06, 2015 at 09:05:44AM -0500, John Ferlan wrote:

 ...snip...

 +
 +/* pinning to all physical cpus means resetting,

 It doesn't.
 By default the iothreads inherit the pinning from the domain's cpumask.

 A completely clear bitmap would be a better value to mean resetting,
 since it makes no sense otherwise. But getting the cpumask in that case
 won't be that easy.


 So again - this is taken from qemuDomainPinVcpuFlags() - if it's invalid
 here, then it's invalid there as well.
 
 Yes, that function might also not behave properly on the corner case of
 pinning a vcpu to all CPUs.
 
 But that one has been released for ages. This is new code that doesn't
 have to repeat its mistakes.
 

No issues with doing things right and obviously I wasn't aware
the current API's are broken...


 Is your objection the comment?  Should it be striken or restated?

 
 It's not the comment I have a problem with, it's the behavior that follows it.
 
 Calling this API on a domain with a per-domain cpuset to pin an iothread
 to all pCPUs will result in:
 
 a) for AFFECT_LIVE
   The iothread will get pinned to all pCPUs
   The pininfo will get deleted from the definition. But missing pininfo
   does not mean all pCPUs if there is a per-domain cpuset.
 
 b) for AFFECT_CONFIG
   The pininfo will be deleted, even though it does not match the
   cpumask.
 
 I think the easiest 'fix' is to just drop all the 'doReset' functionality
 and do not allow deleting pininfo with this API.
 
 Alternatively, (since you already rejected
 VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO above) you can pin the iothread to
 the domain's cpuset.
 
 Also, the full bitmap is ambiguous - does it means 'reset
 it to default' or 'pin it to all pCPUs'?
 

Well let's just say I don't know all the nuances and known
issues of cpu pinning... 

Knowing that the other APIs (vcpu and emulator) are broken - are
you planning to send patches to fix them as well? Or is that
something I should do mimicing whatever is settled upon here?

So, from how I read your comments I have the following diffs which 
removes the doReset conditions, and removes the PinDel calls as a
result of them. Theoretically now nothing calls the PinDel API and
while I suppose I could remove it, once the hot remove an IOThread
code is added, I'd need it back.

With respect to the pin to the domain cpuset - this API takes a new
cpumap so I'm not sure I understand under what condition would thus
use the domain's cpuset.  Maybe I read your comment too literally

John


diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e3d0acf..74440f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5773,7 +5773,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
 virCapsPtr caps = NULL;
 virDomainDefPtr persistentDef = NULL;
 virBitmapPtr pcpumap = NULL;
-bool doReset = false;
 qemuDomainObjPrivatePtr priv;
 virDomainVcpuPinDefPtr *newIOThreadsPin = NULL;
 size_t newIOThreadsPinNum = 0;
@@ -5823,12 +5822,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
 goto endjob;
 }
 
-/* pinning to all physical cpus means resetting,
- * so check if we can reset setting.
- */
-if (virBitmapIsAllSet(pcpumap))
-doReset = true;
-
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 
 if (priv-iothreadpids == NULL) {
@@ -5891,17 +5884,13 @@ qemuDomainPinIOThread(virDomainPtr dom,
 }
 }
 
-if (doReset) {
-virDomainIOThreadsPinDel(vm-def, iothread_id);
-} else {
-if (vm-def-cputune.iothreadspin)
-virDomainVcpuPinDefArrayFree(vm-def-cputune.iothreadspin,
- vm-def-cputune.niothreadspin);
+if (vm-def-cputune.iothreadspin)
+virDomainVcpuPinDefArrayFree(vm-def-cputune.iothreadspin,
+ vm-def-cputune.niothreadspin);
 
-vm-def-cputune.iothreadspin = newIOThreadsPin;
-vm-def-cputune.niothreadspin = newIOThreadsPinNum;
-newIOThreadsPin = NULL;
-}
+vm-def-cputune.iothreadspin = newIOThreadsPin;
+vm-def-cputune.niothreadspin = newIOThreadsPinNum;
+newIOThreadsPin = NULL;
  if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
 goto endjob;
@@ -5930,24 +5919,20 @@ qemuDomainPinIOThread(virDomainPtr dom,
 goto endjob;
 }
 
-if (doReset) {
-virDomainIOThreadsPinDel(persistentDef, iothread_id);
-} else {
-if (!persistentDef-cputune.iothreadspin) {
-if (VIR_ALLOC(persistentDef-cputune.iothreadspin)  0)
-goto endjob;
-persistentDef-cputune.niothreadspin = 0;
-}
-if 

Re: [libvirt] [libvirt-test-API][PATCH v2 2/2] Add test cases for linux_domain

[hujianwei]

On 12/03/15 11:11, Jincheng Miao wrote:

Signed-off-by: Jincheng Miao jm...@redhat.com
---
  cases/linux_domain.conf |   30 ++
  1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 515858a..f3f714f 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -139,6 +139,36 @@ domain:cpu_affinity
  vcpu
  $defaultvcpu
  
+domain:guest_time

+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+
+domain:set_guest_time
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+seconds
+1234567
+nseconds
+0
+
+domain:set_guest_time
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+flags
+sync
+
In the third block, the seconds is required parameter, please add it 
into conf, but when setting flags=1,

API didn't used it, please add comment for this.

  domain:destroy
  guestname
  $defaultname


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

Re: [libvirt] [libvirt-test-API][PATCH 1/2] Add guest setTime and getTime testing

[hujianwei]

On 10/03/15 17:29, Jincheng Miao wrote:

Signed-off-by: Jincheng Miao jm...@redhat.com
---
  repos/domain/guest_time.py |   98 
  repos/domain/set_guest_time.py |  122 
  2 files changed, 220 insertions(+), 0 deletions(-)
  create mode 100644 repos/domain/guest_time.py
  create mode 100644 repos/domain/set_guest_time.py

diff --git a/repos/domain/guest_time.py b/repos/domain/guest_time.py
new file mode 100644
index 000..4111e99
--- /dev/null
+++ b/repos/domain/guest_time.py
@@ -0,0 +1,98 @@
+#!/usr/bin/evn python
+# To test guest time
+
+import time
+import libxml2
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+from utils import utils
+
+required_params = ('guestname', 'username', 'userpassword')
+optional_params = {}
+
+GET_TIME = 'date +%s'
+DELTA = 3
+
+def get_guest_mac(dom):
+ get guest's MAC address by parsing XML
+
+doc = libxml2.parseDoc(dom.XMLDesc())
+cont=doc.xpathNewContext()
+macs = cont.xpathEval(/domain/devices/interface/mac/@address)
+if macs == None:
+return None
+mac = macs[0].content
+return mac
+
+def check_guest_status(domobj):
+ check guest current status
+
+state = domobj.info()[0]
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+state == libvirt.VIR_DOMAIN_SHUTDOWN:
+return False
+else:
+return True
+
+def check_guest_time(t1, t2):
+ check result, the acceptable error within delta
+
+diff = abs(t1 - t2)
+if diff  DELTA:
+return False
+else:
+return True
+
+def guest_time(params):
+ test guest time
+
+logger = params['logger']
+guestname = params['guestname']
+username = params['username']
+userpassword = params['userpassword']
+
+conn = sharedmod.libvirtobj['conn']
+
+domobj = conn.lookupByName(guestname)
+
+# Check domain status
+if check_guest_status(domobj):
+pass
+else:
+domobj.create()
+time.sleep(90)
+
+# get guest MAC
+mac = get_guest_mac(domobj)
+if mac == None:
+logger.error(Failed to get guest's MAC address)
+return 1
+else:
+logger.info(guest's MAC is %s % mac)
+
+ipaddr = utils.mac_to_ip(mac, 180)
+if mac == None:
+logger.error(Failed to get guest's IP address)
+return 1
+else:
+logger.info(guest's IP is %s % ipaddr)
+
+try:
+t1 = utils.remote_exec(ipaddr, username, userpassword, GET_TIME)
+
+t2 = domobj.getTime()['seconds']
+except libvirtError, e:
+logger.error(API error message: %s, error code is %s \
+ % (e.message, e.get_error_code()))
+return 1
+
+if check_guest_time(long(t1), t2):
+logger.info(checking guest time: %s % t2)
+else:
+logger.error(checking guest time: failed)
+return 1
+
+return 0
\ No newline at end of file
diff --git a/repos/domain/set_guest_time.py b/repos/domain/set_guest_time.py
new file mode 100644
index 000..2aee8a4
--- /dev/null
+++ b/repos/domain/set_guest_time.py
@@ -0,0 +1,122 @@
+#!/usr/bin/evn python
+# To test setting guest time
+
+import time
+import libxml2
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+from utils import utils
+
+required_params = ('guestname', 'username', 'userpassword')
+optional_params = {'flags' : 0}
+
+GET_TIME = 'date +%s'
+DELTA = 30
+
+def parse_flags(flags):
+ parse flags
+
+if flags == 'sync':
+return 1
+elif flags == 0:
+return 0
+else:
+return -1
+
+def get_guest_mac(dom):
+ get guest's MAC address by parsing XML
+
+doc = libxml2.parseDoc(dom.XMLDesc())
+cont=doc.xpathNewContext()
+macs = cont.xpathEval(/domain/devices/interface/mac/@address)
+if macs == None:
+return None
+mac = macs[0].content
+return mac
+
+def check_guest_status(domobj):
+ check guest current status
+
+state = domobj.info()[0]
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+state == libvirt.VIR_DOMAIN_SHUTDOWN:
+return False
+else:
+return True
+
+def check_guest_time(t1, t2):
+ check result, the acceptable error within delta
+
+diff = abs(t1 - t2)
+if diff  DELTA:
+return False
+else:
+return True
+
+def set_guest_time(params):
+ test setting guest time
+
+logger = params['logger']
+guestname = params['guestname']
+username = params['username']
+userpassword = params['userpassword']
+f = params.get('flags', 0)
+flags = parse_flags(f)
+
+if flags == -1:
+logger.error(unrecongnized flags: %s % f)
+return 1
+
+conn = sharedmod.libvirtobj['conn']
+
+domobj = conn.lookupByName(guestname)
+
+# Check domain status
+if check_guest_status(domobj):
+pass
+else:
+domobj.create()
+

Re: [libvirt] [libvirt-test-API][PATCH v2 0/2] Add test cases for setTime and getTime

[hujianwei]

On 12/03/15 11:11, Jincheng Miao wrote:

create some test cases for libvirt-python API setTime and getTime

v2:
   user should pass 'seconds' to set_guest_time, and 'nseconds' is optional.

Jincheng Miao (2):
   Add guest setTime and getTime testing
   Add test cases for linux_domain

  cases/linux_domain.conf|   30 ++
  repos/domain/guest_time.py |   98 
  repos/domain/set_guest_time.py |  122 
  3 files changed, 250 insertions(+), 0 deletions(-)
  create mode 100644 repos/domain/guest_time.py
  create mode 100644 repos/domain/set_guest_time.py


ACK

Please see my comments in the below patch mail.
[libvirt] [libvirt-test-API][PATCH v2 2/2] Add test cases for linux_domain

BR,
Jianwei

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

Re: [libvirt] [libvirt-test-API][PATCH] Rewrite case for listAllInterfaces() API

[hongming]

ACK and Pushed

Thanks
Hongming
On 01/08/2015 11:12 PM, jiahu wrote:

Using actual python API to validate test case, rather than use
virsh iface-* command lines.
---
  cases/basic_interface.conf|  12 ++
  repos/interface/iface_list.py | 299 --
  2 files changed, 99 insertions(+), 212 deletions(-)

diff --git a/cases/basic_interface.conf b/cases/basic_interface.conf
index e2125bb..43e37e8 100644
--- a/cases/basic_interface.conf
+++ b/cases/basic_interface.conf
@@ -15,3 +15,15 @@ interface:define
  interface:create
  ifacename
  $testnic
+
+interface:iface_list
+flags
+all
+
+interface:iface_list
+flags
+active
+
+interface:iface_list
+flags
+inactive
diff --git a/repos/interface/iface_list.py b/repos/interface/iface_list.py
index 49f0c05..85f9df9 100644
--- a/repos/interface/iface_list.py
+++ b/repos/interface/iface_list.py
@@ -1,65 +1,26 @@
  #!/usr/bin/env python
+# test listAllInterfaces() API
  
  import os

-import sys
-import re
-import commands
+import libvirt
  
-required_params = ('ifaceopt',)

-optional_params = {}
-
-VIRSH_QUIET_IFACE_LIST = virsh --quiet iface-list %s | awk '{print $%s}'
-NETWORK_CONFIG = /etc/sysconfig/network-scripts/
-IFCONFIG_DRIVER = ifconfig %s | sed 's/[ \t].*//;/^$/d'
-GET_MAC = ip link show %s |sed -n '2p'| awk '{print $2}'
-VIRSH_IFACE_LIST = virsh iface-list %s
-
-names = []
-state = []
-macs = []
-
-def get_option_list(params):
-return options we need to test
-
-logger = params['logger']
-option_list=[]
+from libvirt import libvirtError
+from src import sharedmod
+from utils import utils
  
-value = params['ifaceopt']
  
-if value == 'all':

-option_list = [' ', '--all', '--inactive']
-elif value == '--all' or value == '--inactive':
-option_list.append(value)
-else:
-logger.error(value %s is not supported % value)
-return 1, option_list
+required_params = ('flags',)
+optional_params = {}
  
-return 0, option_list

+NETWORK_CONFIG = /etc/sysconfig/network-scripts/
+IFCONFIG_DRIVER = ifconfig %s | sed 's/[ \t].*//;/^$/d'| cut -d \:\ -f -1
  
-def get_output(command, logger):

-execute shell command
+def get_inteface_list_from_ifcfg(logger):
+
+   return host interface list from ifcfg-*
  
-status, ret = commands.getstatusoutput(command)
-if status:
-logger.error(executing + \ +  command  + \ +  failed)
-logger.error(ret)
-return status, ret
-
-def get_interface_list(option, logger):
- return active host interface list 
-interface_list = []
-status, interface_str = get_output(IFCONFIG_DRIVER % option, logger)
-if not status:
-interface_list = interface_str.split('\n')
-return interface_list
-else:
-logger.error(\ + IFCONFIG_DRIVER % option + \ + error)
-logger.error(interface_str)
-return interface_list
-
-def check_ifacename(names, option, logger):
- verify the validity of output data 
  ifcfg_files = []
+nic_names = []
  for f in os.listdir(NETWORK_CONFIG):
  if f.startswith(ifcfg-):
  f_path = os.path.join(NETWORK_CONFIG, f)
@@ -67,18 +28,6 @@ def check_ifacename(names, option, logger):
  ifcfg_files.append(f_path)
  else:
  logger.warn(%s is not a regular file % f_path)
-
-interface_active = get_interface_list('', logger)
-logger.debug(list of active host interface: %s % interface_active)
-if interface_active == None:
-return 1
-
-interface_all = get_interface_list('-a', logger)
-logger.debug(list of all host interface: %s % interface_all)
-if interface_all == None:
-return 1
-
-
  for ifcfg_file in ifcfg_files:
  fp = open(ifcfg_file, 'r')
  fp.seek(0,0)
@@ -87,165 +36,91 @@ def check_ifacename(names, option, logger):
  device_str = eachLine.rstrip()
  nic_string = device_str.split(=)[1]
  if nic_string.startswith(\):
-nic_name = nic_string[1:-1]
+nic_names = nic_string[1:-1]
  else:
-nic_name = nic_string
+nic_names.append(nic_string)
  break
-
  fp.close()
+return list(set(nic_names))
  
-if option == ' ':

-if nic_name not in interface_active:
-continue
-else:
-if nic_name in names:
-logger.info(it contains interface %s in %s % (nic_name, 
ifcfg_file))
-else:
-logger.error(interface %s in %s couldn't \n\
-  be in the output of virsh iface-list with option 
%s % \
-  (nic_name, ifcfg_file, option))
-return 1
-elif option == '--all':
-if nic_name in names:
-

Re: [libvirt] [PATCH v3 12/15] bridge_driver: Drop networkDriverLock() from everywhere

[Peter Krempa]

On Tue, Mar 10, 2015 at 17:45:18 +0100, Michal Privoznik wrote:
 Now that we have fine grained locks, there's no need to
 lock the whole driver. We can rely on self-locking APIs.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/network/bridge_driver.c  | 71 
 
  src/network/bridge_driver_platform.h |  2 -
  tests/objectlocking.ml   |  2 -
  3 files changed, 75 deletions(-)
 
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index 5ef9910..c6957c3 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -88,16 +88,6 @@ VIR_LOG_INIT(network.bridge_driver);
  
  static virNetworkDriverStatePtr driver;
  
 -
 -static void networkDriverLock(void)
 -{
 -virMutexLock(driver-lock);
 -}
 -static void networkDriverUnlock(void)
 -{
 -virMutexUnlock(driver-lock);
 -}
 -
  static int networkStateCleanup(void);
  
  static int networkStartNetwork(virNetworkObjPtr network);
 @@ -129,9 +119,7 @@ networkObjFromNetwork(virNetworkPtr net)
  virNetworkObjPtr network;
  char uuidstr[VIR_UUID_STRING_BUFLEN];
  
 -networkDriverLock();
  network = virNetworkObjFindByUUID(driver-networks, net-uuid);
 -networkDriverUnlock();
  
  if (!network) {
  virUUIDFormat(net-uuid, uuidstr);
 @@ -557,12 +545,6 @@ networkStateInitialize(bool privileged,
  if (VIR_ALLOC(driver)  0)
  goto error;
  
 -if (virMutexInit(driver-lock)  0) {
 -VIR_FREE(driver);
 -goto error;
 -}
 -networkDriverLock();
 -
  /* configuration/state paths are one of
   * ~/.config/libvirt/... (session/unprivileged)
   * /etc/libvirt/...  /var/(run|lib)/libvirt/... (system/privileged).
 @@ -648,8 +630,6 @@ networkStateInitialize(bool privileged,
  
  driver-networkEventState = virObjectEventStateNew();
  
 -networkDriverUnlock();
 -
  #ifdef HAVE_FIREWALLD
  if (!(sysbus = virDBusGetSystemBus())) {
  virErrorPtr err = virGetLastError();
 @@ -683,8 +663,6 @@ networkStateInitialize(bool privileged,
  return ret;
  
   error:
 -if (driver)
 -networkDriverUnlock();
  networkStateCleanup();
  goto cleanup;
  }
 @@ -700,11 +678,9 @@ networkStateAutoStart(void)
  if (!driver)
  return;
  
 -networkDriverLock();
  virNetworkObjListForEach(driver-networks,
   networkAutostartConfig,

Although it's not obvious as the @driver isn't passed explicitly this
internally calls networkStartDhcpDaemon which accesses
@driver-dnsmasqStateDir which isn't immutable.

Having the access to @driver hidden by the access to a global variable
is really sneaky. I'd prefer if we'd pass it explicitly in this case.


   NULL);
 -networkDriverUnlock();
  }
  
  /**
 @@ -719,7 +695,6 @@ networkStateReload(void)
  if (!driver)
  return 0;
  
 -networkDriverLock();
  virNetworkLoadAllState(driver-networks,
 driver-stateDir);
  virNetworkLoadAllConfigs(driver-networks,
 @@ -730,7 +705,6 @@ networkStateReload(void)
  virNetworkObjListForEach(driver-networks,
   networkAutostartConfig,

same problem as above

   NULL);
 -networkDriverUnlock();
  return 0;
  }
  
 @@ -746,8 +720,6 @@ networkStateCleanup(void)
  if (!driver)
  return -1;
  
 -networkDriverLock();
 -
  virObjectEventStateFree(driver-networkEventState);
  
  /* free inactive networks */
 @@ -762,9 +734,6 @@ networkStateCleanup(void)
  
  virObjectUnref(driver-dnsmasqCaps);
  
 -networkDriverUnlock();
 -virMutexDestroy(driver-lock);
 -
  VIR_FREE(driver);
  
  return 0;
 @@ -2478,9 +2447,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr 
 conn,
  virNetworkObjPtr network;
  virNetworkPtr ret = NULL;
  
 -networkDriverLock();
  network = virNetworkObjFindByUUID(driver-networks, uuid);
 -networkDriverUnlock();
  if (!network) {
  char uuidstr[VIR_UUID_STRING_BUFLEN];
  virUUIDFormat(uuid, uuidstr);
 @@ -2506,9 +2473,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr 
 conn,
  virNetworkObjPtr network;
  virNetworkPtr ret = NULL;
  
 -networkDriverLock();
  network = virNetworkObjFindByName(driver-networks, name);
 -networkDriverUnlock();
  if (!network) {
  virReportError(VIR_ERR_NO_NETWORK,
 _(no network with matching name '%s'), name);
 @@ -2532,12 +2497,10 @@ static int networkConnectNumOfNetworks(virConnectPtr 
 conn)
  if (virConnectNumOfNetworksEnsureACL(conn)  0)
  return -1;
  
 -networkDriverLock();
  nactive = virNetworkObjListNumOfNetworks(driver-networks,
   true,
   virConnectNumOfNetworksCheckACL,
   

Re: [libvirt] [PATCH] qemu: read backing chain names from qemu

[Daniel P. Berrange]

On Wed, Mar 11, 2015 at 03:03:00PM -0600, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
 after a series of disk snapshots into existing destination images,
 followed by active commits of the top image, it is possible for
 qemu 2.2 and earlier to end up tracking a different name for the
 image than what it would have had when opening the chain afresh.
 That is, when starting with the chain 'a - b - c', the name
 associated with 'b' is how it was spelled in the metadata of 'c',
 but when starting with 'a', taking two snapshots into 'a - b - c',
 then committing 'c' back into 'b', the name associated with 'b' is
 now the name used when taking the first snapshot.
 
 Sadly, older qemu doesn't know how to treat different spellings of
 the same filename as identical files (it uses strcmp() instead of
 checking for the same inode), which means libvirt's attempt to
 commit an image using solely the names learned from qcow2 metadata
 fails with a cryptic:
 
 error: internal error: unable to execute QEMU command 'block-commit': Top 
 image file /tmp/images/c/../b/b not found
 
 even though the file exists.  Trying to teach libvirt the rules on
 which name qemu will expect is not worth the effort (besides, we'd
 have to remember it across libvirtd restarts, and track whether a
 file was opened via metadata or via snapshot creation for a given
 qemu process); it is easier to just always directly ask qemu what
 string it expects to see in the first place.

If you're going to ask QEMU for the names, then libvirt needs to
validate that the name it gets back resolves to the same inode
as the name it originally had. We cannot trust QEMU to tell the
truth and cannot let it trick us into relabelling the wrong files
by giving us the name of something completely different.


  qemuDomainObjEnterMonitor(driver, vm);
 -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
 -  speed, mode, async);
 +if (baseSource)
 +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
 + baseSource);

So this needs to valid basePath vs baseSource inodes.


 @@ -16739,9 +16734,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
  disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
  }
  qemuDomainObjEnterMonitor(driver, vm);
 -ret = qemuMonitorBlockCommit(priv-mon, device,
 - topPath, basePath, backingPath,
 - speed);
 +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
 + baseSource);

And here

 +topPath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
 +topSource);

And here


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v5 0/5] Introduce display of IOThreads Information

[John Ferlan]

On 03/06/2015 09:05 AM, John Ferlan wrote:
 Well technically this is the pinning API's, but keeping the subject the
 same for continuity sake.
 
 v4: http://www.redhat.com/archives/libvir-list/2015-March/msg00262.html
 
 Specifically replacing patches 5-9
 
 Changes over v4 - remove the virDomainGetIOThreadPin related code leaving
 only the virDomainPinIOThread code.
 
 Will address jtomko's comments from 3/9 in a separate followup patch
 
 John Ferlan (5):
   Implement public API for virDomainPinIOThread
   remote: Implement the plumbing for virDomainPinIOThread
   domain: Introduce virDomainIOThreadsPin{Add|Del}
   qemu: Add support to pin IOThreads to specific CPU
   virsh: Add iothreadpin command
 
  include/libvirt/libvirt-domain.h |  14 +++
  src/conf/domain_conf.c   |  64 
  src/conf/domain_conf.h   |  10 ++
  src/driver-hypervisor.h  |   8 ++
  src/libvirt-domain.c |  76 ++
  src/libvirt_private.syms |   2 +
  src/libvirt_public.syms  |   1 +
  src/qemu/qemu_driver.c   | 221 
 +++
  src/remote/remote_driver.c   |   1 +
  src/remote/remote_protocol.x |  17 ++-
  src/remote_protocol-structs  |  10 ++
  src/rpc/gendispatch.pl   |   1 +
  tools/virsh-domain.c | 106 +++
  tools/virsh.pod  |  26 +
  14 files changed, 556 insertions(+), 1 deletion(-)
 

Merged in changes from patch4 and pushed the series

Thanks for the reviews -

John

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

[libvirt] [PATCH] qemu: read backing chain names from qemu

[Eric Blake]

https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
after a series of disk snapshots into existing destination images,
followed by active commits of the top image, it is possible for
qemu 2.2 and earlier to end up tracking a different name for the
image than what it would have had when opening the chain afresh.
That is, when starting with the chain 'a - b - c', the name
associated with 'b' is how it was spelled in the metadata of 'c',
but when starting with 'a', taking two snapshots into 'a - b - c',
then committing 'c' back into 'b', the name associated with 'b' is
now the name used when taking the first snapshot.

Sadly, older qemu doesn't know how to treat different spellings of
the same filename as identical files (it uses strcmp() instead of
checking for the same inode), which means libvirt's attempt to
commit an image using solely the names learned from qcow2 metadata
fails with a cryptic:

error: internal error: unable to execute QEMU command 'block-commit': Top image 
file /tmp/images/c/../b/b not found

even though the file exists.  Trying to teach libvirt the rules on
which name qemu will expect is not worth the effort (besides, we'd
have to remember it across libvirtd restarts, and track whether a
file was opened via metadata or via snapshot creation for a given
qemu process); it is easier to just always directly ask qemu what
string it expects to see in the first place.

* src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New
prototype.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup)
(qemuMonitorJSONDiskNameLookupOne): Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit)
(qemuDomainBlockJobImpl): Use it.

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

Adam, since you reported the issue, let me know if you need a
scratch binary to test this patch on.  Jeff, thanks for the idea
on how to solve the problem without touching qemu (of course, it
would still be nice to teach qemu to respect inode equivalence
rather than relying on streq, and even nicer for libvirt to use
node names instead of file names, but those can be later patches
without holding up VDSM now).

 src/qemu/qemu_driver.c   | 28 +++---
 src/qemu/qemu_monitor.c  | 20 +-
 src/qemu/qemu_monitor.h  |  8 +++-
 src/qemu/qemu_monitor_json.c | 87 +++-
 src/qemu/qemu_monitor_json.h |  9 -
 5 files changed, 134 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 51b30b7..e540001 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15932,9 +15932,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 goto endjob;

 if (baseSource) {
-if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
-goto endjob;
-
 if (flags  VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
 if (!virQEMUCapsGet(priv-qemuCaps, 
QEMU_CAPS_CHANGE_BACKING_FILE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -15972,8 +15969,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 }

 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
-  speed, mode, async);
+if (baseSource)
+basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+ baseSource);
+if (!baseSource || basePath)
+ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
+  speed, mode, async);
 if (qemuDomainObjExitMonitor(driver, vm)  0)
 ret = -1;
 if (ret  0) {
@@ -16703,12 +16704,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE)  0))
 goto endjob;

-if (qemuGetDriveSourceString(topSource, NULL, topPath)  0)
-goto endjob;
-
-if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
-goto endjob;
-
 if (flags  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE 
 topSource != disk-src) {
 if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
@@ -16739,9 +16734,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
 disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
 }
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockCommit(priv-mon, device,
- topPath, basePath, backingPath,
- speed);
+basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+ baseSource);
+topPath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+topSource);
+if (basePath  topPath)
+ret = qemuMonitorBlockCommit(priv-mon, device,
+   

Re: [libvirt] [PATCHv3.5 0/4] Automaticaly fill memory element for NUMA enabled guests

[John Ferlan]

On 03/06/2015 09:40 AM, Peter Krempa wrote:
 Pavel's series changed few same places thus the previous version no longer 
 applies.
 
 Peter Krempa (4):
   conf: Replace access to def-mem.max_balloon with accessor functions
   qemu: command: Add helper to align memory sizes
   conf: Automatically use NUMA memory size in case NUMA is enabled
   conf: Make specifying memory optional
 
  docs/schemas/domaincommon.rng  | 18 ++---
  src/conf/domain_conf.c | 81 
 +++---
  src/conf/domain_conf.h |  4 ++
  src/hyperv/hyperv_driver.c |  2 +-
  src/libvirt_private.syms   |  3 +
  src/libxl/libxl_conf.c |  2 +-
  src/libxl/libxl_driver.c   |  8 +--
  src/lxc/lxc_cgroup.c   |  2 +-
  src/lxc/lxc_driver.c   | 12 ++--
  src/lxc/lxc_fuse.c |  4 +-
  src/lxc/lxc_native.c   |  4 +-
  src/openvz/openvz_driver.c |  2 +-
  src/parallels/parallels_driver.c   |  2 +-
  src/parallels/parallels_sdk.c  | 12 ++--
  src/phyp/phyp_driver.c | 11 +--
  src/qemu/qemu_command.c| 23 +++---
  src/qemu/qemu_domain.c | 21 ++
  src/qemu/qemu_domain.h |  2 +
  src/qemu/qemu_driver.c | 21 +++---
  src/qemu/qemu_hotplug.c|  8 ++-
  src/qemu/qemu_process.c|  2 +-
  src/test/test_driver.c |  8 +--
  src/uml/uml_driver.c   |  8 +--
  src/vbox/vbox_common.c |  4 +-
  src/vmware/vmware_driver.c |  2 +-
  src/vmx/vmx.c  | 12 ++--
  src/xen/xm_internal.c  | 14 ++--
  src/xenapi/xenapi_driver.c |  2 +-
  src/xenapi/xenapi_utils.c  |  4 +-
  src/xenconfig/xen_common.c |  8 ++-
  src/xenconfig/xen_sxpr.c   |  9 +--
  .../qemuxml2argv-cpu-numa-no-memory-element.args   |  7 ++
  .../qemuxml2argv-cpu-numa-no-memory-element.xml| 24 +++
  .../qemuxml2argv-minimal-no-memory.xml | 25 +++
  .../qemuxml2argv-numatune-memnode.args |  2 +-
  tests/qemuxml2argvtest.c   |  2 +
  .../qemuxml2xmlout-cpu-numa-no-memory-element.xml  | 28 
  tests/qemuxml2xmltest.c|  1 +
  38 files changed, 299 insertions(+), 105 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-no-memory-element.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-no-memory-element.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-no-memory.xml
  create mode 100644 
 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-no-memory-element.xml
 

Since this is the order of your repo on git://pipo.sk/pipo/libvirt.git,
I'll start with these.

Couple of general comments...

 - I think the new function names are better

 - I think one can tell that the KiB or MiB was realized at some
point in time after KB and MB - as there's stray comments and
variables using the (I guess) now incorrect terminology.  Not that I'm
asking for them to be changed, just an observation.

 - Whether they snuck in after you started this - there are a few direct
references to mem.max_balloon in the bhyve_command.c.  Should they be
replaced by the new accessors?

 - Because it became apparent in patch 4... In virDomainDefParseXML,
just to be complete shouldn't the:

/* Extract domain memory */
if (virDomainParseMemory(./memory[1], NULL, ctxt,
 def-mem.max_balloon, false, true)  0)

be replaced with something like:

unsigned long long memory_value;
...
/* Extract domain memory */
if (virDomainParseMemory(./memory[1], NULL, ctxt,
 memory_value, false, true)  0) {
...
}
virDomainDefSetMemoryInitial(def, memory_value);


Although it seems like overkill - it's just making sure no one tries to
cut-copy-paste in the future.

 - With having virDomainDefGetMemoryInitial understand NUMA vs Balloon,
should virDomainDefSetMemoryInitial check and not set max_balloon?
Theoretically if memory wasn't filled, then we're possible setting
something we shouldn't which could then be output when the domain is
saved, right?

ACK in principal - perhaps give it a day to see if anyone else has
comments or issues...

John

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

Re: [libvirt] [PATCH] qemu: read backing chain names from qemu

[Adam Litke]

On 11/03/15 15:03 -0600, Eric Blake wrote:

Adam, since you reported the issue, let me know if you need a
scratch binary to test this patch on.  Jeff, thanks for the idea
on how to solve the problem without touching qemu (of course, it
would still be nice to teach qemu to respect inode equivalence
rather than relying on streq, and even nicer for libvirt to use
node names instead of file names, but those can be later patches
without holding up VDSM now).


Thanks Eric.  I am in the process of doing a crude port to RHEL-7.1
(libvirt-1.2.8-16) in order to verify it, but a proper build would
probably be best (when you have some time).

--
Adam Litke

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

[libvirt] [PATCH v4 1/7] bridge_driver: Don't access global driver randomly

[Michal Privoznik]

Well, network driver code has the driver accessible as a global
variable. This makes any rework hard, as it's unclear where the
variable is accessed and/or modified. Lets just pass the driver
as a parameter to all functions where needed.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver.c | 430 +---
 1 file changed, 247 insertions(+), 183 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ed05ace..6ff4539 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -86,34 +86,46 @@
 
 VIR_LOG_INIT(network.bridge_driver);
 
-static virNetworkDriverStatePtr driver;
+static virNetworkDriverStatePtr network_driver;
 
+static virNetworkDriverStatePtr
+networkGetDriver(void)
+{
+/* Maybe one day we can store @network_driver in the
+ * connection object, but until then, it's just a global
+ * variable which is returned. */
+return network_driver;
+}
 
-static void networkDriverLock(void)
+static void networkDriverLock(virNetworkDriverStatePtr driver)
 {
 virMutexLock(driver-lock);
 }
-static void networkDriverUnlock(void)
+static void networkDriverUnlock(virNetworkDriverStatePtr driver)
 {
 virMutexUnlock(driver-lock);
 }
 
 static int networkStateCleanup(void);
 
-static int networkStartNetwork(virNetworkObjPtr network);
+static int networkStartNetwork(virNetworkDriverStatePtr driver,
+   virNetworkObjPtr network);
 
-static int networkShutdownNetwork(virNetworkObjPtr network);
+static int networkShutdownNetwork(virNetworkDriverStatePtr driver,
+  virNetworkObjPtr network);
 
-static int networkStartNetworkVirtual(virNetworkObjPtr network);
+static int networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
+  virNetworkObjPtr network);
 
-static int networkShutdownNetworkVirtual(virNetworkObjPtr network);
+static int networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver,
+ virNetworkObjPtr network);
 
 static int networkStartNetworkExternal(virNetworkObjPtr network);
 
 static int networkShutdownNetworkExternal(virNetworkObjPtr network);
 
-static void networkReloadFirewallRules(void);
-static void networkRefreshDaemons(void);
+static void networkReloadFirewallRules(virNetworkDriverStatePtr driver);
+static void networkRefreshDaemons(virNetworkDriverStatePtr driver);
 
 static int networkPlugBandwidth(virNetworkObjPtr net,
 virDomainNetDefPtr iface);
@@ -126,12 +138,13 @@ static void networkNetworkObjTaint(virNetworkObjPtr net,
 static virNetworkObjPtr
 networkObjFromNetwork(virNetworkPtr net)
 {
+virNetworkDriverStatePtr driver = networkGetDriver();
 virNetworkObjPtr network;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-networkDriverLock();
+networkDriverLock(driver);
 network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-networkDriverUnlock();
+networkDriverUnlock(driver);
 
 if (!network) {
 virUUIDFormat(net-uuid, uuidstr);
@@ -200,7 +213,8 @@ networkRunHook(virNetworkObjPtr network,
 }
 
 static char *
-networkDnsmasqLeaseFileNameDefault(const char *netname)
+networkDnsmasqLeaseFileNameDefault(virNetworkDriverStatePtr driver,
+   const char *netname)
 {
 char *leasefile;
 
@@ -210,7 +224,8 @@ networkDnsmasqLeaseFileNameDefault(const char *netname)
 }
 
 static char *
-networkDnsmasqLeaseFileNameCustom(const char *bridge)
+networkDnsmasqLeaseFileNameCustom(virNetworkDriverStatePtr driver,
+  const char *bridge)
 {
 char *leasefile;
 
@@ -220,7 +235,8 @@ networkDnsmasqLeaseFileNameCustom(const char *bridge)
 }
 
 static char *
-networkDnsmasqConfigFileName(const char *netname)
+networkDnsmasqConfigFileName(virNetworkDriverStatePtr driver,
+ const char *netname)
 {
 char *conffile;
 
@@ -240,7 +256,8 @@ networkRadvdPidfileBasename(const char *netname)
 }
 
 static char *
-networkRadvdConfigFileName(const char *netname)
+networkRadvdConfigFileName(virNetworkDriverStatePtr driver,
+   const char *netname)
 {
 char *configfile;
 
@@ -251,7 +268,8 @@ networkRadvdConfigFileName(const char *netname)
 
 /* do needed cleanup steps and remove the network from the list */
 static int
-networkRemoveInactive(virNetworkObjPtr net)
+networkRemoveInactive(virNetworkDriverStatePtr driver,
+  virNetworkObjPtr net)
 {
 char *leasefile = NULL;
 char *customleasefile = NULL;
@@ -270,23 +288,22 @@ networkRemoveInactive(virNetworkObjPtr net)
 goto cleanup;
 }
 
-if (!(leasefile = networkDnsmasqLeaseFileNameDefault(def-name)))
+if (!(leasefile = networkDnsmasqLeaseFileNameDefault(driver, def-name)))
 goto cleanup;
 
-if (!(customleasefile = 

[libvirt] [PATCH v4 7/7] bridge_driver: Use more of networkObjFromNetwork

[Michal Privoznik]

Now that the network driver lock is ash heap of history,
we can use more of networkObjFromNetwork().

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver.c | 33 +
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index bff749a..13e1717 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -168,7 +168,6 @@ networkObjFromNetwork(virNetworkPtr net)
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
 network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-
 if (!network) {
 virUUIDFormat(net-uuid, uuidstr);
 virReportError(VIR_ERR_NO_NETWORK,
@@ -3079,12 +3078,8 @@ networkUndefine(virNetworkPtr net)
 bool active = false;
 virObjectEventPtr event = NULL;
 
-network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
-   %s, _(no network with matching uuid));
+if (!(network = networkObjFromNetwork(net)))
 goto cleanup;
-}
 
 if (virNetworkUndefineEnsureACL(net-conn, network-def)  0)
 goto cleanup;
@@ -3145,12 +3140,8 @@ networkUpdate(virNetworkPtr net,
   VIR_NETWORK_UPDATE_AFFECT_CONFIG,
   -1);
 
-network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
-   %s, _(no network with matching uuid));
+if (!(network = networkObjFromNetwork(net)))
 goto cleanup;
-}
 
 if (virNetworkUpdateEnsureACL(net-conn, network-def, flags)  0)
 goto cleanup;
@@ -3300,13 +3291,8 @@ static int networkCreate(virNetworkPtr net)
 int ret = -1;
 virObjectEventPtr event = NULL;
 
-network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
-   %s, _(no network with matching uuid));
+if (!(network = networkObjFromNetwork(net)))
 goto cleanup;
-}
 
 if (virNetworkCreateEnsureACL(net-conn, network-def)  0)
 goto cleanup;
@@ -,13 +3319,8 @@ static int networkDestroy(virNetworkPtr net)
 int ret = -1;
 virObjectEventPtr event = NULL;
 
-network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
-   %s, _(no network with matching uuid));
+if (!(network = networkObjFromNetwork(net)))
 goto cleanup;
-}
 
 if (virNetworkDestroyEnsureACL(net-conn, network-def)  0)
 goto cleanup;
@@ -3451,13 +3432,9 @@ static int networkSetAutostart(virNetworkPtr net,
 char *configFile = NULL, *autostartLink = NULL;
 int ret = -1;
 
-network = virNetworkObjFindByUUID(driver-networks, net-uuid);
 
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
-   %s, _(no network with matching uuid));
+if (!(network = networkObjFromNetwork(net)))
 goto cleanup;
-}
 
 if (virNetworkSetAutostartEnsureACL(net-conn, network-def)  0)
 goto cleanup;
-- 
2.0.5

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

[libvirt] [PATCH v4 3/7] struct _virNetworkDriverState: Annotate items

[Michal Privoznik]

In order to drop network driver lock, lets annotate which
structure items are immutable, which have self-locking
APIs and so on.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver_platform.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/network/bridge_driver_platform.h 
b/src/network/bridge_driver_platform.h
index b7492e6..904e731 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -34,16 +34,23 @@
 struct _virNetworkDriverState {
 virMutex lock;
 
+/* Immutable pointer, self-locking APIs */
 virNetworkObjListPtr networks;
 
+/* Immutable pointers, Immutable objects */
 char *networkConfigDir;
 char *networkAutostartDir;
 char *stateDir;
 char *pidDir;
 char *dnsmasqStateDir;
 char *radvdStateDir;
+
+/* Require lock to get a reference on the object,
+ * lockless access thereafter
+ */
 dnsmasqCapsPtr dnsmasqCaps;
 
+/* Immutable pointer, self-locking APIs */
 virObjectEventStatePtr networkEventState;
 };
 
-- 
2.0.5

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

Re: [libvirt] [PATCH] Clarify the meaning of version in redirdev filters

[Peter Krempa]

On Thu, Mar 12, 2015 at 13:21:40 +0100, Ján Tomko wrote:
 The version attribute in redirdev filters refers to the revision
 of the device, not the version of the USB protocol.
 
 Explicitly state that this is not the USB protocol and remove references
 to those round version numbers that resemble USB protocol versions.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1177237
 ---
  docs/formatdomain.html.in | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

ACK


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

Re: [libvirt] [PATCH] qemu: read backing chain names from qemu

[Daniel P. Berrange]

On Thu, Mar 12, 2015 at 08:09:35AM -0600, Eric Blake wrote:
 On 03/12/2015 03:14 AM, Daniel P. Berrange wrote:
 
  even though the file exists.  Trying to teach libvirt the rules on
  which name qemu will expect is not worth the effort (besides, we'd
  have to remember it across libvirtd restarts, and track whether a
  file was opened via metadata or via snapshot creation for a given
  qemu process); it is easier to just always directly ask qemu what
  string it expects to see in the first place.
  
  If you're going to ask QEMU for the names, then libvirt needs to
  validate that the name it gets back resolves to the same inode
  as the name it originally had. We cannot trust QEMU to tell the
  truth and cannot let it trick us into relabelling the wrong files
  by giving us the name of something completely different.
  
 
 We are NOT relabelling files based on the information.  I agree that if
 we act on a name returned by qemu, then we are vulnerable to mistakes if
 a guest manages to compromise qemu; but I don't think this is one of
 those situations.
 
  
   qemuDomainObjEnterMonitor(driver, vm);
  -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
  -  speed, mode, async);
  +if (baseSource)
  +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
  + baseSource);
  
  So this needs to valid basePath vs baseSource inodes.
 
 The only use of basePath is the string passed right back to qemu, to
 kick off the commit or pull operation.  Since qemu is using strcmp()
 (rather than inode comparison), feeding it the name it just told us will
 make the commit operation work on the correct node.  Everything else
 that libvirt does, such as relabelling files, is done solely on the
 information libvirt has been tracking (and not reliant on the name
 returned by qemu).
 
 But if it would satisfy your paranoia, I can certainly add a
 verification step that the string being returned by qemu resolves to the
 same inode being tracked by libvirt, at least in the case where the
 disk element resolves to a filename rather than a network disk.

I think it would be desirable, because while your current usage
may be safe with these assumptions, if someone refactors this
6 months later they may not realize the security implications
of this code.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [PATCH v4 4/7] bridge_driver: Drop networkDriverLock() from almost everywhere

[Michal Privoznik]

Now that we have fine grained locks, there's no need to
lock the whole driver. We can rely on self-locking APIs.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver.c | 56 -
 1 file changed, 5 insertions(+), 51 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 863eeac..bff749a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -110,7 +110,9 @@ static dnsmasqCapsPtr
 networkGetDnsmasqCaps(virNetworkDriverStatePtr driver)
 {
 dnsmasqCapsPtr ret;
+networkDriverLock(driver);
 ret = virObjectRef(driver-dnsmasqCaps);
+networkDriverUnlock(driver);
 return ret;
 }
 
@@ -122,8 +124,10 @@ networkDnsmasqCapsRefresh(virNetworkDriverStatePtr driver)
 if (!(caps = dnsmasqCapsNewFromBinary(DNSMASQ)))
 return -1;
 
+networkDriverLock(driver);
 virObjectUnref(driver-dnsmasqCaps);
 driver-dnsmasqCaps = caps;
+networkDriverUnlock(driver);
 return 0;
 }
 
@@ -163,9 +167,7 @@ networkObjFromNetwork(virNetworkPtr net)
 virNetworkObjPtr network;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-networkDriverLock(driver);
 network = virNetworkObjFindByUUID(driver-networks, net-uuid);
-networkDriverUnlock(driver);
 
 if (!network) {
 virUUIDFormat(net-uuid, uuidstr);
@@ -676,6 +678,7 @@ networkStateInitialize(bool privileged,
  network_driver-networkAutostartDir)  0)
 goto error;
 
+networkDriverUnlock(network_driver);
 
 /* Update the internal status of all allegedly active
  * networks according to external conditions on the host
@@ -692,8 +695,6 @@ networkStateInitialize(bool privileged,
 
 network_driver-networkEventState = virObjectEventStateNew();
 
-networkDriverUnlock(network_driver);
-
 #ifdef HAVE_FIREWALLD
 if (!(sysbus = virDBusGetSystemBus())) {
 virErrorPtr err = virGetLastError();
@@ -744,11 +745,9 @@ networkStateAutoStart(void)
 if (!network_driver)
 return;
 
-networkDriverLock(network_driver);
 virNetworkObjListForEach(network_driver-networks,
  networkAutostartConfig,
  network_driver);
-networkDriverUnlock(network_driver);
 }
 
 /**
@@ -763,7 +762,6 @@ networkStateReload(void)
 if (!network_driver)
 return 0;
 
-networkDriverLock(network_driver);
 virNetworkLoadAllState(network_driver-networks,
network_driver-stateDir);
 virNetworkLoadAllConfigs(network_driver-networks,
@@ -774,7 +772,6 @@ networkStateReload(void)
 virNetworkObjListForEach(network_driver-networks,
  networkAutostartConfig,
  NULL);
-networkDriverUnlock(network_driver);
 return 0;
 }
 
@@ -790,8 +787,6 @@ networkStateCleanup(void)
 if (!network_driver)
 return -1;
 
-networkDriverLock(network_driver);
-
 virObjectEventStateFree(network_driver-networkEventState);
 
 /* free inactive networks */
@@ -806,7 +801,6 @@ networkStateCleanup(void)
 
 virObjectUnref(network_driver-dnsmasqCaps);
 
-networkDriverUnlock(network_driver);
 virMutexDestroy(network_driver-lock);
 
 VIR_FREE(network_driver);
@@ -2545,9 +2539,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr 
conn,
 virNetworkObjPtr network;
 virNetworkPtr ret = NULL;
 
-networkDriverLock(driver);
 network = virNetworkObjFindByUUID(driver-networks, uuid);
-networkDriverUnlock(driver);
 if (!network) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virUUIDFormat(uuid, uuidstr);
@@ -2574,9 +2566,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr 
conn,
 virNetworkObjPtr network;
 virNetworkPtr ret = NULL;
 
-networkDriverLock(driver);
 network = virNetworkObjFindByName(driver-networks, name);
-networkDriverUnlock(driver);
 if (!network) {
 virReportError(VIR_ERR_NO_NETWORK,
_(no network with matching name '%s'), name);
@@ -2601,12 +2591,10 @@ static int networkConnectNumOfNetworks(virConnectPtr 
conn)
 if (virConnectNumOfNetworksEnsureACL(conn)  0)
 return -1;
 
-networkDriverLock(driver);
 nactive = virNetworkObjListNumOfNetworks(driver-networks,
  true,
  virConnectNumOfNetworksCheckACL,
  conn);
-networkDriverUnlock(driver);
 
 return nactive;
 }
@@ -2621,12 +2609,10 @@ static int networkConnectListNetworks(virConnectPtr 
conn,
 if (virConnectListNetworksEnsureACL(conn)  0)
 return -1;
 
-networkDriverLock(driver);
 got = virNetworkObjListGetNames(driver-networks,
 true, names, nnames,
 virConnectListNetworksCheckACL,

[libvirt] [PATCH v4 2/7] network_driver: Use accessor for dnsmasqCaps

[Michal Privoznik]

This is not an immutable pointer and can change during lifetime.
Therefore, in order to drop network driver lock, we must use an
internal accessor which does not lock the network driver yet, but
it will soon. Now it merely returns an referenced object.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver.c | 42 --
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6ff4539..863eeac 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -106,6 +106,27 @@ static void networkDriverUnlock(virNetworkDriverStatePtr 
driver)
 virMutexUnlock(driver-lock);
 }
 
+static dnsmasqCapsPtr
+networkGetDnsmasqCaps(virNetworkDriverStatePtr driver)
+{
+dnsmasqCapsPtr ret;
+ret = virObjectRef(driver-dnsmasqCaps);
+return ret;
+}
+
+static int
+networkDnsmasqCapsRefresh(virNetworkDriverStatePtr driver)
+{
+dnsmasqCapsPtr caps;
+
+if (!(caps = dnsmasqCapsNewFromBinary(DNSMASQ)))
+return -1;
+
+virObjectUnref(driver-dnsmasqCaps);
+driver-dnsmasqCaps = caps;
+return 0;
+}
+
 static int networkStateCleanup(void);
 
 static int networkStartNetwork(virNetworkDriverStatePtr driver,
@@ -364,6 +385,7 @@ networkUpdateState(virNetworkObjPtr obj,
void *opaque)
 {
 virNetworkDriverStatePtr driver = opaque;
+dnsmasqCapsPtr dnsmasq_caps = networkGetDnsmasqCaps(driver);
 int ret = -1;
 
 virObjectLock(obj);
@@ -408,7 +430,7 @@ networkUpdateState(virNetworkObjPtr obj,
 ignore_value(virPidFileReadIfAlive(driver-pidDir,
obj-def-name,
obj-dnsmasqPid,
-   
dnsmasqCapsGetBinaryPath(driver-dnsmasqCaps)));
+   
dnsmasqCapsGetBinaryPath(dnsmasq_caps)));
 radvdpidbase = networkRadvdPidfileBasename(obj-def-name);
 if (!radvdpidbase)
 goto cleanup;
@@ -422,6 +444,7 @@ networkUpdateState(virNetworkObjPtr obj,
 ret = 0;
  cleanup:
 virObjectUnlock(obj);
+virObjectUnref(dnsmasq_caps);
 return ret;
 }
 
@@ -1284,6 +1307,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
   char *pidfile,
   dnsmasqContext *dctx)
 {
+dnsmasqCapsPtr dnsmasq_caps = networkGetDnsmasqCaps(driver);
 virCommandPtr cmd = NULL;
 int ret = -1;
 char *configfile = NULL;
@@ -1293,7 +1317,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
 network-dnsmasqPid = -1;
 
 if (networkDnsmasqConfContents(network, pidfile, configstr,
-   dctx, driver-dnsmasqCaps)  0)
+   dctx, dnsmasq_caps)  0)
 goto cleanup;
 if (!configstr)
 goto cleanup;
@@ -1316,7 +1340,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
   LIBEXECDIR)))
 goto cleanup;
 
-cmd = virCommandNew(dnsmasqCapsGetBinaryPath(driver-dnsmasqCaps));
+cmd = virCommandNew(dnsmasqCapsGetBinaryPath(dnsmasq_caps));
 virCommandAddArgFormat(cmd, --conf-file=%s, configfile);
 /* Libvirt gains full control of leases database */
 virCommandAddArgFormat(cmd, --leasefile-ro);
@@ -1326,6 +1350,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
 *cmdout = cmd;
 ret = 0;
  cleanup:
+virObjectUnref(dnsmasq_caps);
 VIR_FREE(configfile);
 VIR_FREE(configstr);
 VIR_FREE(leaseshelper_path);
@@ -1369,7 +1394,7 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver,
 if (dctx == NULL)
 goto cleanup;
 
-if (dnsmasqCapsRefresh(driver-dnsmasqCaps, NULL)  0)
+if (networkDnsmasqCapsRefresh(driver)  0)
 goto cleanup;
 
 ret = networkBuildDhcpDaemonCommandLine(driver, network, cmd, pidfile, 
dctx);
@@ -1623,6 +1648,7 @@ static int
 networkStartRadvd(virNetworkDriverStatePtr driver,
   virNetworkObjPtr network)
 {
+dnsmasqCapsPtr dnsmasq_caps = networkGetDnsmasqCaps(driver);
 char *pidfile = NULL;
 char *radvdpidbase = NULL;
 char *configfile = NULL;
@@ -1632,7 +1658,7 @@ networkStartRadvd(virNetworkDriverStatePtr driver,
 network-radvdPid = -1;
 
 /* Is dnsmasq handling RA? */
-if (DNSMASQ_RA_SUPPORT(driver-dnsmasqCaps)) {
+if (DNSMASQ_RA_SUPPORT(dnsmasq_caps)) {
 ret = 0;
 goto cleanup;
 }
@@ -1698,6 +1724,7 @@ networkStartRadvd(virNetworkDriverStatePtr driver,
 
 ret = 0;
  cleanup:
+virObjectUnref(dnsmasq_caps);
 virCommandFree(cmd);
 VIR_FREE(configfile);
 VIR_FREE(radvdpidbase);
@@ -1709,10 +1736,12 @@ static int
 networkRefreshRadvd(virNetworkDriverStatePtr driver,
 virNetworkObjPtr network)
 {
+

[libvirt] [PATCH v4 5/7] test_driver: Drop testDriverLock() from almost everywhere

[Michal Privoznik]

Well, if 'everywhere' is defined as that part of the driver code
that serves virNetwork* APIs. Again, we lower layers already have
their locks, so there's no point doing big lock.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/test/test_driver.c | 56 +-
 1 file changed, 1 insertion(+), 55 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 90af80a..187bd3d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3495,10 +3495,7 @@ static virNetworkPtr 
testNetworkLookupByUUID(virConnectPtr conn,
 virNetworkObjPtr net;
 virNetworkPtr ret = NULL;
 
-testDriverLock(privconn);
 net = virNetworkObjFindByUUID(privconn-networks, uuid);
-testDriverUnlock(privconn);
-
 if (net == NULL) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -3518,10 +3515,7 @@ static virNetworkPtr 
testNetworkLookupByName(virConnectPtr conn,
 virNetworkObjPtr net;
 virNetworkPtr ret = NULL;
 
-testDriverLock(privconn);
 net = virNetworkObjFindByName(privconn-networks, name);
-testDriverUnlock(privconn);
-
 if (net == NULL) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -3540,11 +3534,8 @@ static int testConnectNumOfNetworks(virConnectPtr conn)
 testConnPtr privconn = conn-privateData;
 int numActive;
 
-testDriverLock(privconn);
 numActive = virNetworkObjListNumOfNetworks(privconn-networks,
true, NULL, conn);
-testDriverUnlock(privconn);
-
 return numActive;
 }
 
@@ -3552,11 +3543,8 @@ static int testConnectListNetworks(virConnectPtr conn, 
char **const names, int n
 testConnPtr privconn = conn-privateData;
 int n;
 
-testDriverLock(privconn);
 n = virNetworkObjListGetNames(privconn-networks,
   true, names, nnames, NULL, conn);
-testDriverUnlock(privconn);
-
 return n;
 }
 
@@ -3565,11 +3553,8 @@ static int testConnectNumOfDefinedNetworks(virConnectPtr 
conn)
 testConnPtr privconn = conn-privateData;
 int numInactive;
 
-testDriverLock(privconn);
 numInactive = virNetworkObjListNumOfNetworks(privconn-networks,
  false, NULL, conn);
-testDriverUnlock(privconn);
-
 return numInactive;
 }
 
@@ -3577,11 +3562,8 @@ static int testConnectListDefinedNetworks(virConnectPtr 
conn, char **const names
 testConnPtr privconn = conn-privateData;
 int n;
 
-testDriverLock(privconn);
 n = virNetworkObjListGetNames(privconn-networks,
   false, names, nnames, NULL, conn);
-testDriverUnlock(privconn);
-
 return n;
 }
 
@@ -3591,15 +3573,10 @@ testConnectListAllNetworks(virConnectPtr conn,
unsigned int flags)
 {
 testConnPtr privconn = conn-privateData;
-int ret = -1;
 
 virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
 
-testDriverLock(privconn);
-ret = virNetworkObjListExport(conn, privconn-networks, nets, NULL, flags);
-testDriverUnlock(privconn);
-
-return ret;
+return virNetworkObjListExport(conn, privconn-networks, nets, NULL, 
flags);
 }
 
 static int testNetworkIsActive(virNetworkPtr net)
@@ -3608,9 +3585,7 @@ static int testNetworkIsActive(virNetworkPtr net)
 virNetworkObjPtr obj;
 int ret = -1;
 
-testDriverLock(privconn);
 obj = virNetworkObjFindByUUID(privconn-networks, net-uuid);
-testDriverUnlock(privconn);
 if (!obj) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -3628,9 +3603,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
 virNetworkObjPtr obj;
 int ret = -1;
 
-testDriverLock(privconn);
 obj = virNetworkObjFindByUUID(privconn-networks, net-uuid);
-testDriverUnlock(privconn);
 if (!obj) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -3651,7 +3624,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr 
conn, const char *xml)
 virNetworkPtr ret = NULL;
 virObjectEventPtr event = NULL;
 
-testDriverLock(privconn);
 if ((def = virNetworkDefParseString(xml)) == NULL)
 goto cleanup;
 
@@ -3671,7 +3643,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr 
conn, const char *xml)
 if (event)
 testObjectEventQueue(privconn, event);
 virNetworkObjEndAPI(net);
-testDriverUnlock(privconn);
 return ret;
 }
 
@@ -3684,7 +3655,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, 
const char *xml)
 virNetworkPtr ret = NULL;
 virObjectEventPtr event = NULL;
 
-testDriverLock(privconn);
 if ((def = virNetworkDefParseString(xml)) == NULL)
 goto cleanup;
 
@@ -3703,7 +3673,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, 
const char *xml)
 if (event)
 testObjectEventQueue(privconn, event);
 

[libvirt] [PATCH v4 0/7] Drop network driver lock

[Michal Privoznik]

Hopefully, the last version. Again, some patches are ACKed
already, but I'm sending them again. Not to trash the review
bandwidth, but for reviewer to get better picture.

Michal Privoznik (7):
  bridge_driver: Don't access global driver randomly
  network_driver: Use accessor for dnsmasqCaps
  struct _virNetworkDriverState: Annotate items
  bridge_driver: Drop networkDriverLock() from almost everywhere
  test_driver: Drop testDriverLock() from almost everywhere
  parallels_network: Drop parallelsDriverLock() from everywhere.
  bridge_driver: Use more of networkObjFromNetwork

 src/network/bridge_driver.c  | 467 ++-
 src/network/bridge_driver_platform.h |   7 +
 src/parallels/parallels_network.c|  33 +--
 src/test/test_driver.c   |  56 +
 4 files changed, 255 insertions(+), 308 deletions(-)

-- 
2.0.5

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

[libvirt] [PATCH v4 6/7] parallels_network: Drop parallelsDriverLock() from everywhere.

[Michal Privoznik]

While in previous commits there were some places that relied on
the big lock, in this file there's no such place and the big
driver lock can be dropped completely. Yay!

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/parallels/parallels_network.c | 33 +
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/src/parallels/parallels_network.c 
b/src/parallels/parallels_network.c
index 1bcd2d3..1d3b694 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -349,9 +349,7 @@ int parallelsNetworkClose(virConnectPtr conn)
 if (!privconn)
 return 0;
 
-parallelsDriverLock(privconn);
 virObjectUnref(privconn-networks);
-parallelsDriverUnlock(privconn);
 return 0;
 }
 
@@ -360,11 +358,8 @@ static int parallelsConnectNumOfNetworks(virConnectPtr 
conn)
 int nactive;
 parallelsConnPtr privconn = conn-privateData;
 
-parallelsDriverLock(privconn);
 nactive = virNetworkObjListNumOfNetworks(privconn-networks,
  true, NULL, conn);
-parallelsDriverUnlock(privconn);
-
 return nactive;
 }
 
@@ -375,11 +370,8 @@ static int parallelsConnectListNetworks(virConnectPtr conn,
 parallelsConnPtr privconn = conn-privateData;
 int got;
 
-parallelsDriverLock(privconn);
 got = virNetworkObjListGetNames(privconn-networks,
 true, names, nnames, NULL, conn);
-parallelsDriverUnlock(privconn);
-
 return got;
 }
 
@@ -388,11 +380,8 @@ static int 
parallelsConnectNumOfDefinedNetworks(virConnectPtr conn)
 int ninactive;
 parallelsConnPtr privconn = conn-privateData;
 
-parallelsDriverLock(privconn);
 ninactive = virNetworkObjListNumOfNetworks(privconn-networks,
false, NULL, conn);
-parallelsDriverUnlock(privconn);
-
 return ninactive;
 }
 
@@ -403,10 +392,8 @@ static int 
parallelsConnectListDefinedNetworks(virConnectPtr conn,
 parallelsConnPtr privconn = conn-privateData;
 int got;
 
-parallelsDriverLock(privconn);
 got = virNetworkObjListGetNames(privconn-networks,
 false, names, nnames, NULL, conn);
-parallelsDriverUnlock(privconn);
 return got;
 }
 
@@ -415,15 +402,10 @@ static int parallelsConnectListAllNetworks(virConnectPtr 
conn,
unsigned int flags)
 {
 parallelsConnPtr privconn = conn-privateData;
-int ret = -1;
 
 virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
 
-parallelsDriverLock(privconn);
-ret = virNetworkObjListExport(conn, privconn-networks, nets, NULL, flags);
-parallelsDriverUnlock(privconn);
-
-return ret;
+return virNetworkObjListExport(conn, privconn-networks, nets, NULL, 
flags);
 }
 
 static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
@@ -433,9 +415,7 @@ static virNetworkPtr 
parallelsNetworkLookupByUUID(virConnectPtr conn,
 virNetworkObjPtr network;
 virNetworkPtr ret = NULL;
 
-parallelsDriverLock(privconn);
 network = virNetworkObjFindByUUID(privconn-networks, uuid);
-parallelsDriverUnlock(privconn);
 if (!network) {
 virReportError(VIR_ERR_NO_NETWORK,
%s, _(no network with matching uuid));
@@ -456,9 +436,7 @@ static virNetworkPtr 
parallelsNetworkLookupByName(virConnectPtr conn,
 virNetworkObjPtr network;
 virNetworkPtr ret = NULL;
 
-parallelsDriverLock(privconn);
 network = virNetworkObjFindByName(privconn-networks, name);
-parallelsDriverUnlock(privconn);
 if (!network) {
 virReportError(VIR_ERR_NO_NETWORK,
_(no network with matching name '%s'), name);
@@ -481,10 +459,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
 
 virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
 
-parallelsDriverLock(privconn);
 network = virNetworkObjFindByUUID(privconn-networks, net-uuid);
-parallelsDriverUnlock(privconn);
-
 if (!network) {
 virReportError(VIR_ERR_NO_NETWORK,
%s, _(no network with matching uuid));
@@ -504,9 +479,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
 virNetworkObjPtr obj;
 int ret = -1;
 
-parallelsDriverLock(privconn);
 obj = virNetworkObjFindByUUID(privconn-networks, net-uuid);
-parallelsDriverUnlock(privconn);
 if (!obj) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -524,9 +497,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
 virNetworkObjPtr obj;
 int ret = -1;
 
-parallelsDriverLock(privconn);
 obj = virNetworkObjFindByUUID(privconn-networks, net-uuid);
-parallelsDriverUnlock(privconn);
 if (!obj) {
 virReportError(VIR_ERR_NO_NETWORK, NULL);
 goto cleanup;
@@ -545,9 +516,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
   

Re: [libvirt] [PATCH] qemu: read backing chain names from qemu

[Eric Blake]

On 03/12/2015 03:14 AM, Daniel P. Berrange wrote:

 even though the file exists.  Trying to teach libvirt the rules on
 which name qemu will expect is not worth the effort (besides, we'd
 have to remember it across libvirtd restarts, and track whether a
 file was opened via metadata or via snapshot creation for a given
 qemu process); it is easier to just always directly ask qemu what
 string it expects to see in the first place.
 
 If you're going to ask QEMU for the names, then libvirt needs to
 validate that the name it gets back resolves to the same inode
 as the name it originally had. We cannot trust QEMU to tell the
 truth and cannot let it trick us into relabelling the wrong files
 by giving us the name of something completely different.
 

We are NOT relabelling files based on the information.  I agree that if
we act on a name returned by qemu, then we are vulnerable to mistakes if
a guest manages to compromise qemu; but I don't think this is one of
those situations.

 
  qemuDomainObjEnterMonitor(driver, vm);
 -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
 -  speed, mode, async);
 +if (baseSource)
 +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
 + baseSource);
 
 So this needs to valid basePath vs baseSource inodes.

The only use of basePath is the string passed right back to qemu, to
kick off the commit or pull operation.  Since qemu is using strcmp()
(rather than inode comparison), feeding it the name it just told us will
make the commit operation work on the correct node.  Everything else
that libvirt does, such as relabelling files, is done solely on the
information libvirt has been tracking (and not reliant on the name
returned by qemu).

But if it would satisfy your paranoia, I can certainly add a
verification step that the string being returned by qemu resolves to the
same inode being tracked by libvirt, at least in the case where the
disk element resolves to a filename rather than a network disk.

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



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

Re: [libvirt] [PATCHv2.5 01/10] conf: Add support for parsing and formatting max memory and slot count

[John Ferlan]

On 03/04/2015 11:24 AM, Peter Krempa wrote:
 Add a XML element that will allow to specify maximum supportable memory
 and the count of memory slots to use with memory hotplug.
 
 To avoid possible confusion and misuse of the new element this patch
 also explicitly forbids the use of the maxMemory setting in individual
 drivers's post parse callbacks. This limitation will be lifted when the
 support is implemented.
 ---
  docs/formatdomain.html.in| 19 +++
  docs/schemas/domaincommon.rng|  8 +
  src/bhyve/bhyve_domain.c |  4 +++
  src/conf/domain_conf.c   | 66 
 
  src/conf/domain_conf.h   |  7 
  src/libvirt_private.syms |  1 +
  src/libxl/libxl_domain.c |  5 +++
  src/lxc/lxc_domain.c |  4 +++
  src/openvz/openvz_driver.c   | 11 --
  src/parallels/parallels_driver.c |  6 +++-
  src/phyp/phyp_driver.c   |  6 +++-
  src/qemu/qemu_domain.c   |  4 +++
  src/uml/uml_driver.c |  6 +++-
  src/vbox/vbox_common.c   |  6 +++-
  src/vmware/vmware_driver.c   |  6 +++-
  src/vmx/vmx.c|  6 +++-
  src/xen/xen_driver.c |  4 +++
  src/xenapi/xenapi_driver.c   |  6 +++-
  tests/domainschemadata/maxMemory.xml | 19 +++
  19 files changed, 185 insertions(+), 9 deletions(-)
  create mode 100644 tests/domainschemadata/maxMemory.xml
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 6276a61..1f0f770 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -675,6 +675,7 @@
  pre
  lt;domaingt;
...
 +  lt;maxMemory slots='16' unit='KiB'gt;1524288lt;/maxMemorygt;
lt;memory unit='KiB'gt;524288lt;/memorygt;
lt;currentMemory unit='KiB'gt;524288lt;/currentMemorygt;
...
 @@ -708,6 +709,24 @@
  span class='since'codeunit/code since 0.9.11/span,
  span class='since'codedumpCore/code since 0.10.2
  (QEMU only)/span/dd
 +  dtcodemaxMemory/code/dt
 +  ddThe run time maximum memory allocation of the guest. The initial
 +memory specified by codelt;memorygt;/code can be increased by
 +hot-plugging of memory to the limit specified by this element.

Given the series I just reviewed - perhaps this needs a tweak to include
memory or numa

 +
 +The codeunit/code attribute behaves the same as for
 +codememory/code.

lt;memorygt;

 +
 +The codeslots/code attribute specifies the number of slots
 +available for adding memory to the guest. The bounds are hypervisor
 +specific.
 +
 +Note that due to alignment of the memory chunks added via memory
 +hotplug the full size allocation specified by this element may be
 +impossible to achieve.
 +span class='since'Since 1.2.14/span
 +  /dd
 +
dtcodecurrentMemory/code/dt
ddThe actual allocation of memory for the guest. This value can
  be less than the maximum allocation, to allow for ballooning
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 3272ad1..8e107e6 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -593,6 +593,14 @@
  /element
/optional
optional
 +element name=maxMemory
 +  ref name=scaledInteger/
 +  attribute name=slots
 +ref name=unsignedInt/
 +  /attribute
 +/element
 +  /optional
 +  optional
  element name=currentMemory
ref name='scaledInteger'/
  /element
 diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
 index ecb1758..25ef852 100644
 --- a/src/bhyve/bhyve_domain.c
 +++ b/src/bhyve/bhyve_domain.c
 @@ -67,6 +67,10 @@ bhyveDomainDefPostParse(virDomainDefPtr def,
 VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) 
  0)
  return -1;
 
 +/* memory hotplug tunables are not supported by this driver */
 +if (virDomainDefCheckUnsupportedMemoryHotplug(def)  0)
 +return -1;
 +
  return 0;
  }
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 84a9938..4d765f9 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -980,6 +980,27 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root,
  }
 
 
 +/**
 + * virDomainDefCheckUnsupportedMemoryHotplug:
 + * @def: domain definition
 + *
 + * Returns -1 if the domain definition would enable memory hotplug via the
 + * maxMemory tunable and reports an error. Otherwise returns 0.
 + */
 +int
 +virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def)
 +{
 +/* memory hotplug tunables are not supported by this driver */
 +if (def-mem.max_memory  0 || def-mem.memory_slots  0) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(memory hotplug tunables maxMemory are 

Re: [libvirt] [PATCHv2.5 02/10] qemu: Implement setup of memory hotplug parameters

[John Ferlan]

On 03/04/2015 11:24 AM, Peter Krempa wrote:
 To enable memory hotplug the maximum memory size and slot count need to
 be specified. As qemu supports now other units than mebibytes when
 specifying memory, use the new interface in this case.
 ---
  docs/formatdomain.html.in  |  2 +-
  src/qemu/qemu_command.c| 34 
 ++
  src/qemu/qemu_domain.c |  8 ++---
  .../qemuxml2argv-memory-hotplug-nonuma.xml | 22 ++
  .../qemuxml2argv-memory-hotplug.args   |  6 
  .../qemuxml2argv-memory-hotplug.xml| 34 
 ++
  tests/qemuxml2argvtest.c   |  4 +++
  tests/qemuxml2xmltest.c|  3 ++
  8 files changed, 102 insertions(+), 11 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml
 

ACK

John

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

Re: [libvirt] [PATCHv2.5 04/10] conf: Add interface to parse and format memory device information

[John Ferlan]

On 03/04/2015 11:24 AM, Peter Krempa wrote:
 This patch adds code that parses and formats configuration for memory
 devices.
 
 A simple configuration would be:
 memory model='dimm'
   target
 size unit='KiB'524287/size
 node0/node
   /target
 /memory
 
 A complete configuration of a memory device:
 memory model='dimm'
   source
 pagesize unit='KiB'4096/pagesize
 nodemask1-3/nodemask
   /source
   target
 size unit='KiB'524287/size
 node1/node
   /target
 /memory
 
 This patch preemptively forbids use of the memory device in individual
 drivers so the users are warned right away that the device is not
 supported.
 ---
  docs/formatdomain.html.in  |  79 +
  docs/schemas/domaincommon.rng  |  50 
  src/bhyve/bhyve_domain.c   |   5 +-
  src/conf/domain_conf.c | 324 
 -
  src/conf/domain_conf.h |  34 +++
  src/libvirt_private.syms   |   2 +
  src/libxl/libxl_domain.c   |   3 +
  src/lxc/lxc_domain.c   |   4 +
  src/openvz/openvz_driver.c |   3 +
  src/qemu/qemu_domain.c |   3 +
  src/qemu/qemu_driver.c |  13 +
  src/qemu/qemu_hotplug.c|   3 +
  src/uml/uml_driver.c   |   3 +
  src/xen/xen_driver.c   |   3 +
  src/xenapi/xenapi_driver.c |   3 +
  .../qemuxml2argv-memory-hotplug-dimm.xml   |  50 
  16 files changed, 579 insertions(+), 3 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index fcb4ca2..005f6f6 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -5901,6 +5901,85 @@ qemu-kvm -net nic,model=? /dev/null
  /dd
/dl
 
 +h4a name=elementsMemoryMemory devices/a/h4
 +
 +p
 +  Apart from initial memory the memory devices allow adding additional
 +  memory for the guest in form of memory modules. These devices also 
 allow
 +  hot-add and hot-remove of guest's memory.

In addition to the initial memory assigned to the guest, memory devices
allow additional memory to be assigned to the guest in the form of
memory modules. There can be 1 or many memory devices configured;
however, NUMA must also be configured for the guest. A memory device can
be hot-plugged or hot-unplugged depending on the guests' memory resource
needs.

 +  span class=sinceSince 1.2.14/span
 +/p
 +
 +p
 +  Example: usage of the memory devices
 +/p
 +pre
 +  ...
 +  lt;devicesgt;
 +lt;memory model='dimm'gt;
 +  lt;targetgt;
 +lt;size unit='KiB'gt;524287lt;/sizegt;
 +lt;nodegt;0lt;/nodegt;
 +  lt;/targetgt;
 +lt;/memorygt;
 +lt;memory model='dimm'gt;
 +  lt;sourcegt;
 +lt;pagesize unit='KiB'gt;4096lt;/pagesizegt;
 +lt;nodemaskgt;1-3lt;/nodemaskgt;
 +  lt;/sourcegt;
 +  lt;targetgt;
 +lt;size unit='KiB'gt;524287lt;/sizegt;
 +lt;nodegt;1lt;/nodegt;
 +  lt;/targetgt;
 +lt;/memorygt;
 +  lt;/devicesgt;
 +  ...
 +/pre
 +dl
 +  dtcodemodel/code/dt
 +  dd
 +p
 +  Currently only the codedimm/code model is supported that
 +  will result in adding a virtual DIMM module to the guest. Note that
 +  hypervisors require having NUMA enabled for the guest for the 
 memory
 +  modules to work.

Reads strangely - the that will result in adding a virtual DIMM module
to the guest almost seems redundant, but I also see it's importance.
Perhaps if the that will result in adding is replace by in order to add.

Whether it's worth repeating the NUMA here or not is up to you, I just
figured it was better to call it out earlier rather...

 +/p
 +  /dd
 +
 +  dtcodesource/code/dt
 +  dd
 +p
 +  The optional source element allows to fine tune the source of the
 +  memory used for the given memory device. If the element is not
 +  provided defaults configured via codenumatune/code are used.
 +/p
 +p
 +  codepagesize/code can be used to override the default host page
 +  size used for backing the memory device.

Is pagesize something hypervisor specific or should it be suggested as
a power of 2 at least?

 +/p
 +p
 +  codenodemask/code can be used to override the default set of 
 NUMA
 +  nodes where the memory would be allocated.
 +/p
 +  /dd
 +

Is it worth nothing that either or both can be used (eg, both are optional).

 +  dtcodetarget/code/dt
 +  dd
 +p
 +  The mandatory codetarget/code element configures the placement 
 and
 +  sizing of the 

Re: [libvirt] [PATCHv2.5 03/10] conf: Add device address type for dimm devices

[John Ferlan]

On 03/04/2015 11:24 AM, Peter Krempa wrote:
 Dimm devices are described by the slot and base address. Add a new
 address type to be able to describe such address.
 ---
  docs/schemas/domaincommon.rng | 18 +++
  src/conf/domain_conf.c| 74 
 ++-
  src/conf/domain_conf.h|  9 ++
  3 files changed, 100 insertions(+), 1 deletion(-)
 
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 8e107e6..c1c02e4 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -4028,6 +4028,18 @@
/attribute
  /optional
/define
 +  define name=dimmaddress
 +optional
 +  attribute name=slot
 +ref name=unsignedInt/
 +  /attribute
 +/optional
 +optional
 +  attribute name=base
 +ref name=hexuint/
 +  /attribute
 +/optional
 +  /define
define name=devices
  element name=devices
interleave
 @@ -4447,6 +4459,12 @@
  valuevirtio-mmio/value
/attribute
  /group
 +group
 +  attribute name=type
 +valuedimm/value
 +  /attribute
 +  ref name=dimmaddress/
 +/group
/choice
  /element
/define
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4d765f9..3be52a7 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -249,7 +249,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
virtio-s390,
ccw,
virtio-mmio,
 -  isa)
 +  isa,
 +  dimm)
 
  VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST,
disk,
 @@ -2828,6 +2829,11 @@ virDomainDeviceInfoAddressIsEqual(const 
 virDomainDeviceInfo *a,
  if (memcmp(a-addr.isa, b-addr.isa, sizeof(a-addr.isa)))
  return false;
  break;
 +
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
 +if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm)))
 +return false;
 +break;
  }
 
  return true;
 @@ -3657,6 +3663,12 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
  virBufferAsprintf(buf,  irq='0x%x', info-addr.isa.irq);
  break;
 
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
 +virBufferAsprintf(buf,  slot='%u', info-addr.dimm.slot);
 +virBufferAsprintf(buf,  base='0x%llx', info-addr.dimm.base);
 +
 +break;
 +
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 @@ -4026,6 +4038,41 @@ virDomainDeviceISAAddressParseXML(xmlNodePtr node,
  return ret;
  }
 
 +
 +static int
 +virDomainDeviceDimmAddressParseXML(xmlNodePtr node,
 +   virDomainDeviceDimmAddressPtr addr)
 +{
 +int ret = -1;
 +char *tmp = NULL;
 +
 +if (!(tmp = virXMLPropString(node, slot)) ||
 +virStrToLong_uip(tmp, NULL, 10, addr-slot)  0) {
 +virReportError(VIR_ERR_XML_ERROR,
 +   _(invalid or missing dimm slot id '%s'),
 +   NULLSTR(tmp));
 +goto cleanup;
 +}
 +VIR_FREE(tmp);
 +
 +if (!(tmp = virXMLPropString(node, base)) ||
 +virStrToLong_ullp(tmp, NULL, 16, addr-base)  0) {
 +virReportError(VIR_ERR_XML_ERROR,
 +   _(invalid or missing dimm base address '%s'),
 +   NULLSTR(tmp));
 +goto cleanup;
 +}
 +VIR_FREE(tmp);
 +
 +ret = 0;
 +
 + cleanup:
 +VIR_FREE(tmp);
 +
 +return ret;
 +}
 +
 +
  /* Parse the XML definition for a device address
   * @param node XML nodeset to parse for device address definition
   */
 @@ -4167,6 +4214,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
 _(virtio-s390 bus doesn't have an address));
  goto cleanup;
 
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
 +if (virDomainDeviceDimmAddressParseXML(address, info-addr.dimm)  
 0)
 +goto cleanup;
 +break;
 +
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
  break;
 @@ -15295,6 +15347,26 @@ 
 virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
  }
  break;
 
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
 +if (src-addr.dimm.slot != dst-addr.dimm.slot) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(Target device dimm slot %d does not match 
 + source %d),
 +   dst-addr.dimm.slot,
 +   src-addr.dimm.slot);

Both are %u not %d

ACK - with the adjustment.

John

 +return false;
 +}
 +
 +if (src-addr.dimm.base != dst-addr.dimm.base) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   

Re: [libvirt] [PATCH v2] qemu: read backing chain names from qemu

[Shanzhi Yu]

I do meet libvirtd crash sometime when test this patch(I also
met it when test v1 yesterday, but can not reproduce it 100%.)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe9d39700 (LWP 25413)]
virJSONValueObjectGetString (object=0x0, key=key@entry=0x7fffe4f72429
filename) at util/virjson.c:1074
1074if (object-type != VIR_JSON_TYPE_OBJECT)
(gdb) t a a bt

Thread 6 (Thread 0x7fffe9d39700 (LWP 25413)):
#0  virJSONValueObjectGetString (object=0x0,
key=key@entry=0x7fffe4f72429 filename) at util/virjson.c:1074
#1  0x7fffe4f2a1f4 in qemuMonitorJSONDiskNameLookupOne
(image=optimized out, top=0x7fffd40013b0,
target=target@entry=0x7fffd40013b0)
at qemu/qemu_monitor_json.c:3901
#2  0x7fffe4f2a1bc in qemuMonitorJSONDiskNameLookupOne
(image=optimized out, top=top@entry=0x7fffdc0fc940,
target=target@entry=0x7fffd40013b0)
at qemu/qemu_monitor_json.c:3898
#3  0x7fffe4f31800 in qemuMonitorJSONDiskNameLookup (mon=optimized
out, device=0x7fffd429cee0 drive-virtio-disk0, top=0x7fffdc0fc940,
target=target@entry=0x7fffd40013b0) at qemu/qemu_monitor_json.c:3963
#4  0x7fffe4f1f87e in qemuMonitorDiskNameLookup (mon=optimized
out, device=optimized out, top=optimized out,
target=target@entry=0x7fffd40013b0)
at qemu/qemu_monitor.c:3475
#5  0x7fffe4f55775 in qemuDomainBlockCommit (dom=optimized out,
path=optimized out, base=optimized out, top=optimized out,
bandwidth=optimized out,
flags=optimized out) at qemu/qemu_driver.c:16937
#6  0x775ff933 in virDomainBlockCommit
(dom=dom@entry=0x7fffd429d630, disk=0x7fffd40010a0 vda, base=0x0,
top=0x0, bandwidth=0, flags=5)
at libvirt-domain.c:10218
#7  0x555736fe in remoteDispatchDomainBlockCommit
(server=optimized out, msg=optimized out, args=0x7fffd429d9c0,
rerr=0x7fffe9d38cb0,
client=optimized out) at remote_dispatch.h:2594
#8  remoteDispatchDomainBlockCommitHelper (server=optimized out,
client=optimized out, msg=optimized out, rerr=0x7fffe9d38cb0,
args=0x7fffd429d9c0,
ret=optimized out) at remote_dispatch.h:2564
#9  0x77653db9 in virNetServerProgramDispatchCall
(msg=0x557d8240, client=0x557ce4a0, server=0x557cc820,
prog=0x557d4a40)
at rpc/virnetserverprogram.c:437
#10 virNetServerProgramDispatch (prog=0x557d4a40,
server=server@entry=0x557cc820, client=0x557ce4a0,
msg=0x557d8240) at rpc/virnetserverprogram.c:307
#11 0x555989d8 in virNetServerProcessMsg (msg=optimized out,
prog=optimized out, client=optimized out, srv=0x557cc820) at
rpc/virnetserver.c:172
#12 virNetServerHandleJob (jobOpaque=optimized out,
opaque=0x557cc820) at rpc/virnetserver.c:193
#13 0x7755ed8e in virThreadPoolWorker
(opaque=opaque@entry=0x557d8370) at util/virthreadpool.c:144
#14 0x7755e72e in virThreadHelper (data=optimized out) at
util/virthread.c:197
#15 0x75de252a in start_thread (arg=0x7fffe9d39700) at
pthread_create.c:310
#16 0x75b1e22d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109



On 03/13/2015 04:23 AM, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
 after a series of disk snapshots into existing destination images,
 followed by active commits of the top image, it is possible for
 qemu 2.2 and earlier to end up tracking a different name for the
 image than what it would have had when opening the chain afresh.
 That is, when starting with the chain 'a - b - c', the name
 associated with 'b' is how it was spelled in the metadata of 'c',
 but when starting with 'a', taking two snapshots into 'a - b - c',
 then committing 'c' back into 'b', the name associated with 'b' is
 now the name used when taking the first snapshot.

 Sadly, older qemu doesn't know how to treat different spellings of
 the same filename as identical files (it uses strcmp() instead of
 checking for the same inode), which means libvirt's attempt to
 commit an image using solely the names learned from qcow2 metadata
 fails with a cryptic:

 error: internal error: unable to execute QEMU command 'block-commit': Top 
 image file /tmp/images/c/../b/b not found

 even though the file exists.  Trying to teach libvirt the rules on
 which name qemu will expect is not worth the effort (besides, we'd
 have to remember it across libvirtd restarts, and track whether a
 file was opened via metadata or via snapshot creation for a given
 qemu process); it is easier to just always directly ask qemu what
 string it expects to see in the first place.

 As a safety valve, we validate that any name returned by qemu
 still maps to the same local file as we have tracked it, so that
 a compromised qemu cannot accidentally cause us to act on an
 incorrect file.

 * src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New
 prototype.
 * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):
 Likewise.
 * src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.
 * src/qemu/qemu_monitor_json.c 

[libvirt] [PATCH] parallels: fix prlsdkCheckUnsupportedParams checks

[Maxim Nestratov]

for memory limits since unset ones are no longer zero

Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
 src/parallels/parallels_sdk.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index a775348..4c90a18 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1791,10 +1791,10 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 }
 
 if (def-mem.nhugepages ||
-def-mem.hard_limit ||
-def-mem.soft_limit ||
+virMemoryLimitIsSet(def-mem.hard_limit) ||
+virMemoryLimitIsSet(def-mem.soft_limit) ||
 def-mem.min_guarantee ||
-def-mem.swap_hard_limit) {
+virMemoryLimitIsSet(def-mem.swap_hard_limit)) {
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Memory parameter is not supported 
-- 
1.7.1

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

Re: [libvirt] [PATCH v2] virnetdev: fix build with old kernel

[Michal Privoznik]

On 11.03.2015 15:27, Pavel Hrdina wrote:
 Commit c9027d8f added a detection of NIC HW features, but some of them
 are not available in old kernel.  Very old kernels lack enum
 ethtool_flags and even if this enum is present, not all values are
 available for all kernels.  To be sure that we have everything in kernel
 that we need, we must check for existence of most of that flags, because
 only few of them were defined at first.
 
 Also to successfully build libvirt with older kernel we need to include
 linux/types.h before linux/ethtool.h to have __u32 and friends
 defined.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  configure.ac |  5 +
  src/util/virnetdev.c | 43 +++
  2 files changed, 32 insertions(+), 16 deletions(-)
 
 diff --git a/configure.ac b/configure.ac
 index e071813..2fedd1a 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -383,6 +383,11 @@ AC_CHECK_TYPE([struct ifreq],
  #include net/if.h
]])
  
 +AC_CHECK_DECLS([ETH_FLAG_TXVLAN, ETH_FLAG_NTUPLE, ETH_FLAG_RXHASH, 
 ETH_FLAG_LRO,
 +ETHTOOL_GGSO, ETHTOOL_GGRO, ETHTOOL_GFLAGS],
 +  [], [], [[#include linux/ethtool.h
 +  ]])
 +

GSO and GRO were added in the same commit 37c3185a. So one of these
checks can be dropped.

ACK with that fixed.

Michal

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

Re: [libvirt] [PATCH] parallels: add VIR_ARCH_I686 capability to parallels driver

[Michal Privoznik]

On 10.03.2015 20:32, Maxim Nestratov wrote:
 as soon as x32 architecture is also supported
 ---
  src/parallels/parallels_driver.c |7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/src/parallels/parallels_driver.c 
 b/src/parallels/parallels_driver.c
 index 650b790..0b3761d 100644
 --- a/src/parallels/parallels_driver.c
 +++ b/src/parallels/parallels_driver.c
 @@ -99,6 +99,13 @@ parallelsBuildCapabilities(void)
   NULL, 0, NULL)) == NULL)
  goto error;
  
 +if ((guest = virCapabilitiesAddGuest(caps, hvm,
 + VIR_ARCH_I686,
 + parallels,
 + NULL, 0, NULL)) == NULL)
 +goto error;
 +
 +
  if (virCapabilitiesAddGuestDomain(guest,
parallels, NULL, NULL, 0, NULL) == 
 NULL)
  goto error;
 

ACKed and pushed.

Michal

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

[libvirt] [PATCH 3/7] virsh: domain: Fix the change-media command

[Peter Krempa]

The command did not modify the disk type and thus didn't allow to change
media from a file image to a block backed image or vice versa. In
addition when operating on a network backed removable devices the
command would replace the while source subelement with an invalid one.

This patch adds the --block option that allows to specify that the new
image is block backed and assumes that without that option all images
are file backed. Since network backends were always mangled it should
not cause problems.
---
 tools/virsh-domain.c | 149 ++-
 tools/virsh.pod  |   6 ++-
 2 files changed, 79 insertions(+), 76 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 23d0b63..cf9bd65 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11177,11 +11177,10 @@ vshFindDisk(const char *doc,
 }

 typedef enum {
-VSH_PREPARE_DISK_XML_NONE = 0,
-VSH_PREPARE_DISK_XML_EJECT,
-VSH_PREPARE_DISK_XML_INSERT,
-VSH_PREPARE_DISK_XML_UPDATE,
-} vshPrepareDiskXMLType;
+VSH_UPDATE_DISK_XML_EJECT,
+VSH_UPDATE_DISK_XML_INSERT,
+VSH_UPDATE_DISK_XML_UPDATE,
+} vshUpdateDiskXMLType;

 /* Helper function to prepare disk XML. Could be used for disk
  * detaching, media changing(ejecting, inserting, updating)
@@ -11189,15 +11188,14 @@ typedef enum {
  * success, or NULL on failure. Caller must free the result.
  */
 static char *
-vshPrepareDiskXML(xmlNodePtr disk_node,
-  const char *source,
-  const char *path,
-  int type)
+vshUpdateDiskXML(xmlNodePtr disk_node,
+ const char *new_source,
+ bool source_block,
+ const char *target,
+ vshUpdateDiskXMLType type)
 {
-xmlNodePtr cur = NULL;
-char *disk_type = NULL;
+xmlNodePtr source = NULL;
 char *device_type = NULL;
-xmlNodePtr new_node = NULL;
 char *ret = NULL;

 if (!disk_node)
@@ -11205,62 +11203,64 @@ vshPrepareDiskXML(xmlNodePtr disk_node,

 device_type = virXMLPropString(disk_node, device);

-if (STREQ_NULLABLE(device_type, cdrom) ||
-STREQ_NULLABLE(device_type, floppy)) {
-bool has_source = false;
-disk_type = virXMLPropString(disk_node, type);
+if (!(STREQ_NULLABLE(device_type, cdrom) ||
+  STREQ_NULLABLE(device_type, floppy))) {
+vshError(NULL, _(The disk device '%s' is not removable), target);
+goto cleanup;
+}

-cur = disk_node-children;
-while (cur != NULL) {
-if (cur-type == XML_ELEMENT_NODE 
-xmlStrEqual(cur-name, BAD_CAST source)) {
-has_source = true;
-break;
-}
-cur = cur-next;
+/* find the current source subelement */
+for (source = disk_node-children; source; source = source-next) {
+if (source-type == XML_ELEMENT_NODE 
+xmlStrEqual(source-name, BAD_CAST source))
+break;
+}
+
+if (type == VSH_UPDATE_DISK_XML_EJECT) {
+if (!source) {
+vshError(NULL, _(The disk device '%s' doesn't have media), 
target);
+goto cleanup;
 }

-if (!has_source) {
-if (type == VSH_PREPARE_DISK_XML_EJECT) {
-vshError(NULL, _(The disk device '%s' doesn't have media),
- path);
-goto cleanup;
-}
+/* forcibly switch to empty file cdrom */
+source_block = false;
+new_source = NULL;
+} else if (!new_source) {
+vshError(NULL, _(New disk media source was not specified));
+goto cleanup;
+}

-if (source) {
-new_node = xmlNewNode(NULL, BAD_CAST source);
-if (STREQ(disk_type, block))
-xmlNewProp(new_node, BAD_CAST dev, BAD_CAST source);
-else
-xmlNewProp(new_node, BAD_CAST disk_type, BAD_CAST source);
-xmlAddChild(disk_node, new_node);
-} else if (type == VSH_PREPARE_DISK_XML_INSERT) {
-vshError(NULL, _(No source is specified for inserting 
media));
-goto cleanup;
-} else if (type == VSH_PREPARE_DISK_XML_UPDATE) {
-vshError(NULL, _(No source is specified for updating media));
-goto cleanup;
-}
-}
+if (type == VSH_UPDATE_DISK_XML_INSERT  source) {
+vshError(NULL, _(The disk device '%s' already has media), target);
+goto cleanup;
+}

-if (has_source) {
-if (type == VSH_PREPARE_DISK_XML_INSERT) {
-vshError(NULL, _(The disk device '%s' already has media),
- path);
-goto cleanup;
-}
+/* remove current source */
+if (source) {
+xmlUnlinkNode(source);
+xmlFreeNode(source);
+source = NULL;
+}

-/* Remove the 

[libvirt] [PATCH 1/7] virsh: domain: Don't use vshPrepareDiskXML for creating XML to detach disk

[Peter Krempa]

Since cmdDetachDisk() calls into vshPrepareDiskXML() with
type == VSH_PREPARE_DISK_XML_NONE  source == NULL this would result
into skipping all the checks and effectively turn the function into a
XML formatter.

This patch changes the code to use the formatter directly so that the
function can be refactored in a easier way.
---
 tools/virsh-domain.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1a364bb..8bc0512 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11365,9 +11365,10 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
 if (!(disk_node = vshFindDisk(doc, target, VSH_FIND_DISK_NORMAL)))
 goto cleanup;

-if (!(disk_xml = vshPrepareDiskXML(disk_node, NULL, NULL,
-   VSH_PREPARE_DISK_XML_NONE)))
+if (!(disk_xml = virXMLNodeToString(NULL, disk_node))) {
+vshSaveLibvirtError();
 goto cleanup;
+}

 if (flags != 0 || current)
 ret = virDomainDetachDeviceFlags(dom, disk_xml, flags);
-- 
2.2.2

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

[libvirt] [PATCH 7/7] conf: disk: Simplify checking if source definition was parsed

[Peter Krempa]

Previously we had to check for 3 fields to see if the source was filled.
Repurpose one of the variables as a boolean flag and use it instead of
combining multiple sources.

For the condition that checks that only CDROM/FLOPPY drives can be empty
we can use the virStorageSourceIsEmpty() helper.
---
 src/conf/domain_conf.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 127fc91..dee3a00 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5901,7 +5901,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *sgio = NULL;
 char *driverName = NULL;
 char *driverType = NULL;
-const char *source = NULL;
+bool source = false;
 char *target = NULL;
 char *trans = NULL;
 char *bus = NULL;
@@ -5965,13 +5965,13 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 cur = node-children;
 while (cur != NULL) {
 if (cur-type == XML_ELEMENT_NODE) {
-if (!source  !def-src-hosts  !def-src-srcpool 
-xmlStrEqual(cur-name, BAD_CAST source)) {
+if (!source  xmlStrEqual(cur-name, BAD_CAST source)) {
 sourceNode = cur;

 if (virDomainDiskSourceParse(cur, ctxt, def-src)  0)
 goto error;
-source = def-src-path;
+
+source = true;

 if (def-src-type == VIR_STORAGE_TYPE_NETWORK) {
 if (def-src-protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI)
@@ -6398,7 +6398,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 /* Only CDROM and Floppy devices are allowed missing source path
  * to indicate no media present. LUN is for raw access CD-ROMs
  * that are not attached to a physical device presently */
-if (source == NULL  def-src-hosts == NULL  !def-src-srcpool 
+if (virStorageSourceIsEmpty(def-src) 
 (def-device == VIR_DOMAIN_DISK_DEVICE_DISK ||
  (flags  VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) {
 virReportError(VIR_ERR_NO_SOURCE,
@@ -6430,7 +6430,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 virReportError(VIR_ERR_NO_TARGET, %s, tmp);
 VIR_FREE(tmp);
 } else {
-virReportError(VIR_ERR_NO_TARGET, source ? %s : NULL, source);
+virReportError(VIR_ERR_NO_TARGET, def-src-path ? %s : NULL, 
def-src-path);
 }
 goto error;
 }
-- 
2.2.2

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

[libvirt] [PATCH 4/7] qemu: hotplug: Use checker function to check if disk is empty

[Peter Krempa]

---
 src/qemu/qemu_hotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6ad48f7..29e0fe3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -222,7 +222,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 goto error;
 }

-if (!virStorageSourceIsLocalStorage(newsrc) || newsrc-path) {
+if (!virStorageSourceIsEmpty(newsrc)) {
 if (qemuGetDriveSourceString(newsrc, conn, sourcestr)  0)
 goto error;

-- 
2.2.2

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

Re: [libvirt] [PATCH] vbox: use user cache dir when screenshotting.

[Michal Privoznik]

On 09.03.2015 16:07, Dawid Zamirski wrote:
 For VBOX it's most likely that the connection is vbox:///session and it
 runs with local non-root account. This caused permission denied when
 LOCALSTATEDIR was used to create temp file. This patch makes use of the
 virGetUserCacheDirectory to address this problem for non-root users.
 ---
  src/vbox/vbox_common.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)
 
 diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
 index 55d3624..a548252 100644
 --- a/src/vbox/vbox_common.c
 +++ b/src/vbox/vbox_common.c
 @@ -7254,8 +7254,10 @@ vboxDomainScreenshot(virDomainPtr dom,
  IMachine *machine = NULL;
  nsresult rc;
  char *tmp;
 +char *cacheDir;
  int tmp_fd = -1;
  unsigned int max_screen;
 +uid_t uid = geteuid();
  char *ret = NULL;
  
  if (!data-vboxObj)
 @@ -7288,8 +7290,18 @@ vboxDomainScreenshot(virDomainPtr dom,
  return NULL;
  }
  
 -if (virAsprintf(tmp, %s/cache/libvirt/vbox.screendump.XX, 
 LOCALSTATEDIR)  0) {
 +if (uid != 0)
 +cacheDir = virGetUserCacheDirectory();

If one side of 'else' has braces, so ought the other one.

 +else {
 +if (virAsprintf(cacheDir, %s/cache/libvirt, LOCALSTATEDIR)  0) {
 +VBOX_RELEASE(machine);
 +return NULL;
 +}
 +}
 +
 +if (cacheDir  virAsprintf(tmp, %s/vbox.screendump.XX, cacheDir) 
  0) {

cacheDir should not be NULL here. The only way that could be is if 
virGetUserCacheDirectory() failed, in which case we want to throw an error 
anyway. So I'm squashing this in, ACKing and pushing:

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index a548252..bd71c81 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7257,7 +7257,7 @@ vboxDomainScreenshot(virDomainPtr dom,
 char *cacheDir;
 int tmp_fd = -1;
 unsigned int max_screen;
-uid_t uid = geteuid();
+bool privileged = geteuid() == 0;
 char *ret = NULL;
 
 if (!data-vboxObj)
@@ -7290,13 +7290,10 @@ vboxDomainScreenshot(virDomainPtr dom,
 return NULL;
 }
 
-if (uid != 0)
-cacheDir = virGetUserCacheDirectory();
-else {
-if (virAsprintf(cacheDir, %s/cache/libvirt, LOCALSTATEDIR)  0) {
-VBOX_RELEASE(machine);
-return NULL;
-}
+if ((privileged  virAsprintf(cacheDir, %s/cache/libvirt, 
LOCALSTATEDIR)  0) ||
+(!privileged  !(cacheDir = virGetUserCacheDirectory( {
+VBOX_RELEASE(machine);
+return NULL;
 }
 
 if (cacheDir  virAsprintf(tmp, %s/vbox.screendump.XX, cacheDir)  
0) {

Congratulations on your first libvirt contribution!

Michal

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

[libvirt] [PATCH 5/7] qemu: driver: Fix cold-update of removable storage devices

[Peter Krempa]

Only selected fields from the disk source were copied when cold updating
source in a CDROM drive. When such drive was backed by a network file
this resulted into corruption of the definition:

disk type='network' device='cdrom'
  driver name='qemu' type='raw' cache='none'/
  source protocol='gluster' name='gluster-vol1(null)'
host name='localhost'/
  /source
  target dev='vdc' bus='virtio'/
  readonly/
  address type='pci' domain='0x' bus='0x00' slot='0x0a' 
function='0x0'/
/disk

Update the whole source instead of cherry-picking elements.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1166024
---
 src/qemu/qemu_driver.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3263ac..0062f36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8010,19 +8010,11 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
  * Update 'orig'
  * We allow updating src/type//driverType/cachemode/
  */
-VIR_FREE(orig-src-path);
-orig-src-path = disk-src-path;
-orig-src-type = disk-src-type;
 orig-cachemode = disk-cachemode;
-if (disk-src-driverName) {
-VIR_FREE(orig-src-driverName);
-orig-src-driverName = disk-src-driverName;
-disk-src-driverName = NULL;
-}
-if (disk-src-format)
-orig-src-format = disk-src-format;
-disk-src-path = NULL;
-orig-startupPolicy = disk-startupPolicy;
+
+virStorageSourceFree(orig-src);
+orig-src = disk-src;
+disk-src = NULL;
 break;

 case VIR_DOMAIN_DEVICE_GRAPHICS:
-- 
2.2.2

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

[libvirt] [PATCH 2/7] virsh: domain: Add --print-xml flag for command change-media

[Peter Krempa]

Allow printing the XML that would be used mostly for debugging purposes.
---
 tools/virsh-domain.c | 17 +
 tools/virsh.pod  |  3 +++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8bc0512..23d0b63 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12274,6 +12274,10 @@ static const vshCmdOptDef opts_change_media[] = {
  .type = VSH_OT_BOOL,
  .help = N_(force media changing)
 },
+{.name = print-xml,
+ .type = VSH_OT_BOOL,
+ .help = N_(print XML document rather than change media)
+},
 {.name = NULL}
 };

@@ -12354,12 +12358,17 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd)
 if (!(disk_xml = vshPrepareDiskXML(disk_node, source, path, prepare_type)))
 goto cleanup;

-if (virDomainUpdateDeviceFlags(dom, disk_xml, flags) != 0) {
-vshError(ctl, _(Failed to complete action %s on media), action);
-goto cleanup;
+if (vshCommandOptBool(cmd, print-xml)) {
+vshPrint(ctl, %s, disk_xml);
+} else {
+if (virDomainUpdateDeviceFlags(dom, disk_xml, flags) != 0) {
+vshError(ctl, _(Failed to complete action %s on media), action);
+goto cleanup;
+}
+
+vshPrint(ctl, _(succeeded to complete action %s on media\n), action);
 }

-vshPrint(ctl, _(succeeded to complete action %s on media\n), action);
 ret = true;

  cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 714de34..7d43f6f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2581,6 +2581,7 @@ expected.

 =item Bchange-media Idomain Ipath [I--eject] [I--insert]
 [I--update] [Isource] [I--force] [[I--live] [I--config] | 
[I--current]]
+[I--print-xml]

 Change media of CDROM or floppy drive. Ipath can be the fully-qualified path
 or the unique target name (target dev='hdc') of the disk device. Isource
@@ -2604,6 +2605,8 @@ the hypervisor's implementation.
 Both I--live and I--config flags may be given, but I--current is
 exclusive. If no flag is specified, behavior is different depending
 on hypervisor.
+If I--print-xml is specified the XML that would be used to change media is
+printed instead of changing the media.

 =back

-- 
2.2.2

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

Re: [libvirt] [PATCH v2] virnetdev: fix build with old kernel

[Pavel Hrdina]

On Thu, Mar 12, 2015 at 04:43:28PM +0100, Michal Privoznik wrote:
 On 11.03.2015 15:27, Pavel Hrdina wrote:
  Commit c9027d8f added a detection of NIC HW features, but some of them
  are not available in old kernel.  Very old kernels lack enum
  ethtool_flags and even if this enum is present, not all values are
  available for all kernels.  To be sure that we have everything in kernel
  that we need, we must check for existence of most of that flags, because
  only few of them were defined at first.
  
  Also to successfully build libvirt with older kernel we need to include
  linux/types.h before linux/ethtool.h to have __u32 and friends
  defined.
  
  Signed-off-by: Pavel Hrdina phrd...@redhat.com
  ---
   configure.ac |  5 +
   src/util/virnetdev.c | 43 +++
   2 files changed, 32 insertions(+), 16 deletions(-)
  
  diff --git a/configure.ac b/configure.ac
  index e071813..2fedd1a 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -383,6 +383,11 @@ AC_CHECK_TYPE([struct ifreq],
   #include net/if.h
 ]])
   
  +AC_CHECK_DECLS([ETH_FLAG_TXVLAN, ETH_FLAG_NTUPLE, ETH_FLAG_RXHASH, 
  ETH_FLAG_LRO,
  +ETHTOOL_GGSO, ETHTOOL_GGRO, ETHTOOL_GFLAGS],
  +  [], [], [[#include linux/ethtool.h
  +  ]])
  +
 
 GSO and GRO were added in the same commit 37c3185a. So one of these
 checks can be dropped.

Well, actually only SGSO and GGSO are in that commit and SGRO and GGRO are in
different commit b240a0e5.

Pushed now with both checks, thanks for review.

Pavel

 
 ACK with that fixed.
 
 Michal

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

[libvirt] [PATCH 6/7] util: storage: Fix check for empty storage device

[Peter Krempa]

If the storage device type is parsed as network our parser still allows
it to omit the source element. The empty drive check would not trigger
on such device as it expects that every network storage source is valid.

Use VIR_STORAGE_NET_PROTOCOL_NONE as a marker that the storage source is
empty.
---
 src/util/virstoragefile.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 78a7a9f..96be02e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1989,6 +1989,10 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
 if (src-type == VIR_STORAGE_TYPE_NONE)
 return true;

+if (src-type == VIR_STORAGE_TYPE_NETWORK 
+src-protocol == VIR_STORAGE_NET_PROTOCOL_NONE)
+return true;
+
 return false;
 }

-- 
2.2.2

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

Re: [libvirt] [PATCH] vmx: add e1000e to supported NIC models.

[Michal Privoznik]

On 09.03.2015 16:16, Dawid Zamirski wrote:
 From: Dawid Zamirski dzamir...@dattobackup.com
 
 This NIC model is supported on hardware version 8 and newer and libvirt
 ESX driver does support those.
 ---
  src/vmx/vmx.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
 index ac2542a..fe6b883 100644
 --- a/src/vmx/vmx.c
 +++ b/src/vmx/vmx.c
 @@ -2536,10 +2536,11 @@ virVMXParseEthernet(virConfPtr conf, int controller, 
 virDomainNetDefPtr *def)
  if (STRCASENEQ(virtualDev, vlance) 
  STRCASENEQ(virtualDev, vmxnet) 
  STRCASENEQ(virtualDev, vmxnet3) 
 -STRCASENEQ(virtualDev, e1000)) {
 +STRCASENEQ(virtualDev, e1000) 
 +STRCASENEQ(virtualDev, e1000e)) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(Expecting VMX entry '%s' to be 'vlance' or 
 'vmxnet' or 
 - 'vmxnet3' or 'e1000' but found '%s'), 
 virtualDev_name,
 + 'vmxnet3' or 'e1000e' or 'e1000e' but found 
 '%s'), virtualDev_name,
 virtualDev);
  goto cleanup;
  }
 @@ -3592,11 +3593,12 @@ virVMXFormatEthernet(virDomainNetDefPtr def, int 
 controller,
  STRCASENEQ(def-model, vmxnet) 
  STRCASENEQ(def-model, vmxnet2) 
  STRCASENEQ(def-model, vmxnet3) 
 -STRCASENEQ(def-model, e1000)) {
 +STRCASENEQ(def-model, e1000) 
 +STRCASENEQ(def-model, e1000e)) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(Expecting domain XML entry 
 'devices/interface/model' 
   to be 'vlance' or 'vmxnet' or 'vmxnet2' or 
 'vmxnet3' 
 - or 'e1000' but found '%s'), def-model);
 + or 'e1000' or 'e1000e' but found '%s'), 
 def-model);
  return -1;
  }
  
 

ACKed and pushed.

I've noticed increasing number of e-mails form datto.com domain. If
you're developing an mgmt application which is open-source, do you mind
proposing a patch for docs/apps.html.in too? It's the source for the
following page:

  http://libvirt.org/apps.html

Michal

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

[libvirt] [PATCH 0/7] Fix ejecting of network backed cdroms via virsh

[Peter Krempa]

It turned out that not only virsh was broken ...

Peter Krempa (7):
  virsh: domain: Don't use vshPrepareDiskXML for creating XML to detach
disk
  virsh: domain: Add --print-xml flag for command change-media
  virsh: domain: Fix the change-media command
  qemu: hotplug: Use checker function to check if disk is empty
  qemu: driver: Fix cold-update of removable storage devices
  util: storage: Fix check for empty storage device
  conf: disk: Simplify checking if source definition was parsed

 src/conf/domain_conf.c|  12 ++--
 src/qemu/qemu_driver.c|  16 ++---
 src/qemu/qemu_hotplug.c   |   2 +-
 src/util/virstoragefile.c |   4 ++
 tools/virsh-domain.c  | 171 --
 tools/virsh.pod   |   7 +-
 6 files changed, 112 insertions(+), 100 deletions(-)

-- 
2.2.2

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

Re: [libvirt] [PATCH] qemu: read backing chain names from qemu

[Eric Blake]

On 03/12/2015 08:23 AM, Daniel P. Berrange wrote:


 But if it would satisfy your paranoia, I can certainly add a
 verification step that the string being returned by qemu resolves to the
 same inode being tracked by libvirt, at least in the case where the
 disk element resolves to a filename rather than a network disk.
 
 I think it would be desirable, because while your current usage
 may be safe with these assumptions, if someone refactors this
 6 months later they may not realize the security implications
 of this code.

v2 posted on those grounds.

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



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

[libvirt] [PATCH v2] qemu: read backing chain names from qemu

[Eric Blake]

https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
after a series of disk snapshots into existing destination images,
followed by active commits of the top image, it is possible for
qemu 2.2 and earlier to end up tracking a different name for the
image than what it would have had when opening the chain afresh.
That is, when starting with the chain 'a - b - c', the name
associated with 'b' is how it was spelled in the metadata of 'c',
but when starting with 'a', taking two snapshots into 'a - b - c',
then committing 'c' back into 'b', the name associated with 'b' is
now the name used when taking the first snapshot.

Sadly, older qemu doesn't know how to treat different spellings of
the same filename as identical files (it uses strcmp() instead of
checking for the same inode), which means libvirt's attempt to
commit an image using solely the names learned from qcow2 metadata
fails with a cryptic:

error: internal error: unable to execute QEMU command 'block-commit': Top image 
file /tmp/images/c/../b/b not found

even though the file exists.  Trying to teach libvirt the rules on
which name qemu will expect is not worth the effort (besides, we'd
have to remember it across libvirtd restarts, and track whether a
file was opened via metadata or via snapshot creation for a given
qemu process); it is easier to just always directly ask qemu what
string it expects to see in the first place.

As a safety valve, we validate that any name returned by qemu
still maps to the same local file as we have tracked it, so that
a compromised qemu cannot accidentally cause us to act on an
incorrect file.

* src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New
prototype.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup)
(qemuMonitorJSONDiskNameLookupOne): Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit)
(qemuDomainBlockJobImpl): Use it.

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

v2: as suggested by Dan, add a sanity checking valve to ensure we
don't use qemu's string until vetting that it resolves to the same
local name we are already tracking

 src/qemu/qemu_driver.c   | 28 ++---
 src/qemu/qemu_monitor.c  | 20 -
 src/qemu/qemu_monitor.h  |  8 +++-
 src/qemu/qemu_monitor_json.c | 97 +++-
 src/qemu/qemu_monitor_json.h |  9 +++-
 5 files changed, 144 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3263ac..f0e530d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16132,9 +16132,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 goto endjob;

 if (baseSource) {
-if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
-goto endjob;
-
 if (flags  VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
 if (!virQEMUCapsGet(priv-qemuCaps, 
QEMU_CAPS_CHANGE_BACKING_FILE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -16172,8 +16169,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 }

 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
-  speed, mode, async);
+if (baseSource)
+basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+ baseSource);
+if (!baseSource || basePath)
+ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
+  speed, mode, async);
 if (qemuDomainObjExitMonitor(driver, vm)  0)
 ret = -1;
 if (ret  0) {
@@ -16903,12 +16904,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE)  0))
 goto endjob;

-if (qemuGetDriveSourceString(topSource, NULL, topPath)  0)
-goto endjob;
-
-if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
-goto endjob;
-
 if (flags  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE 
 topSource != disk-src) {
 if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
@@ -16939,9 +16934,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
 disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
 }
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockCommit(priv-mon, device,
- topPath, basePath, backingPath,
- speed);
+basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+ baseSource);
+topPath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src,
+topSource);
+if (basePath  topPath)
+ret = qemuMonitorBlockCommit(priv-mon, device,
+ topPath, 

[libvirt] GSOC 2015: Libvirt Projects

[Richa Sehgal]

Dear Mentors,

I am currently pursuing Master's in University of Illinois at Urbana
Champaign, USA and completed by B.Tech from Indian Institute of Technology
- Delhi (IIT- Delhi).

I am interested in contributing to Libvirt community through GSOC 2015. I
am using VirtualBox to run Ubuntu and have used VMware Player in the past.
Actually I am very interested in understanding virtualization at a more
practical level and do not want to limit myself to theoretical knowledge. I
have played with QEMU during the OS class, but I am new to Libvirt, and
thus would like to start with beginner level projects. There are two that I
found:

1. Enhancing libvirt-designer
2. Abstracting device address allocation

Are these projects still open, or students have already applied to them?
Are there any other projects that I can start with? I am really excited
about contributing to the libvirt project. Please let me know of a suitable
way of starting with these. I look forward for a useful engagement with the
community this summer. My irc name is: richashi.

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

[libvirt] [PATCH] qemu: skip precreation of network disks

[Michael Chapman]

Commit cf54c60699833b3791a5d0eb3eb5a1948c267f6b introduced the ability
to create missing storage volumes during migration. For network disks,
however, we may not necessarily be able to detect whether they already
exist -- there is no straight-forward way to map the disk to a storage
volume, and even if there were it's possible no configured storage pool
actually contains the disk.

It is better to assume the network disk exists in this case, rather than
aborting the migration completely. If the volume really is missing, QEMU
will generate an appropriate error later in the migration.

Signed-off-by: Michael Chapman m...@very.puzzling.org
---
 src/qemu/qemu_migration.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 83be435..ebe8af5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1507,9 +1507,13 @@ qemuMigrationPrecreateDisk(virConnectPtr conn,
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 break;
 
+case VIR_STORAGE_TYPE_NETWORK:
+VIR_DEBUG(Skipping creation of network disk '%s',
+  disk-dst);
+return 0;
+
 case VIR_STORAGE_TYPE_BLOCK:
 case VIR_STORAGE_TYPE_DIR:
-case VIR_STORAGE_TYPE_NETWORK:
 case VIR_STORAGE_TYPE_NONE:
 case VIR_STORAGE_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
-- 
2.1.0

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

Re: [libvirt] Enhancing block/disk migration in libvirt

[Kashyap Chamarthy]

On Tue, Feb 17, 2015 at 07:05:13PM +1100, Tony Breeds wrote:
 On Tue, Feb 17, 2015 at 09:02:57AM +0100, Michal Privoznik wrote:
 
  Oh, I forgot to paste the API name: virDomainMigrate3
 
 That'll narrow the search!  Thanks!

Is there an upstream libvirt bug for this work? If not, can it be filed
with the context from this thread for tracking purpose? I'm asking
because, a few upstream OpenStack Nova folks expressed interest on IRC
to track this work.

URL to file:


https://bugzilla.redhat.com/bugzilla/enter_bug.cgi?product=Virtualization%20Toolscomponent=libvirt

-- 
/kashyap

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

[libvirt] [PATCH] Clarify the meaning of version in redirdev filters

[Ján Tomko]

The version attribute in redirdev filters refers to the revision
of the device, not the version of the USB protocol.

Explicitly state that this is not the USB protocol and remove references
to those round version numbers that resemble USB protocol versions.

https://bugzilla.redhat.com/show_bug.cgi?id=1177237
---
 docs/formatdomain.html.in | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ede20de..40e2b29 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3320,7 +3320,7 @@
   lt;boot order='1'/gt;
 lt;/redirdevgt;
 lt;redirfiltergt;
-  lt;usbdev class='0x08' vendor='0x1234' product='0xbeef' version='2.00' 
allow='yes'/gt;
+  lt;usbdev class='0x08' vendor='0x1234' product='0xbeef' version='2.56' 
allow='yes'/gt;
   lt;usbdev allow='no'/gt;
 lt;/redirfiltergt;
   lt;/devicesgt;
@@ -3363,7 +3363,8 @@
 codeclass/code attribute is the USB Class code, for example,
 0x08 represents mass storage devices. The USB device can be addressed 
by
 vendor / product id using the codevendor/code and 
codeproduct/code attributes.
-codeversion/code is the bcdDevice value of USB device, such as 
1.00, 1.10 and 2.00.
+codeversion/code is the device revision from the bcdDevice field 
(not
+the version of the USB protocol).
 These four attributes are optional and code-1/code can be used to 
allow
 any value for them. codeallow/code attribute is mandatory,
 'yes' means allow, 'no' for deny.
-- 
2.0.5

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

[libvirt] [libvirt-test-API][PATCH V2 0/2] Add connection_getAllDomainStats test case

[jiahu]

The testing case will validate the getAllDomainStats API in class virConnect

V2:
Added new domainListGetStats API in this case

jiahu (2):
  Add connection_getAllDomainStats test case to linux_domain.conf
  Add connection_getAllDomainStats test case

 cases/linux_domain.conf   |  14 +
 repos/virconn/connection_getAllDomainStats.py | 549 ++
 2 files changed, 563 insertions(+)
 create mode 100644 repos/virconn/connection_getAllDomainStats.py

-- 
1.8.1.4

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

[libvirt] [libvirt-test-API][PATCH V2 1/2] Add connection_getAllDomainStats test case to linux_domain.conf

[jiahu]

---
 cases/linux_domain.conf | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 552f001..a2c01fa 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -34,6 +34,20 @@ domain:start
 guestname
 $defaultname
 
+virconn:connection_getAllDomainStats
+stats
+state|cpu|balloon|vcpu|interface|block
+flags
+
active|inactive|persistent|transient|running|paused|shutoff|other|backing|enforce
+
+virconn:connection_getAllDomainStats
+stats
+state|cpu|balloon|vcpu|interface|block
+flags
+backing|enforce
+doms
+$defaultname
+
 domain:securitylabel
 guestname
 $defaultname
-- 
1.8.1.4

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

Re: [libvirt] [libvirt-test-API][PATCH 2/2] Add connection_getAllDomainStats test case

[hujianwei]

On 11/03/15 17:14, hongming wrote:

Hi jiahu

Please add the following api calling to your case, we can test them 
using the same checking methods .


def domainListGetStats(self, doms, stats=0, flags=0):
 Query statistics for given domains.

Thanks
Hongming


OK, the V2 patch series has been sent out, please review them.

BR,
Jianwei

On 03/05/2015 04:02 PM, jiahu wrote:

The case will validate the getAllDomainStats API in class virConnect
---
  repos/virconn/connection_getAllDomainStats.py | 528 
++

  1 file changed, 528 insertions(+)
  create mode 100644 repos/virconn/connection_getAllDomainStats.py

diff --git a/repos/virconn/connection_getAllDomainStats.py 
b/repos/virconn/connection_getAllDomainStats.py

new file mode 100644
index 000..023564a
--- /dev/null
+++ b/repos/virconn/connection_getAllDomainStats.py
@@ -0,0 +1,528 @@
+#!/usr/bin/env python
+# test getAllDomainStats() API for libvirt
+
+import libvirt
+
+from xml.dom import minidom
+from libvirt import libvirtError
+from src import sharedmod
+from utils import utils
+
+required_params = ()
+optional_params = {'stats': '','flags': ''}
+
+ds = {state: libvirt.VIR_DOMAIN_STATS_STATE,
+  cpu: libvirt.VIR_DOMAIN_STATS_CPU_TOTAL,
+  balloon: libvirt.VIR_DOMAIN_STATS_BALLOON,
+  vcpu: libvirt.VIR_DOMAIN_STATS_VCPU,
+  interface: libvirt.VIR_DOMAIN_STATS_INTERFACE,
+  block: libvirt.VIR_DOMAIN_STATS_BLOCK}
+
+fg = {active: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE,
+  inactive: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE,
+  persistent: 
libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT,

+  transient: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT,
+  running: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING,
+  paused: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED,
+  shutoff: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF,
+  other: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER,
+  backing: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING,
+  enforce: 
libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS}

+
+def filer_domains(logger, flags):
+
+   return a filtered domains set
+
+a = set(active_domains(logger))
+d = set(defined_domains(logger))
+if flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT 
and \

+   flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a | d
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT:
+domains = d
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a - d
+else:
+domains = a | d
+if flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE and \
+   flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains = (a | d)
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE:
+domains = a
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains = (d - a)
+else:
+domains = a | d
+return domains
+
+def active_domains(logger):
+
+   return active domains on current uri
+
+NUM = ls /run/libvirt/qemu|grep \.xml\
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(.xml,) for item in output]
+if status == 0:
+logger.debug(Got active domains: %s % output)
+return output
+else:
+logger.debug(Got active domains: %s % output)
+return output
+
+def defined_domains(logger):
+
+   return defined domains on current uri
+
+NUM = ls /etc/libvirt/qemu|grep \.xml\
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(.xml,) for item in output]
+if status == 0:
+logger.debug(Got defined domains: %s % output)
+return output
+else:
+logger.debug(Got defined domains: %s % output)
+return output
+
+def compare_value(logger,op1,op2):
+
+   compare 2 variables value
+
+if op1 != op2:
+logger.debug(Check %s: Fail % op2)
+return False
+else:
+logger.debug(Check %s: Pass % op2)
+return True
+
+def check_vcpu(logger,dom_name,dom_active,dom_eles):
+
+   check vcpu info of given domain
+
+iDOM_XML = /etc/libvirt/qemu/ + dom_name +.xml
+aDOM_XML = /run/libvirt/qemu/ + dom_name +.xml
+if dom_active:
+xml = minidom.parse(aDOM_XML)
+dom = xml.getElementsByTagName('domain')[0]
+vcpu = dom.getElementsByTagName('vcpu')[0]
+vcpu_max = int(vcpu.childNodes[0].data)
+if not vcpu.getAttribute('current'):
+vcpu_cur = vcpu_max
+else:
+vcpu_cur = int(vcpu.getAttribute('current'))
+
+logger.debug(Checking vcpu.current: %d \
+% dom_eles.get(vcpu.current))
+if not compare_value(logger,vcpu_cur, \
+dom_eles.get(vcpu.current)):
+return False
+

[libvirt] [libvirt-test-API][PATCH V2 2/2] Add connection_getAllDomainStats test case

[jiahu]

The case will validate the getAllDomainStats and domainListGetStats
two APIs in class virConnect
---
 repos/virconn/connection_getAllDomainStats.py | 549 ++
 1 file changed, 549 insertions(+)
 create mode 100644 repos/virconn/connection_getAllDomainStats.py

diff --git a/repos/virconn/connection_getAllDomainStats.py 
b/repos/virconn/connection_getAllDomainStats.py
new file mode 100644
index 000..d95004f
--- /dev/null
+++ b/repos/virconn/connection_getAllDomainStats.py
@@ -0,0 +1,549 @@
+#!/usr/bin/env python
+# test getAllDomainStats() API for libvirt
+
+import libvirt
+
+from xml.dom import minidom
+from libvirt import libvirtError
+from src import sharedmod
+from utils import utils
+
+required_params = ()
+optional_params = {'stats': '','flags': '','doms':''}
+
+ds = {state: libvirt.VIR_DOMAIN_STATS_STATE,
+  cpu: libvirt.VIR_DOMAIN_STATS_CPU_TOTAL,
+  balloon: libvirt.VIR_DOMAIN_STATS_BALLOON,
+  vcpu: libvirt.VIR_DOMAIN_STATS_VCPU,
+  interface: libvirt.VIR_DOMAIN_STATS_INTERFACE,
+  block: libvirt.VIR_DOMAIN_STATS_BLOCK}
+
+fg = {active: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE,
+  inactive: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE,
+  persistent: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT,
+  transient: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT,
+  running: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING,
+  paused: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED,
+  shutoff: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF,
+  other: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER,
+  backing: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING,
+  enforce: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS}
+
+def filer_domains(logger, flags):
+
+   return a filtered domains set
+
+a = set(active_domains(logger))
+d = set(defined_domains(logger))
+if flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT and \
+   flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a | d
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT:
+domains = d
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a - d
+else:
+domains = a | d
+if flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE and \
+   flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains = (a | d)
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE:
+domains = a
+elif flags  libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains = (d - a)
+else:
+domains = a | d
+return domains
+
+def active_domains(logger):
+
+   return active domains on current uri
+
+NUM = ls /run/libvirt/qemu|grep \.xml\
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(.xml,) for item in output]
+if status == 0:
+logger.debug(Got active domains: %s % output)
+return output
+else:
+logger.debug(Got active domains: %s % output)
+return output
+
+def defined_domains(logger):
+
+   return defined domains on current uri
+
+NUM = ls /etc/libvirt/qemu|grep \.xml\
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(.xml,) for item in output]
+if status == 0:
+logger.debug(Got defined domains: %s % output)
+return output
+else:
+logger.debug(Got defined domains: %s % output)
+return output
+
+def compare_value(logger,op1,op2):
+
+   compare 2 variables value
+
+if op1 != op2:
+logger.debug(Check %s: Fail % op2)
+return False
+else:
+logger.debug(Check %s: Pass % op2)
+return True
+
+def check_vcpu(logger,dom_name,dom_active,dom_eles):
+
+   check vcpu info of given domain
+
+iDOM_XML = /etc/libvirt/qemu/ + dom_name +.xml
+aDOM_XML = /run/libvirt/qemu/ + dom_name +.xml
+if dom_active:
+xml = minidom.parse(aDOM_XML)
+dom = xml.getElementsByTagName('domain')[0]
+vcpu = dom.getElementsByTagName('vcpu')[0]
+vcpu_max = int(vcpu.childNodes[0].data)
+if not vcpu.getAttribute('current'):
+vcpu_cur = vcpu_max
+else:
+vcpu_cur = int(vcpu.getAttribute('current'))
+
+logger.debug(Checking vcpu.current: %d \
+% dom_eles.get(vcpu.current))
+if not compare_value(logger,vcpu_cur, \
+dom_eles.get(vcpu.current)):
+return False
+logger.debug(Checking vcpu.maximum: %d \
+% dom_eles.get(vcpu.maximum))
+if not compare_value(logger,vcpu_max, \
+dom_eles.get(vcpu.maximum)):
+return False
+else:
+xml = minidom.parse(iDOM_XML)
+vcpu = xml.getElementsByTagName('vcpu')[0]
+vcpu_max = 

Re: [libvirt] Applying for GSoC project 'Introducing job control to the storage driver' in libvirt

[Michal Privoznik]

On 09.03.2015 07:29, Taowei Luo wrote:
 Hello, everyone.
 

Hey!

 I'm Taowei Luo. A participator in Google Summer of Code last year for
 the project rewriting vbox driver in libvirt. I am so glad that my
 project isn't found in this year's GSoC project list :)
 
 Having a nice experience last year, libvirt is my top option to join
 the GSoC 2015. And my interesting project is “Introducing job control
 to the storage driver”.

It's nice to see people returning.

 
 I have read some materials about this project. Including I noticed
 that Tucker had attempted it last year. It seems he didn't make it
 through the final evaluation. During his working, some discussion were
 made in mail-list. It is really inspiring to me. So I have some basic
 ideas for now.
 
 The project requires job control functions (at least, reporting the
 progress) in storage driver. Obviously, it contains two part. First
 find codes that really do the storage work which may take a long
 period and can be asynchronized. Then, extract it to the job control
 part so make it under the control. It would not be hard to find codes
 that really need asynchronization. Maybe by dumping the debug message
 and tracing the function calls. So the big part is design the APIs for
 job control. I think the goal for API design is not only make it
 workable but also make it reasonable and extendable.

The idea is to turn qemuDomainObj*Job* a separate module, that would
work over virObject. Subsequently, we can add BeginJob and EndJob APIs
into storage driver too. Moreover, places where libvirt monitors
progress of a storage operation, we can fill in jobInfo strucut
(although, that one in qemu is not generic enough I guess, so maybe we
need to invent a new one). Then, CancelJob API can be implemented in
storage driver too.

 
 To achieve this, a lot of details were discussed in last year's
 proceeding. I summered it and add my own opinion.
 
 Parallel jobs: The idea result is that several jobs work in parallel
 and libvirt monitors and controls it. There are two ways for parallel:
 thread and process. I prefer process. In process, we can easier
 implement the idea of *control* by signal. Process has better
 independence than thread. What's more, it is a low coupling design.

Libvirt already uses that. virCommandExec* is to be find all over the
storage driver.

 
 Synchronization: process can use system level lock to make sure it
 obtain the resources. If the process can't obtain it could exit with
 failure (or wait). In process, we can leave most resource competition
 handling by OS. If thread is used instead, we need to think about
 resource competition between libvirt and other process, and at the
 same time, those competitions in libvirt thread.
 
 Management: We execute those asynchronous codes in a new process. In
 libvirt, it invokes those processes with parsed arguments. Libvirtd
 would have a process pool to store the pids and some attached
 information for each process. Signal would be used to communicate
 between process and libvirt.
 
 Expandability: Some other jobs like domain migration could be in
 implemented under this design. It's all about creating new jobs with
 parsed arguments, which tells the child process what to do and what
 resources they need.
 
 Privacy: If new jobs are created in process, user may access the
 process directly and not noticing libvirt. Function sigaction(), which
 provides the pid of sending process, could be used to register
 responding functions. Meanwhile, atexit() register functions that
 execute when the process is going to exit. It is helpful on notifying
 libvirtd the end of the job.

This is already handled by virCommand subsystem.

 
 What already have: Besides the discussion, some other resources would
 be helpful for this project. qemu has a prototype for job control in
 migration. We already have gnulib and tools in util/virprocess.h and
 util/virthread.h to achieve parallel.
 
 When undertaking, I would firstly implement the job control part. It
 would have some basic functions. Make one storage API work in it.
 Then, adding job control support for the rest APIs.
 
 This is all about what I came up with. Maybe those ideas are old and
 repeating. But I think it is a workable plan. Waiting for feedback.

Majority looks reasonable to me.

Michal

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

[libvirt] [PATCH 05/12] qemu: monitor: Implement HMP version for listing all block device stats

[Peter Krempa]

Add a different version of parser for info blockstats that basically
parses the same information as the existing copy of the function.

This will allow us to remove the single device version
qemuMonitorGetBlockStatsInfo in the future.

The new implementation uses few new helpers so it should be more
understandable and provides a test case to verify that it works.
---
 src/qemu/qemu_monitor.c  |  16 +-
 src/qemu/qemu_monitor_text.c | 129 +++
 src/qemu/qemu_monitor_text.h |   3 +
 tests/Makefile.am|   8 ++-
 tests/qemumonitortest.c  |  89 -
 5 files changed, 240 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 95a6989..149e743 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1867,8 +1867,20 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
 if (!(*ret_stats = virHashCreate(10, virHashValueFree)))
 goto error;

-if (qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain)  0)
-goto error;
+if (mon-json) {
+if (qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain) 
 0)
+goto error;
+} else {
+ if (backingChain) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+_(text monitor doesn't support block stats for 
+  backing chain members));
+ goto error;
+ }
+
+ if (qemuMonitorTextGetAllBlockStatsInfo(mon, *ret_stats)  0)
+ goto error;
+}

 return 0;

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 2de281f..8b2ef90 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -838,6 +838,135 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon,
 return ret;
 }

+
+int
+qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon,
+virHashTablePtr hash)
+{
+qemuBlockStatsPtr stats = NULL;
+char *info = NULL;
+char *dev_name;
+char **lines = NULL;
+char **values = NULL;
+char *line;
+char *value;
+char *key;
+size_t i;
+size_t j;
+int ret = -1;
+
+if (qemuMonitorHMPCommand(mon, info blockstats, info)  0)
+goto cleanup;
+
+/* If the command isn't supported then qemu prints the supported info
+ * commands, so the output starts info .  Since this is unlikely to be
+ * the name of a block device, we can use this to detect if qemu supports
+ * the command. */
+if (strstr(info, \ninfo )) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _('info blockstats' not supported by this qemu));
+goto cleanup;
+}
+
+/* The output format for both qemu  KVM is:
+ *   blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=%
+ *   (repeated for each block device)
+ * where '%' is a 64 bit number.
+ */
+if (!(lines = virStringSplit(info, \n, 0)))
+goto cleanup;
+
+for (i = 0; lines[i]  *lines[i]; i++) {
+line = lines[i];
+
+if (VIR_ALLOC(stats)  0)
+goto cleanup;
+
+/* set the entries to -1, the JSON monitor enforces them, but it would
+ * be overly complex to achieve this here */
+stats-rd_req = -1;
+stats-rd_bytes = -1;
+stats-wr_req = -1;
+stats-wr_bytes = -1;
+stats-rd_total_times = -1;
+stats-wr_total_times = -1;
+stats-flush_req = -1;
+stats-flush_total_times = -1;
+
+/* extract device name and make sure that it's followed by
+ * a colon and space */
+dev_name = line;
+if (!(line = strchr(line, ':'))  line[1] != ' ') {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(info blockstats reply was malformed));
+goto cleanup;
+}
+
+*line = '\0';
+line += 2;
+
+if (STRPREFIX(dev_name, QEMU_DRIVE_HOST_PREFIX))
+dev_name += strlen(QEMU_DRIVE_HOST_PREFIX);
+
+if (!(values = virStringSplit(line,  , 0)))
+goto cleanup;
+
+for (j = 0; values[j]  *values[j]; j++) {
+key = values[j];
+
+if (!(value = strchr(key, '='))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(info blockstats entry was malformed));
+goto cleanup;
+}
+
+*value = '\0';
+value++;
+
+#define QEMU_MONITOR_TEXT_READ_BLOCK_STAT(NAME, VAR)   
\
+if (STREQ(key, NAME)) {
\
+if (virStrToLong_ll(value, NULL, 10, VAR)  0) {  
\
+virReportError(VIR_ERR_INTERNAL_ERROR, 
\
+   _('info blockstats' contains malformed
\
+   

[libvirt] [PATCH 12/12] qemu: monitor: Kill qemuMonitorGetBlockStats(Info, ParamsNumber)

[Peter Krempa]

The functions and their QMP and HMP implementations are no longer needed
since everything is now done via the *AllStats functions.
---
 src/qemu/qemu_monitor.c  |  62 --
 src/qemu/qemu_monitor.h  |  14 ---
 src/qemu/qemu_monitor_json.c | 135 -
 src/qemu/qemu_monitor_json.h |  12 ---
 src/qemu/qemu_monitor_text.c | 198 ---
 src/qemu/qemu_monitor_text.h |  12 ---
 6 files changed, 433 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e4ff06e..4bdd8d8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1808,45 +1808,6 @@ qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo,
 return info;
 }

-int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
- const char *dev_name,
- long long *rd_req,
- long long *rd_bytes,
- long long *rd_total_times,
- long long *wr_req,
- long long *wr_bytes,
- long long *wr_total_times,
- long long *flush_req,
- long long *flush_total_times)
-{
-int ret;
-VIR_DEBUG(mon=%p dev=%s, mon, dev_name);
-
-if (!mon) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(monitor must not be NULL));
-return -1;
-}
-
-if (mon-json)
-ret = qemuMonitorJSONGetBlockStatsInfo(mon, dev_name,
-   rd_req, rd_bytes,
-   rd_total_times,
-   wr_req, wr_bytes,
-   wr_total_times,
-   flush_req,
-   flush_total_times);
-else
-ret = qemuMonitorTextGetBlockStatsInfo(mon, dev_name,
-   rd_req, rd_bytes,
-   rd_total_times,
-   wr_req, wr_bytes,
-   wr_total_times,
-   flush_req,
-   flush_total_times);
-return ret;
-}
-

 /**
  * qemuMonitorGetAllBlockStatsInfo:
@@ -1921,29 +1882,6 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
 }


-/* Return 0 and update @nparams with the number of block stats
- * QEMU supports if success. Return -1 if failure.
- */
-int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon,
- int *nparams)
-{
-int ret;
-VIR_DEBUG(mon=%p nparams=%p, mon, nparams);
-
-if (!mon) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(monitor must not be NULL));
-return -1;
-}
-
-if (mon-json)
-ret = qemuMonitorJSONGetBlockStatsParamsNumber(mon, nparams);
-else
-ret = qemuMonitorTextGetBlockStatsParamsNumber(mon, nparams);
-
-return ret;
-}
-
 int qemuMonitorGetBlockExtent(qemuMonitorPtr mon,
   const char *dev_name,
   unsigned long long *extent)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 72498b3..b30da34 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -357,17 +357,6 @@ struct qemuDomainDiskInfo *
 qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo,
const char *dev_name);

-int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
- const char *dev_name,
- long long *rd_req,
- long long *rd_bytes,
- long long *rd_total_times,
- long long *wr_req,
- long long *wr_bytes,
- long long *wr_total_times,
- long long *flush_req,
- long long *flush_total_times);
-
 typedef struct _qemuBlockStats qemuBlockStats;
 typedef qemuBlockStats *qemuBlockStatsPtr;
 struct _qemuBlockStats {
@@ -394,9 +383,6 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
 bool backingChain)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

-int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon,
- int *nparams);
-
 int qemuMonitorGetBlockExtent(qemuMonitorPtr mon,
   const char *dev_name,
   unsigned long long *extent);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 76baaf6..0f32a0a 100644
--- 

[libvirt] [PATCH 02/12] qemu: monitor: Drop parsing of 'errs' from block info

[Peter Krempa]

The error count statistic is not supported by qemu, so there's no need
to pass the variables around if the result is ignored anyways.
---
 src/qemu/qemu_driver.c   | 11 ++-
 src/qemu/qemu_monitor.c  |  9 +++--
 src/qemu/qemu_monitor.h  |  3 +--
 src/qemu/qemu_monitor_json.c |  6 ++
 src/qemu/qemu_monitor_json.h |  3 +--
 src/qemu/qemu_monitor_text.c |  5 ++---
 src/qemu/qemu_monitor_text.h |  3 +--
 tests/qemumonitorjsontest.c  | 19 +--
 8 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e94275f..06168b1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10536,6 +10536,9 @@ qemuDomainBlockStats(virDomainPtr dom,

 priv = vm-privateData;

+/* qemu doesn't report the error count */
+stats-errs = -1;
+
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorGetBlockStatsInfo(priv-mon,
diskAlias,
@@ -10546,8 +10549,7 @@ qemuDomainBlockStats(virDomainPtr dom,
stats-wr_bytes,
NULL,
NULL,
-   NULL,
-   stats-errs);
+   NULL);
 if (qemuDomainObjExitMonitor(driver, vm)  0)
 ret = -1;

@@ -10574,7 +10576,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv;
 long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times;
-long long wr_total_times, flush_req, flush_total_times, errs;
+long long wr_total_times, flush_req, flush_total_times;
 char *diskAlias = NULL;

 virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -10643,8 +10645,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
wr_bytes,
wr_total_times,
flush_req,
-   flush_total_times,
-   errs);
+   flush_total_times);

 if (qemuDomainObjExitMonitor(driver, vm)  0)
 ret = -1;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 94495cd..24e87b7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1817,8 +1817,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
  long long *wr_bytes,
  long long *wr_total_times,
  long long *flush_req,
- long long *flush_total_times,
- long long *errs)
+ long long *flush_total_times)
 {
 int ret;
 VIR_DEBUG(mon=%p dev=%s, mon, dev_name);
@@ -1836,8 +1835,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
wr_req, wr_bytes,
wr_total_times,
flush_req,
-   flush_total_times,
-   errs);
+   flush_total_times);
 else
 ret = qemuMonitorTextGetBlockStatsInfo(mon, dev_name,
rd_req, rd_bytes,
@@ -1845,8 +1843,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
wr_req, wr_bytes,
wr_total_times,
flush_req,
-   flush_total_times,
-   errs);
+   flush_total_times);
 return ret;
 }

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 133d42d..72498b3 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -366,8 +366,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
  long long *wr_bytes,
  long long *wr_total_times,
  long long *flush_req,
- long long *flush_total_times,
- long long *errs);
+ long long *flush_total_times);

 typedef struct _qemuBlockStats qemuBlockStats;
 typedef qemuBlockStats *qemuBlockStatsPtr;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 832f589..612553b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1677,15 +1677,14 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
  long long *wr_bytes,
  

Re: [libvirt] [PATCH] Introduce virBitmapIsBitSet

[Martin Kletzander]

On Wed, Mar 11, 2015 at 07:34:22PM +0100, Ján Tomko wrote:

A helper that never returns an error and treats bits out of bitmap range
as false.

Use it everywhere we use ignore_value on virBitmapGetBit, or loop over
the bitmap size.
---
src/conf/domain_conf.c|  4 +---
src/conf/node_device_conf.c   |  4 +---
src/conf/snapshot_conf.c  |  6 ++
src/conf/storage_conf.c   |  4 +---
src/libvirt_private.syms  |  1 +
src/libxl/libxl_domain.c  |  4 +---
src/libxl/libxl_driver.c  |  5 +
src/nodeinfo.c|  6 +-
src/qemu/qemu_capabilities.c  |  7 +--
src/storage/storage_backend.c |  4 +---
src/util/virbitmap.c  | 18 ++
src/util/virbitmap.h  |  5 +
src/util/vircgroup.c  |  4 +---
src/util/virdnsmasq.c |  6 +-
src/util/virportallocator.c   | 17 +++--
src/util/virprocess.c |  9 ++---
src/xen/xen_driver.c  |  4 +---
src/xen/xend_internal.c   |  5 +
18 files changed, 43 insertions(+), 70 deletions(-)



Nice cleanup, ACK.


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

[libvirt] [PATCH v3 05/15] test_driver: Use virNetworkObjEndAPI

[Michal Privoznik]

So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/test/test_driver.c | 42 ++
 1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 20c77de..72f40ed 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3507,8 +3507,7 @@ static virNetworkPtr 
testNetworkLookupByUUID(virConnectPtr conn,
 ret = virGetNetwork(conn, net-def-name, net-def-uuid);
 
  cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 return ret;
 }
 
@@ -3531,8 +3530,7 @@ static virNetworkPtr 
testNetworkLookupByName(virConnectPtr conn,
 ret = virGetNetwork(conn, net-def-name, net-def-uuid);
 
  cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 return ret;
 }
 
@@ -3620,8 +3618,7 @@ static int testNetworkIsActive(virNetworkPtr net)
 ret = virNetworkObjIsActive(obj);
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
 return ret;
 }
 
@@ -3641,8 +3638,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
 ret = obj-persistent;
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
 return ret;
 }
 
@@ -3674,8 +3670,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr 
conn, const char *xml)
 virNetworkDefFree(def);
 if (event)
 testObjectEventQueue(privconn, event);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 testDriverUnlock(privconn);
 return ret;
 }
@@ -3707,8 +3702,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, 
const char *xml)
 virNetworkDefFree(def);
 if (event)
 testObjectEventQueue(privconn, event);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 testDriverUnlock(privconn);
 return ret;
 }
@@ -3745,8 +3739,7 @@ static int testNetworkUndefine(virNetworkPtr network)
  cleanup:
 if (event)
 testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 testDriverUnlock(privconn);
 return ret;
 }
@@ -3795,8 +3788,7 @@ testNetworkUpdate(virNetworkPtr net,
 
 ret = 0;
  cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
 testDriverUnlock(privconn);
 return ret;
 }
@@ -3832,8 +3824,7 @@ static int testNetworkCreate(virNetworkPtr network)
  cleanup:
 if (event)
 testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 return ret;
 }
 
@@ -3865,8 +3856,7 @@ static int testNetworkDestroy(virNetworkPtr network)
  cleanup:
 if (event)
 testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 testDriverUnlock(privconn);
 return ret;
 }
@@ -3892,8 +3882,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network,
 ret = virNetworkDefFormat(privnet-def, flags);
 
  cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 return ret;
 }
 
@@ -3921,8 +3910,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr 
network) {
 ignore_value(VIR_STRDUP(bridge, privnet-def-bridge));
 
  cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 return bridge;
 }
 
@@ -3946,8 +3934,7 @@ static int testNetworkGetAutostart(virNetworkPtr network,
 ret = 0;
 
  cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 return ret;
 }
 
@@ -3971,8 +3958,7 @@ static int testNetworkSetAutostart(virNetworkPtr network,
 ret = 0;
 
  cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(privnet);
 return ret;
 }
 
-- 
2.0.5

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

[libvirt] [PATCH v3 03/15] network_conf: Introduce virNetworkObjEndAPI

[Michal Privoznik]

This is practically copy of qemuDomObjEndAPI. The reason why is
it so widely available is to avoid code duplication, since the
function is going to be called from our bridge driver, test
driver and parallels driver too.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/network_conf.c  | 20 ++--
 src/conf/network_conf.h  |  1 +
 src/libvirt_private.syms |  1 +
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 609990c..f677e3c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -136,6 +136,16 @@ virNetworkObjNew(void)
 return NULL;
 }
 
+void
+virNetworkObjEndAPI(virNetworkObjPtr *net)
+{
+if (!*net)
+return;
+
+virObjectUnlock(*net);
+*net = NULL;
+}
+
 virNetworkObjListPtr virNetworkObjListNew(void)
 {
 virNetworkObjListPtr nets;
@@ -3041,8 +3051,8 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
 if (!virFileStripSuffix(entry-d_name, .xml))
 continue;
 
-if ((net = virNetworkLoadState(nets, stateDir, entry-d_name)))
-virObjectUnlock(net);
+net = virNetworkLoadState(nets, stateDir, entry-d_name);
+virNetworkObjEndAPI(net);
 }
 
 closedir(dir);
@@ -3082,8 +3092,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
configDir,
autostartDir,
entry-d_name);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 }
 
 closedir(dir);
@@ -4266,8 +4275,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
 }
 
  cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(net);
 return ret;
 }
 
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 1423676..e0ed714 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -276,6 +276,7 @@ struct _virNetworkObj {
 };
 
 virNetworkObjPtr virNetworkObjNew(void);
+void virNetworkObjEndAPI(virNetworkObjPtr *net);
 
 typedef struct _virNetworkObjList virNetworkObjList;
 typedef virNetworkObjList *virNetworkObjListPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5e78cf7..5fbe094 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -563,6 +563,7 @@ virNetworkIpDefPrefix;
 virNetworkLoadAllConfigs;
 virNetworkLoadAllState;
 virNetworkObjAssignDef;
+virNetworkObjEndAPI;
 virNetworkObjFindByName;
 virNetworkObjFindByUUID;
 virNetworkObjGetPersistentDef;
-- 
2.0.5

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