[libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 04545fd..7f8a7ce 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11337,4 +11337,6 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) for (i = 0; i info-ndevAlias; i++) VIR_FREE(info-devAlias[i]); VIR_FREE(info-devAlias); + +VIR_FREE(info); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote: in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) This does fix the memory leak and makes the function behave like it's documented in virDomainGetFSInfo and virDomainFSInfoFree: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree But it changes the public API - if there are applications that already work around this function by freeing the info, this change would introduce a double free. I would NACK this if the documentation for both APIs didn't say that's how this function should behave. I'd like to hear a second opinion. Jan diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 04545fd..7f8a7ce 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11337,4 +11337,6 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) for (i = 0; i info-ndevAlias; i++) VIR_FREE(info-devAlias[i]); VIR_FREE(info-devAlias); + +VIR_FREE(info); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/4] util: introduce 2 new helpers for get interface IPv6 address
On 03/10/2015 07:50 AM, John Ferlan wrote:
On 03/09/2015 11:37 AM, Luyao Huang wrote:
On 03/09/2015 07:50 AM, John Ferlan wrote:
On 02/28/2015 04:08 AM, Luyao Huang wrote:
Introduce virNetDevGetIfaddrsAddress to help to get interface IPv6
and IPv4 address.
Introduce virNetDevGetIPAddress to use virNetDevGetIfaddrsAddress
and virNetDevGetIPv4Address to get IP address.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
v2: add a new function virNetDevGetIPAddress to call
virNetDevGetIfaddrsAddress
, and make virNetDevGetIfaddrsAddress can get ipv4 address for a
interface.
Also add a error output in virNetDevGetIfaddrsAddress when get
multiple ip
address for a host interface.
src/libvirt_private.syms | 1 +
src/util/virnetdev.c | 98
src/util/virnetdev.h | 4 ++
3 files changed, 103 insertions(+)
...
+if (ifa-ifa_addr-sa_family == (want_ipv6 ? AF_INET6 :
AF_INET)) {
+if (++nIPaddr 1)
+break;
[1]... hmm not sure about this...
+if (want_ipv6) {
+addr-len = sizeof(addr-data.inet6);
+memcpy(addr-data.inet6, ifa-ifa_addr, addr-len);
+} else {
+addr-len = sizeof(addr-data.inet4);
+memcpy(addr-data.inet4, ifa-ifa_addr, addr-len);
+}
+addr-data.stor.ss_family = ifa-ifa_addr-sa_family;
+}
+}
+
+if (nIPaddr == 1)
+ret = 0;
+else if (nIPaddr 1)
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Interface '%s' has multiple %s address),
s/address/addresses
+ ifname, want_ipv6 ? IPv6 : IPv4);
[1]
But is this really desired... It seems from the previous review, if
someone wants a specific address they use listen type='address'
Otherwise, they use this function. Since you'll be returning either an
IPv4 or IPv6 address here you'd be causing an error here if the network
had more than one IPv4 address defined; whereas, the previous code just
returned the first from the ioctl(SIOCGIFADDR...).
I think once you have an address you just return the first one found and
document it that way avoiding this path completely. You could also note
that if you want a specific address use the type='address'
I had an idea but my eyes almost close ;) so i write here without test
them.
I think it will be better if we make this function to get mutli address
and return the number of address we get, like this:
+static int
+virNetDevGetIfaddrsAddress(const char *ifname,
+ bool want_ipv6,
+ virSocketAddrPtr *addrs)
We'd need to have an naddrs to know how many we can stuff in... or you'd
need to do the *REALLOC on the fly in this module for each address found.
Yes, i forgot this and please ignore the code, it has so many mistake
(so it is not a good idea to write a patch when you want sleep:-( )
Interesting idea, but you'd be just throwing things away. I could
perhaps having some code periodically cache addresses... or cache
addresses, subscribe to some event to let you know when a new address is
generated so you can add it to your list (if there is one)...
But, how do you use it?
Sorry, i don't know what these words' (how do you use it ?) mean.
I guess your mean is ask me how to use the code or function you
mentioned, i think maybe
we can avoid to use it :)
However this should be another patch which add a function to get a list
of ip address.
John
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't fill in nicindexes for session mode libvirtd
On 03/10/2015 04:39 AM, Richard W.M. Jones wrote:
On Tue, Mar 10, 2015 at 02:32:04AM -0400, Laine Stump wrote:
Commit 4bbe1029f fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the niceindex array would be filled in for macvtap as well as tap
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by if
(cfg-privileged). The result was that the fixed libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:
Unable to open control socket: Operation not permitted
To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg-privileged.
This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244
---
If someone (Rich?) needs this pushed before I am awake, please feel
free to push it. (also push to the 1.2.13-maint branch if you do)
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1510797..3d1483e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7861,6 +7861,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
int actualType = virDomainNetGetActualType(net);
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7936,7 +7937,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* network and bridge use a tap device, and direct uses a
* macvtap device
*/
-if (nicindexes nnicindexes net-ifname) {
+if (cfg-privileged nicindexes nnicindexes net-ifname) {
if (virNetDevGetIndex(net-ifname, nicindex) 0 ||
VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) 0)
goto cleanup;
Looks sensible, ACK.
As Laine says, please make sure this gets into 1.2.13-maint because it
currently affects all 1.2.13 / Rawhide users.
I just pushed it to master and to v1.2.13-maint.
Also, why isn't there a regression test that would have picked this
up? A trivial reproducer is:
$ guestfish -a /dev/null --network run
but any test case that launches a guest with a network interface as
non-root would have caught this.
This fails only if a tap device is used, which requires a properly
configured qemu-bridge-helper and a bridge device with a particular name
(and that no other session mode libvirtd be currently running for the
user running the test), and we can't require that for the unit tests in
make check, so it would need to be done in the tck tests.
--
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 freepages test cases
Add freepages test cases. V2: Assign ps.strip().upper() to ps variable in free_page.py Jincheng Miao (2): Add freepage test Add freepage test case to test_connection.conf cases/test_connection.conf |6 +++ repos/virconn/free_pages.py | 97 +++ 2 files changed, 103 insertions(+), 0 deletions(-) create mode 100644 repos/virconn/free_pages.py -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH V2 2/2] Add freepage test case to test_connection.conf
Signed-off-by: Jincheng Miao jm...@redhat.com --- cases/test_connection.conf |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/cases/test_connection.conf b/cases/test_connection.conf index ccde119..4211511 100644 --- a/cases/test_connection.conf +++ b/cases/test_connection.conf @@ -29,3 +29,9 @@ virconn:connection_nodeinfo virconn:connection_version conn lxc:/// + +virconn:free_pages +cellid +0 +pagesize +4k,2M -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 2/2] Add test cases for linux_domain
Signed-off-by: Jincheng Miao jm...@redhat.com --- cases/linux_domain.conf | 26 ++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index 515858a..5e63de5 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -139,6 +139,32 @@ domain:cpu_affinity vcpu $defaultvcpu +domain:guest_time +guestname +$defaultname +username +$username +userpassword +$password + +domain:set_guest_time +guestname +$defaultname +username +$username +userpassword +$password + +domain:set_guest_time +guestname +$defaultname +username +$username +userpassword +$password +flags +sync + domain:destroy guestname $defaultname -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 0/2] Add test cases for setTime and getTime
create some test cases for libvirt-python API setTime and getTime Jincheng Miao (2): Add guest setTime and getTime testing Add test cases for linux_domain cases/linux_domain.conf| 26 + repos/domain/guest_time.py | 98 repos/domain/set_guest_time.py | 122 3 files changed, 246 insertions(+), 0 deletions(-) create mode 100644 repos/domain/guest_time.py create mode 100644 repos/domain/set_guest_time.py -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virnet*: Don't unlock object in dispose
As of bba93d40 all of our RPC objects are derived from virObjectLockable. However, during rewrite some errors sneaked in. For instance, the dispose functions to virNetClient and virNetServerClient objects were not only freeing allocated memory, but unlocking themselves. This is wrong. Object should never disappear while locked. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/rpc/virnetclient.c | 2 -- src/rpc/virnetserverclient.c | 1 - 2 files changed, 3 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index d7455b5..7fca055 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -622,8 +622,6 @@ void virNetClientDispose(void *obj) #endif virNetMessageClear(client-msg); - -virObjectUnlock(client); } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index b2a4fdf..f5259c2 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -866,7 +866,6 @@ void virNetServerClientDispose(void *obj) virObjectUnref(client-tlsCtxt); #endif virObjectUnref(client-sock); -virObjectUnlock(client); } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
On Tue, Mar 10, 2015 at 10:32:04AM +0100, Ján Tomko wrote: On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote: in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) This does fix the memory leak and makes the function behave like it's documented in virDomainGetFSInfo and virDomainFSInfoFree: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree But it changes the public API - if there are applications that already work around this function by freeing the info, this change would introduce a double free. Well, this is a good point that we might break somebody's application, but the virDomainFSInfoFree is the only one *Free API that doesn't call free/unref on passed argument. In the documentation the @info is an array and we specifically says that calling virDomainFSInfoFree on each element and then just free the @info is sufficient, but it isn't and there will be a memory leak. It changes behavior of public API to correctly free the allocated memory as documentation says. Let's hear another opinion, but I'm for fixing it. Pavel I would NACK this if the documentation for both APIs didn't say that's how this function should behave. I'd like to hear a second opinion. Jan diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 04545fd..7f8a7ce 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11337,4 +11337,6 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) for (i = 0; i info-ndevAlias; i++) VIR_FREE(info-devAlias[i]); VIR_FREE(info-devAlias); + +VIR_FREE(info); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API][PATCH V2 1/2] Add freepage test
ACK
Please pay attention to below format next time.
Applying: Add freepage test
/root/libvirt-test-API/.git/rebase-apply/patch:88: trailing whitespace.
/root/libvirt-test-API/.git/rebase-apply/patch:90: trailing whitespace.
/root/libvirt-test-API/.git/rebase-apply/patch:95: trailing whitespace.
warning: 3 lines add whitespace errors.
BR,
Jianwei
On 03/10/2015 05:29 PM, Jincheng Miao wrote:
For system default pagesize, it's hard to calculate,
and it changes all the time, so just skip it. For others,
reading from sysfs to get free pages.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
repos/virconn/free_pages.py | 97 +++
1 files changed, 97 insertions(+), 0 deletions(-)
create mode 100644 repos/virconn/free_pages.py
diff --git a/repos/virconn/free_pages.py b/repos/virconn/free_pages.py
new file mode 100644
index 000..516b9f2
--- /dev/null
+++ b/repos/virconn/free_pages.py
@@ -0,0 +1,97 @@
+#!/usr/bin/env python
+# test libvirt free pages
+
+import os
+import resource
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('cellid', 'pagesize',)
+optional_params = {}
+
+HUGEPAGE_PATH =
'/sys/devices/system/node/node%s/hugepages/hugepages-%skB/free_hugepages'
+
+def parse_unit(pagesz):
+ parse a integer value, its unit is KiB
+
+val = int(pagesz[0:len(pagesz)-1])
+unit = pagesz[-1]
+if unit == 'K':
+unit = 1
+elif unit == 'M':
+unit = 1024
+elif unit == 'G':
+unit = 1024*1024
+else:
+return None
+
+return val * unit
+
+def parse_page_list(pagesize):
+ parse page size
+
+if pagesize == None:
+return None
+
+l = list()
+for ps in pagesize.split(','):
+ps = ps.strip().upper()
+val = parse_unit(ps)
+if val == None:
+return None
+l.append(val)
+return l
+
+def check_free_pages(page_list, cell_id, free_page, logger):
+ check page size
+
+for ps in page_list:
+# if pagesize is equal to system pagesize, since it is hard to
+# calculate, so we just pass it
+if resource.getpagesize()/1024 == ps:
+logger.info(skip to check default %sKB-page % ps)
+continue
+
+sysfs_path = HUGEPAGE_PATH % (cell_id, ps)
+if not os.access(sysfs_path, os.R_OK):
+logger.error(could not find %s % sysfs_path)
+return False
+f= open(sysfs_path)
+fp = int(f.read())
+f.close()
+if not fp == free_page[0][ps]:
+logger.error(Free %sKB page checking failed % ps)
+return False
+logger.info(Free %sKB page: %s % (ps, fp))
+
+return True
+
+def free_pages(params):
+ test libvirt free pages
+
+logger = params['logger']
+cell_id = int(params['cellid'])
+
+conn = sharedmod.libvirtobj['conn']
+
+page_list = parse_page_list(params['pagesize'])
+if page_list == None:
+logger.error(pagesize could not be recognized)
+return 1
+
+try:
+free_page = conn.getFreePages(page_list, cell_id, 1)
+
+if check_free_pages(page_list, cell_id, free_page, logger):
+logger.info(Success to check free page)
+else:
+logger.error(Failed to check free page)
+return 1
+except libvirtError, e:
+logger.error(API error message: %s, error code is %s %
+ e.message)
+return 1
+return 0
\ No newline at end of file
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 1/2] Add guest setTime and getTime testing
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()
+time.sleep(90)
+
+# get guest MAC
+mac =
[libvirt] [libvirt-test-API][PATCH V2 1/2] Add freepage test
For system default pagesize, it's hard to calculate,
and it changes all the time, so just skip it. For others,
reading from sysfs to get free pages.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
repos/virconn/free_pages.py | 97 +++
1 files changed, 97 insertions(+), 0 deletions(-)
create mode 100644 repos/virconn/free_pages.py
diff --git a/repos/virconn/free_pages.py b/repos/virconn/free_pages.py
new file mode 100644
index 000..516b9f2
--- /dev/null
+++ b/repos/virconn/free_pages.py
@@ -0,0 +1,97 @@
+#!/usr/bin/env python
+# test libvirt free pages
+
+import os
+import resource
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('cellid', 'pagesize',)
+optional_params = {}
+
+HUGEPAGE_PATH =
'/sys/devices/system/node/node%s/hugepages/hugepages-%skB/free_hugepages'
+
+def parse_unit(pagesz):
+ parse a integer value, its unit is KiB
+
+val = int(pagesz[0:len(pagesz)-1])
+unit = pagesz[-1]
+if unit == 'K':
+unit = 1
+elif unit == 'M':
+unit = 1024
+elif unit == 'G':
+unit = 1024*1024
+else:
+return None
+
+return val * unit
+
+def parse_page_list(pagesize):
+ parse page size
+
+if pagesize == None:
+return None
+
+l = list()
+for ps in pagesize.split(','):
+ps = ps.strip().upper()
+val = parse_unit(ps)
+if val == None:
+return None
+l.append(val)
+return l
+
+def check_free_pages(page_list, cell_id, free_page, logger):
+ check page size
+
+for ps in page_list:
+# if pagesize is equal to system pagesize, since it is hard to
+# calculate, so we just pass it
+if resource.getpagesize()/1024 == ps:
+logger.info(skip to check default %sKB-page % ps)
+continue
+
+sysfs_path = HUGEPAGE_PATH % (cell_id, ps)
+if not os.access(sysfs_path, os.R_OK):
+logger.error(could not find %s % sysfs_path)
+return False
+f= open(sysfs_path)
+fp = int(f.read())
+f.close()
+if not fp == free_page[0][ps]:
+logger.error(Free %sKB page checking failed % ps)
+return False
+logger.info(Free %sKB page: %s % (ps, fp))
+
+return True
+
+def free_pages(params):
+ test libvirt free pages
+
+logger = params['logger']
+cell_id = int(params['cellid'])
+
+conn = sharedmod.libvirtobj['conn']
+
+page_list = parse_page_list(params['pagesize'])
+if page_list == None:
+logger.error(pagesize could not be recognized)
+return 1
+
+try:
+free_page = conn.getFreePages(page_list, cell_id, 1)
+
+if check_free_pages(page_list, cell_id, free_page, logger):
+logger.info(Success to check free page)
+else:
+logger.error(Failed to check free page)
+return 1
+except libvirtError, e:
+logger.error(API error message: %s, error code is %s %
+ e.message)
+return 1
+return 0
\ No newline at end of file
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
On Tue, Mar 10, 2015 at 11:38:52AM +0100, Pavel Hrdina wrote: On Tue, Mar 10, 2015 at 10:32:04AM +0100, Ján Tomko wrote: On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote: in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) This does fix the memory leak and makes the function behave like it's documented in virDomainGetFSInfo and virDomainFSInfoFree: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree But it changes the public API - if there are applications that already work around this function by freeing the info, this change would introduce a double free. Well, this is a good point that we might break somebody's application, but the virDomainFSInfoFree is the only one *Free API that doesn't call free/unref on passed argument. In the documentation the @info is an array and we specifically says that calling virDomainFSInfoFree on each element and then just free the @info is sufficient, but it isn't and there will be a memory leak. It changes behavior of public API to correctly free the allocated memory as documentation says. Let's hear another opinion, but I'm for fixing it. I agree, in the normal way of usage of this API, this will always be wrong. virsh is leaking this, python bindings are leaking this, perl bindings are leaking this. Only applications written in C have the opportunity to do a workaround, and most of our apps are not written in C. I think we have to fix this really. 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] Do not inline virNumaNodeIsAvailable
On Mon, Mar 09, 2015 at 02:03:16PM +0100, Martin Kletzander wrote: On Mon, Mar 09, 2015 at 10:09:17AM +, Daniel P. Berrange wrote: On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote: On Thu, Mar 05, 2015 at 11:09:41AM +, Daniel P. Berrange wrote: On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote: Explicitly request that virNumaNodeIsAvailable not be inlined. This fixes the test suite when building with clang (3.5.1). Huh, so clang will inline functions, even if they are exported in the .so library ? Is there some clang compiler flag we can use to stop that ? I'd only expect it to inline stuff which was declared static, or whose impl body was in the header file If I understand it correctly, that means that clang is not compatible with gcc. Excerpt from gcc online docs [1]: When a function is both inline and static, if all calls to the function are integrated into the caller, and the function's address is never used, then the function's own assembler code is never referenced. Excerpt from gcc online docs [1]: By default, Clang builds C code in GNU C11 mode, so it uses standard C99 semantics for the inline keyword. These semantics are different from those in GNU C89 mode, which is the default mode in versions of GCC prior to 5.0. However further reading of the second documentation and c89 semantics it doesn't say anything about the fact that such function should be inlined. But we haven't added the 'inline' keyword to this function at all - it is just a normal function marked for export in the .so file, so I'm puzzelled why it is getting inlined. Exactly, that's what I'm trying to find out as well. Anyway, is this clang 3.6 specific? I don't have this problem when compiling with 3.5. Nor does this show with gcc -std=gnu11. I'm getting 3.6 to check whether that's the difference. After updating clang and llvm from 3.5 to 3.6, I still don't get this error. And I have only 4 (fake) nodes available, so it _is_ rewriting that function. I'm getting the error with 3.5.1, as I said in the commit message. These are the failing qemuxml2argvtest cases: 60) QEMU XML-2-ARGV hugepages-pages ... libvirt: error : internal error: NUMA node 1 is unavailable 63) QEMU XML-2-ARGV hugepages-shared ... libvirt: error : internal error: NUMA node 1 is unavailable 324) QEMU XML-2-ARGV numatune-memnode ... libvirt: error : internal error: NUMA node 1 is unavailable 326) QEMU XML-2-ARGV numatune-memnode-no-memory ... libvirt: error : internal error: NUMA node 3 is unavailable 329) QEMU XML-2-ARGV numatune-auto-prefer ... libvirt: error : internal error: NUMA node 1 is unavailable So with 4 fake nodes, the tests could still pass even if the function is not mocked. Try changing the nodeset in #326 to 4 if it fails. [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html [2] http://clang.llvm.org/compatibility.html --- This only leaves the mysterious check-protocol failure. That's not that mysterious, it's just that we check the order and clang sorts enums before structs, but gcc doesn't. Also clang adds public: to structs, so it probably treats it as a C++ or C# structs or something? By the way if I compile with clang with -std=gnu11 or -std=gnu99, the public: stuff is gone :) It is mysterious, because it doesn't fail consistently. It was working for me after I tried it again after 'git clean -fxd', today it failed again (though I don't remember if I ran autogen again). Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not inline virNumaNodeIsAvailable
On Tue, Mar 10, 2015 at 01:05:22PM +0100, Ján Tomko wrote: On Mon, Mar 09, 2015 at 02:03:16PM +0100, Martin Kletzander wrote: On Mon, Mar 09, 2015 at 10:09:17AM +, Daniel P. Berrange wrote: On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote: On Thu, Mar 05, 2015 at 11:09:41AM +, Daniel P. Berrange wrote: On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote: Explicitly request that virNumaNodeIsAvailable not be inlined. This fixes the test suite when building with clang (3.5.1). Huh, so clang will inline functions, even if they are exported in the .so library ? Is there some clang compiler flag we can use to stop that ? I'd only expect it to inline stuff which was declared static, or whose impl body was in the header file If I understand it correctly, that means that clang is not compatible with gcc. Excerpt from gcc online docs [1]: When a function is both inline and static, if all calls to the function are integrated into the caller, and the function's address is never used, then the function's own assembler code is never referenced. Excerpt from gcc online docs [1]: By default, Clang builds C code in GNU C11 mode, so it uses standard C99 semantics for the inline keyword. These semantics are different from those in GNU C89 mode, which is the default mode in versions of GCC prior to 5.0. However further reading of the second documentation and c89 semantics it doesn't say anything about the fact that such function should be inlined. But we haven't added the 'inline' keyword to this function at all - it is just a normal function marked for export in the .so file, so I'm puzzelled why it is getting inlined. Exactly, that's what I'm trying to find out as well. Anyway, is this clang 3.6 specific? I don't have this problem when compiling with 3.5. Nor does this show with gcc -std=gnu11. I'm getting 3.6 to check whether that's the difference. After updating clang and llvm from 3.5 to 3.6, I still don't get this error. And I have only 4 (fake) nodes available, so it _is_ rewriting that function. I'm getting the error with 3.5.1, as I said in the commit message. These are the failing qemuxml2argvtest cases: 60) QEMU XML-2-ARGV hugepages-pages ... libvirt: error : internal error: NUMA node 1 is unavailable 63) QEMU XML-2-ARGV hugepages-shared ... libvirt: error : internal error: NUMA node 1 is unavailable 324) QEMU XML-2-ARGV numatune-memnode ... libvirt: error : internal error: NUMA node 1 is unavailable 326) QEMU XML-2-ARGV numatune-memnode-no-memory ... libvirt: error : internal error: NUMA node 3 is unavailable 329) QEMU XML-2-ARGV numatune-auto-prefer ... libvirt: error : internal error: NUMA node 1 is unavailable So with 4 fake nodes, the tests could still pass even if the function is not mocked. Try changing the nodeset in #326 to 4 if it fails. [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html [2] http://clang.llvm.org/compatibility.html --- This only leaves the mysterious check-protocol failure. That's not that mysterious, it's just that we check the order and clang sorts enums before structs, but gcc doesn't. Also clang adds public: to structs, so it probably treats it as a C++ or C# structs or something? By the way if I compile with clang with -std=gnu11 or -std=gnu99, the public: stuff is gone :) It is mysterious, because it doesn't fail consistently. It was working for me after I tried it again after 'git clean -fxd', today it failed again (though I don't remember if I ran autogen again). How exactly are you running the build with clang ? Are you just doing this CC=clang ./autogen.sh make make check Or is there more to it than that ? 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] virnet*: Don't unlock object in dispose
On Tue, Mar 10, 2015 at 11:20:14AM +0100, Michal Privoznik wrote: As of bba93d40 all of our RPC objects are derived from virObjectLockable. However, during rewrite some errors sneaked in. For instance, the dispose functions to virNetClient and virNetServerClient objects were not only freeing allocated memory, but unlocking themselves. This is wrong. Object should never disappear while locked. ACK, ref counting ensures we have the only reference and are unlocked when dispose runs. 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] qemu: fix memory leak in qemuAgentGetFSInfo
On 03/10/2015 05:32 PM, Ján Tomko wrote: On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote: in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) This does fix the memory leak and makes the function behave like it's documented in virDomainGetFSInfo and virDomainFSInfoFree: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree But it changes the public API - if there are applications that already work around this function by freeing the info, this change would introduce a double free. I would NACK this if the documentation for both APIs didn't say that's how this function should behave. I'd like to hear a second opinion. I don't think this documentation make any confusable. for using the function virDomainGetFSInfo(), user also need to call virDomainFSInfoFree() on each array element, and call free() info. example: virDomainFSInfoPtr *info; ndata = virDomainGetFSInfo(dom, info, 0); for (i = 0; i ndata; i++) virDomainFSInfoFree(info[i]); VIR_FREE(info); Thanks, Chen Jan diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 04545fd..7f8a7ce 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11337,4 +11337,6 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) for (i = 0; i info-ndevAlias; i++) VIR_FREE(info-devAlias[i]); VIR_FREE(info-devAlias); + +VIR_FREE(info); } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't fill in nicindexes for session mode libvirtd
On Tue, Mar 10, 2015 at 02:32:04AM -0400, Laine Stump wrote:
Commit 4bbe1029f fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the niceindex array would be filled in for macvtap as well as tap
s/niceindex/nicindex/ if you haven't pushed already.
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by if
(cfg-privileged). The result was that the fixed libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:
Unable to open control socket: Operation not permitted
To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg-privileged.
This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244
---
If someone (Rich?) needs this pushed before I am awake, please feel
free to push it. (also push to the 1.2.13-maint branch if you do)
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1510797..3d1483e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7861,6 +7861,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
int actualType = virDomainNetGetActualType(net);
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7936,7 +7937,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* network and bridge use a tap device, and direct uses a
* macvtap device
*/
-if (nicindexes nnicindexes net-ifname) {
+if (cfg-privileged nicindexes nnicindexes net-ifname) {
if (virNetDevGetIndex(net-ifname, nicindex) 0 ||
VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) 0)
goto cleanup;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
pgpuiFocHl93r.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/4] util: introduce 2 new helpers for get interface IPv6 address
On 03/10/2015 06:03 AM, lhuang wrote: ...snip... Interesting idea, but you'd be just throwing things away. I could perhaps having some code periodically cache addresses... or cache addresses, subscribe to some event to let you know when a new address is generated so you can add it to your list (if there is one)... But, how do you use it? Sorry, i don't know what these words' (how do you use it ?) mean. My point was - how does the existing code decide which of the 'n' addresses found/returned to use as the address? I guess your mean is ask me how to use the code or function you mentioned, i think maybe we can avoid to use it :) However this should be another patch which add a function to get a list of ip address. Sure, but would it be just for display purposes only? Once there is more than one address per dev_name being queried, code that wants to use an address will need to have some decision point in order to filter out addresses known to be bad. Perhaps this is done by type - multicast, localnet, etc or perhaps by some other lookup mechanism. I'm thinking of the netinet/in.h macros (search on MULTICAST, LOCAL, LOOPBACK, etc.). Whatever filters are desired, they could be added as an attribute list of sorts (eg, filter='multicast,local') that way it's up to the consumer to decide which filters apply knowing what that filter maps to. In the example you provided (2001:db8:ca2:2::1/64) - what about that address made it unusable? That's what needs to be filtered... Doing a google search on the address ironically points a bz that Laine owns... I'm not up on all the IPv6 addressing rules, so my view is a more high level... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't fill in nicindexes for session mode libvirtd
On Tue, Mar 10, 2015 at 02:32:04AM -0400, Laine Stump wrote:
Commit 4bbe1029f fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the niceindex array would be filled in for macvtap as well as tap
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by if
(cfg-privileged). The result was that the fixed libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:
Unable to open control socket: Operation not permitted
To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg-privileged.
This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244
---
If someone (Rich?) needs this pushed before I am awake, please feel
free to push it. (also push to the 1.2.13-maint branch if you do)
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1510797..3d1483e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7861,6 +7861,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
int actualType = virDomainNetGetActualType(net);
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7936,7 +7937,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* network and bridge use a tap device, and direct uses a
* macvtap device
*/
-if (nicindexes nnicindexes net-ifname) {
+if (cfg-privileged nicindexes nnicindexes net-ifname) {
if (virNetDevGetIndex(net-ifname, nicindex) 0 ||
VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) 0)
goto cleanup;
Just tested with this patch locally, using Rich's reproducer. Fixes the
issue.
Before applying this patch, tested with version
libvirt-daemon-kvm-1.2.13-1.fc23.x86_64:
$ whoami
kashyapc
$ guestfish -a /dev/null --network run
libguestfs: error: could not create appliance through libvirt.
. . .
Original error from libvirt: Unable to open control socket: Operation not
permitted [code=38 domain=0]
After applying this patch (applied on current git):
$ git describe
v1.2.13-100-gb7d027b
$ guestfish -a /dev/null --network run
$ echo $?
0
So, FWIW:
Tested-By: Kashyap Chamarthy kcham...@redhat.com
--
/kashyap
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virQEMUCapsInitQMP: Don't dispose locked @vm
When creating qemu capabilities, a dummy virDomainObj is created just
because our monitor code expects that. However, the object is created
locked already. Then, under cleanup label, we simply unref the object
which results in whole domain object to be disposed. The object lock
is destroyed subsequently, but hey - it's still locked:
==24845== Thread #14's call to pthread_mutex_destroy failed
==24845==with error code 16 (EBUSY: Device or resource busy)
==24845==at 0x4C3024E: pthread_mutex_destroy (in
/usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so)
==24845==by 0x531F72E: virMutexDestroy (virthread.c:83)
==24845==by 0x5302977: virObjectLockableDispose (virobject.c:237)
==24845==by 0x5302A89: virObjectUnref (virobject.c:265)
==24845==by 0x1DD37866: virQEMUCapsInitQMP (qemu_capabilities.c:3397)
==24845==by 0x1DD37CC6: virQEMUCapsNewForBinary (qemu_capabilities.c:3481)
==24845==by 0x1DD381E2: virQEMUCapsCacheLookup (qemu_capabilities.c:3609)
==24845==by 0x1DD30F8A: virQEMUCapsInitGuest (qemu_capabilities.c:744)
==24845==by 0x1DD31889: virQEMUCapsInit (qemu_capabilities.c:1020)
==24845==by 0x1DD7DD36: virQEMUDriverCreateCapabilities (qemu_conf.c:888)
==24845==by 0x1DDC57C0: qemuStateInitialize (qemu_driver.c:803)
==24845==by 0x53DC743: virStateInitialize (libvirt.c:777)
==24845==
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_capabilities.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8193805..dce40e0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3394,7 +3394,10 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (monpath)
ignore_value(unlink(monpath));
VIR_FREE(monpath);
-virObjectUnref(vm);
+if (vm) {
+virObjectUnlock(vm);
+virObjectUnref(vm);
+}
virObjectUnref(xmlopt);
if (pid != 0) {
--
2.0.5
--
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
On Fri, Mar 06, 2015 at 09:05:44AM -0500, John Ferlan wrote:
Add qemuDomainPinIOThread to handle setting the CPU affinity
for a specific IOThread
Signed-off-by: John Ferlan jfer...@redhat.com
---
include/libvirt/libvirt-domain.h | 9 ++
src/qemu/qemu_driver.c | 221
+++
2 files changed, 230 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index f2f7eb5..11ffd07 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3218,6 +3218,15 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
# define VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN cputune.emulatorpin
/**
+ * VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN:
+ *
+ * Macro represents formatted pinning for one IOThread specified by id which
is
+ * appended to the parameter name, for example cputune.iothreadpin1,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+# define VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN cputune.iothreadpin%u
+
+/**
* VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES:
*
* Macro represents proportional weight of the scheduler used on the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b37474f..aad08b2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5780,6 +5780,226 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom,
return ret;
}
+static int
+qemuDomainPinIOThread(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+if (virDomainPinIOThreadEnsureACL(dom-conn, vm-def, flags) 0)
+goto cleanup;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Changing affinity for IOThread dynamically is
+ not allowed when CPU placement is 'auto'));
+goto cleanup;
+}
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
+persistentDef) 0)
+goto endjob;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE)
+persistentDef = vm-def;
+
+/* Coverity didn't realize that targetDef must be set if we got here. */
+sa_assert(persistentDef);
+
+if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
+goto endjob;
+
+if (virBitmapIsAllClear(pcpumap)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(Empty iothread cpumap list for pinning));
+goto endjob;
+}
+
+/* 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 check if we can reset setting.
+ */
+if (virBitmapIsAllSet(pcpumap))
+doReset = true;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+
+if (priv-iothreadpids == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(IOThread affinity is not supported));
+goto endjob;
+}
+
+if (iothread_id priv-niothreadpids) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(iothread value out of range %d %d),
+ iothread_id, priv-niothreadpids);
+goto endjob;
+}
+
+if (vm-def-cputune.iothreadspin) {
+/* The VcpuPinDefCopy works for IOThreads too */
Maybe it should be renamed to something like virDomainPinDefCopy then?
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virQEMUCapsInitQMP: Don't dispose locked @vm
On Tue, Mar 10, 2015 at 02:30:16PM +0100, Michal Privoznik wrote:
When creating qemu capabilities, a dummy virDomainObj is created just
because our monitor code expects that. However, the object is created
locked already. Then, under cleanup label, we simply unref the object
which results in whole domain object to be disposed. The object lock
is destroyed subsequently, but hey - it's still locked:
==24845== Thread #14's call to pthread_mutex_destroy failed
==24845==with error code 16 (EBUSY: Device or resource busy)
==24845==at 0x4C3024E: pthread_mutex_destroy (in
/usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so)
==24845==by 0x531F72E: virMutexDestroy (virthread.c:83)
==24845==by 0x5302977: virObjectLockableDispose (virobject.c:237)
==24845==by 0x5302A89: virObjectUnref (virobject.c:265)
==24845==by 0x1DD37866: virQEMUCapsInitQMP (qemu_capabilities.c:3397)
==24845==by 0x1DD37CC6: virQEMUCapsNewForBinary (qemu_capabilities.c:3481)
==24845==by 0x1DD381E2: virQEMUCapsCacheLookup (qemu_capabilities.c:3609)
==24845==by 0x1DD30F8A: virQEMUCapsInitGuest (qemu_capabilities.c:744)
==24845==by 0x1DD31889: virQEMUCapsInit (qemu_capabilities.c:1020)
==24845==by 0x1DD7DD36: virQEMUDriverCreateCapabilities (qemu_conf.c:888)
==24845==by 0x1DDC57C0: qemuStateInitialize (qemu_driver.c:803)
==24845==by 0x53DC743: virStateInitialize (libvirt.c:777)
==24845==
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_capabilities.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8193805..dce40e0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3394,7 +3394,10 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (monpath)
ignore_value(unlink(monpath));
VIR_FREE(monpath);
-virObjectUnref(vm);
+if (vm) {
+virObjectUnlock(vm);
+virObjectUnref(vm);
+}
Looks like a job for EndAPI ;)
ACK
virObjectUnref(xmlopt);
if (pid != 0) {
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
pgpHkMZ1SzU6y.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Return bool in virBitmapGetBit
On Fri, Mar 06, 2015 at 01:01:13PM -0700, Eric Blake wrote:
On 03/06/2015 10:02 AM, Ján Tomko wrote:
Most of the callers use the pattern:
bool b;
ignore_value(virBitmapGetBit(.., b));
if (b) {
...
}
And the rest treats the failure (which can only happen
when the requested bit is out of bitmap bounds) the same
as they treat a 'false' bit.
Blindly returning false for out of bounds may work for some (most?)
callers, but I still wonder if we should expose the full bounds-checking
version under a different name, and/or expose a variant that lets the
caller specify the default value to return for an out-of-bounds
reference. That is, while you've made the common case short, I wonder
if we need the extra control for correctness elsewhere.
From the 23 callers in this patch:
10 explicitly ignore_value
5 are in a for loop from 0 to virBitmapSize (virProcessSetAffinity,
nodeCapsInitNUMA, virDomainSnapshotAlignDisks, libxlDomainGetVcpuPinInfo)
1 checks the range before calling GetBit (virPortAllocatorSetUsed)
1 loops against the bitmap range (virPortAllocatorAcquired)
I think these don't need any bounds-checking
2 of them take an enum as an argument (*CapsGet), I don't think we need
a special case for QEMU_CAPS_LAST either.
qemuBuildMemoryBackendStr admits to asking for out-of-range bits, so it
would require checking the bounds up front, or a default of 'false'.
In the rest of the callers, out-of-bounds queries are programming bugs,
but since the origins of their indexes are not so straightforward,
perhaps those should keep using the range-checking version.
So there's only one caller where a default of 'false' makes sense.
I think a function named virBitmapIsBitSet could return false on
out-of-range (how can it be set if it's not in the bitmap?)
I'm not outright rejecting this patch, but I'm also not sure if it is
the right move to take - is our attempt at bounds-checking in order to
prevent programming bugs helping us avoid bugs (then this patch is
losing that safety), or is it just causing needless noise of boilerplate
(most callers using ignore_value, in which case this patch is the right
thing). Anyone else with an opinion?
In the meantime I've pushed the other two acked patches.
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not inline virNumaNodeIsAvailable
On Tue, Mar 10, 2015 at 01:45:38PM +0100, Martin Kletzander wrote: On Tue, Mar 10, 2015 at 12:24:03PM +, Daniel P. Berrange wrote: On Tue, Mar 10, 2015 at 01:05:22PM +0100, Ján Tomko wrote: These are the failing qemuxml2argvtest cases: 60) QEMU XML-2-ARGV hugepages-pages ... libvirt: error : internal error: NUMA node 1 is unavailable 63) QEMU XML-2-ARGV hugepages-shared ... libvirt: error : internal error: NUMA node 1 is unavailable 324) QEMU XML-2-ARGV numatune-memnode ... libvirt: error : internal error: NUMA node 1 is unavailable 326) QEMU XML-2-ARGV numatune-memnode-no-memory ... libvirt: error : internal error: NUMA node 3 is unavailable 329) QEMU XML-2-ARGV numatune-auto-prefer ... libvirt: error : internal error: NUMA node 1 is unavailable So with 4 fake nodes, the tests could still pass even if the function is not mocked. Try changing the nodeset in #326 to 4 if it fails. I tried changing that, it fails. I tried adjusting the tests to more nodes, it fails. After adjusting the mock function again, it works. So it gets mocked all the time, but I know where the difference is, probably. Try building with -O0, it will probably disable the inlining. However, if that's the case, I believe it's still clang's fault since they don't document inlining functions without the inline keyword just because you optimize. -O0 does disable the inlining, but introduces the stack size warning. [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html [2] http://clang.llvm.org/compatibility.html --- This only leaves the mysterious check-protocol failure. That's not that mysterious, it's just that we check the order and clang sorts enums before structs, but gcc doesn't. Also clang adds public: to structs, so it probably treats it as a C++ or C# structs or something? By the way if I compile with clang with -std=gnu11 or -std=gnu99, the public: stuff is gone :) It is mysterious, because it doesn't fail consistently. Oh, it does fail all the time for me. Did I mistakenly build with gcc to fix it? It was working for me after I tried it again after 'git clean -fxd', today it failed again (though I don't remember if I ran autogen again). How exactly are you running the build with clang ? Are you just doing this CC=clang ./autogen.sh make make check Or is there more to it than that ? ./autogen.sh --system --without-driver-modules CC=clang make -j5 check Now it failed two times in a row, and once without parallel make. I have no idea what I did back then that fixed it. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
On Tue, Mar 10, 2015 at 10:46:45AM +, Daniel P. Berrange wrote: On Tue, Mar 10, 2015 at 11:38:52AM +0100, Pavel Hrdina wrote: On Tue, Mar 10, 2015 at 10:32:04AM +0100, Ján Tomko wrote: On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote: in virDomainFSInfoFree(), don't free the virDomainFSInfo data. ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793 ==10670==at 0x4A06BC3: calloc (vg_replace_malloc.c:618) ==10670==by 0x509DEBD: virAlloc (viralloc.c:144) ==10670==by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837) ==10670==by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238) Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) This does fix the memory leak and makes the function behave like it's documented in virDomainGetFSInfo and virDomainFSInfoFree: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree But it changes the public API - if there are applications that already work around this function by freeing the info, this change would introduce a double free. Well, this is a good point that we might break somebody's application, but the virDomainFSInfoFree is the only one *Free API that doesn't call free/unref on passed argument. In the documentation the @info is an array and we specifically says that calling virDomainFSInfoFree on each element and then just free the @info is sufficient, but it isn't and there will be a memory leak. It changes behavior of public API to correctly free the allocated memory as documentation says. Let's hear another opinion, but I'm for fixing it. I agree, in the normal way of usage of this API, this will always be wrong. virsh is leaking this, python bindings are leaking this, perl bindings are leaking this. Only applications written in C have the opportunity to do a workaround, and most of our apps are not written in C. I think we have to fix this really. Now pushed. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not inline virNumaNodeIsAvailable
On Tue, Mar 10, 2015 at 12:24:03PM +, Daniel P. Berrange wrote: On Tue, Mar 10, 2015 at 01:05:22PM +0100, Ján Tomko wrote: On Mon, Mar 09, 2015 at 02:03:16PM +0100, Martin Kletzander wrote: On Mon, Mar 09, 2015 at 10:09:17AM +, Daniel P. Berrange wrote: On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote: On Thu, Mar 05, 2015 at 11:09:41AM +, Daniel P. Berrange wrote: On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote: Explicitly request that virNumaNodeIsAvailable not be inlined. This fixes the test suite when building with clang (3.5.1). Huh, so clang will inline functions, even if they are exported in the .so library ? Is there some clang compiler flag we can use to stop that ? I'd only expect it to inline stuff which was declared static, or whose impl body was in the header file If I understand it correctly, that means that clang is not compatible with gcc. Excerpt from gcc online docs [1]: When a function is both inline and static, if all calls to the function are integrated into the caller, and the function's address is never used, then the function's own assembler code is never referenced. Excerpt from gcc online docs [1]: By default, Clang builds C code in GNU C11 mode, so it uses standard C99 semantics for the inline keyword. These semantics are different from those in GNU C89 mode, which is the default mode in versions of GCC prior to 5.0. However further reading of the second documentation and c89 semantics it doesn't say anything about the fact that such function should be inlined. But we haven't added the 'inline' keyword to this function at all - it is just a normal function marked for export in the .so file, so I'm puzzelled why it is getting inlined. Exactly, that's what I'm trying to find out as well. Anyway, is this clang 3.6 specific? I don't have this problem when compiling with 3.5. Nor does this show with gcc -std=gnu11. I'm getting 3.6 to check whether that's the difference. After updating clang and llvm from 3.5 to 3.6, I still don't get this error. And I have only 4 (fake) nodes available, so it _is_ rewriting that function. I'm getting the error with 3.5.1, as I said in the commit message. I missed that, sorry. These are the failing qemuxml2argvtest cases: 60) QEMU XML-2-ARGV hugepages-pages ... libvirt: error : internal error: NUMA node 1 is unavailable 63) QEMU XML-2-ARGV hugepages-shared ... libvirt: error : internal error: NUMA node 1 is unavailable 324) QEMU XML-2-ARGV numatune-memnode ... libvirt: error : internal error: NUMA node 1 is unavailable 326) QEMU XML-2-ARGV numatune-memnode-no-memory ... libvirt: error : internal error: NUMA node 3 is unavailable 329) QEMU XML-2-ARGV numatune-auto-prefer ... libvirt: error : internal error: NUMA node 1 is unavailable So with 4 fake nodes, the tests could still pass even if the function is not mocked. Try changing the nodeset in #326 to 4 if it fails. I tried changing that, it fails. I tried adjusting the tests to more nodes, it fails. After adjusting the mock function again, it works. So it gets mocked all the time, but I know where the difference is, probably. Try building with -O0, it will probably disable the inlining. However, if that's the case, I believe it's still clang's fault since they don't document inlining functions without the inline keyword just because you optimize. [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html [2] http://clang.llvm.org/compatibility.html --- This only leaves the mysterious check-protocol failure. That's not that mysterious, it's just that we check the order and clang sorts enums before structs, but gcc doesn't. Also clang adds public: to structs, so it probably treats it as a C++ or C# structs or something? By the way if I compile with clang with -std=gnu11 or -std=gnu99, the public: stuff is gone :) It is mysterious, because it doesn't fail consistently. Oh, it does fail all the time for me. It was working for me after I tried it again after 'git clean -fxd', today it failed again (though I don't remember if I ran autogen again). How exactly are you running the build with clang ? Are you just doing this CC=clang ./autogen.sh make make check Or is there more to it than that ? I have a script that does export CC=clang and then ./autogen.sh and make (everything). 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 :| pgpS0I5VN19si.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Follow-up patches for IOThread API/display
On Mon, Mar 09, 2015 at 08:27:10AM -0400, John Ferlan wrote:
On 03/08/2015 07:03 AM, Ján Tomko wrote:
ACK series, thanks for touching up VcpuInfo and EmulatorInfo as well!
There's one bug that I noticed:
If the CPUs are pinned domain-wide, that is:
vcpu placement='static' cpuset='0-1'4/vcpu
iothreads2/iothreads
Both vcpu threads and iothreads will inherit this pinning.
For a shutoff domain, vcpupininfo will display 0-1 for all vcpus, but
iothreadsinfo shows 0-4, even though they will get pinned to 0-1 after
domain startup.
Turns out the vpcupin info is filled for all the vcpus when the XML is
parsed since commit 10f8a45deb0f057a70a0d49794d3a3d19d17dceb
Falling back to targetDef-cpumask in qemuDomainGetIOThreadsConfig
(as qemuDomainGetEmulatorPinInfo does) would solve that too.
Squashed the following into patch 4 to address this...
I don't think this functional change belongs to the patch refactoring
the function to use the virBitmap* APIs. But since the functionality has
not yet been released, nobody should feel the need to backport any of
these commits, so mixing them up is not that big deal.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2a9db1b..ceceafa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5653,6 +5653,8 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
virDomainIOThreadInfoPtr **info)
{
virDomainIOThreadInfoPtr *info_ret = NULL;
+virBitmapPtr bitmap = NULL;
+virBitmapPtr cpumask = NULL;
int hostcpus;
size_t i;
int ret = -1;
@@ -5668,7 +5670,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
for (i = 0; i targetDef-iothreads; i++) {
virDomainVcpuPinDefPtr pininfo;
-virBitmapPtr bitmap = NULL;
if (VIR_ALLOC(info_ret[i]) 0)
goto cleanup;
@@ -5681,20 +5682,22 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr
targetDef,
targetDef-cputune.niothreadspin,
i + 1);
if (!pininfo) {
-if (!(bitmap = virBitmapNew(hostcpus)))
-goto cleanup;
-virBitmapSetAll(bitmap);
+if (targetDef-cpumask) {
+cpumask = targetDef-cpumask;
+} else {
+if (!(bitmap = virBitmapNew(hostcpus)))
+goto cleanup;
+virBitmapSetAll(bitmap);
+cpumask = bitmap;
+}
} else {
-bitmap = pininfo-cpumask;
+cpumask = pininfo-cpumask;
}
-if (virBitmapToData(bitmap, info_ret[i]-cpumap,
-info_ret[i]-cpumaplen) 0) {
-if (!pininfo)
-virBitmapFree(bitmap);
+if (virBitmapToData(cpumask, info_ret[i]-cpumap,
+info_ret[i]-cpumaplen) 0)
goto cleanup;
-}
-if (!pininfo)
-virBitmapFree(bitmap);
+virBitmapFree(bitmap);
+bitmap = NULL;
}
*info = info_ret;
@@ -5707,6 +5710,7 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
virDomainIOThreadsInfoFree(info_ret[i]);
VIR_FREE(info_ret);
}
+virBitmapFree(bitmap);
return ret;
}
Replaced the overly aggressively removed lines from patch 5...
I will send a separate patch for the domain_conf.c change that
addresses any needs for iothreadspin setup to use the def-cpumask.
However the squashed-in patch could've benefited from going through a
standard review process. If you plan to copy the vcpupin code in domain_conf.c
to do the same with iothreadspin, the squashed-in change is redundant.
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/5] conf: Add 'family' attribute to graphics 'listen' element
On 03/10/2015 12:58 PM, Ján Tomko wrote: On Mon, Mar 09, 2015 at 08:04:59PM -0400, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4. As Laine mentioned in his reply to v1: https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html This is intended to be run only on networks with one address. With more addresses, you cannot control which one to use. It's been a while since I've had to think whether getifaddrs() would return the same IPv4 address in the first entry as would be return from the ioctl(SIOCGIFADDR)... IIRC, IPv4 addresses can be aliased to the same device, but how getifaddrs handles returning addresses I just don't have recent exposure to.. If you want to listen on IPv6, don't configure an IPv4 address on the network and vice versa. This attribute does not seem that useful to me. The original bug https://bugzilla.redhat.com/show_bug.cgi?id=1192318 complained about 'no usable address' I think the bug here is not treating an ipv6 address as an IP address, not that we cannot choose the attribute by family. Jan hmmm... true we're really just looking to get an address and shouldn't care what style it is. However, what if someone has both configured and wants to force usage of one over the other? Maybe they've separate their IPv4 and IPv6 addresses to connect to different places. Perhaps forcing certain protocols over IPv6 rather than IPv4? Since both can be defined in one network object - it's possible - how or why it would be done I haven't given too much thought to. John The graphics XML may look like this after this commit: graphics type='spice' port='5900' autoport='yes' listen type='network' address='192.168.0.1' network='vepa-net' family='ipv4'/ /graphics -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 04/15] bridge_driver: Use virNetworkObjEndAPI
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/network/bridge_driver.c | 57 +++--
1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 70467f6..97fda3f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2496,8 +2496,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr
conn,
ret = virGetNetwork(conn, network-def-name, network-def-uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -2522,8 +2521,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr
conn,
ret = virGetNetwork(conn, network-def-name, network-def-uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -2670,8 +2668,7 @@ static int networkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
return ret;
}
@@ -2689,8 +2686,7 @@ static int networkIsPersistent(virNetworkPtr net)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
return ret;
}
@@ -2959,8 +2955,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn,
const char *xml)
virNetworkDefFree(def);
if (event)
virObjectEventStateQueue(driver-networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3016,8 +3011,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn,
const char *xml)
virObjectEventStateQueue(driver-networkEventState, event);
if (freeDef)
virNetworkDefFree(def);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3077,8 +3071,7 @@ networkUndefine(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver-networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3249,8 +3242,7 @@ networkUpdate(virNetworkPtr net,
}
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3284,8 +3276,7 @@ static int networkCreate(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver-networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3335,8 +3326,7 @@ static int networkDestroy(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver-networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3364,8 +3354,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(def, flags);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -3389,8 +3378,7 @@ static char *networkGetBridgeName(virNetworkPtr net) {
ignore_value(VIR_STRDUP(bridge, network-def-bridge));
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return bridge;
}
@@ -3410,8 +3398,7 @@ static int networkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -3478,8 +3465,7 @@ static int networkSetAutostart(virNetworkPtr net,
cleanup:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
networkDriverUnlock();
return ret;
}
@@ -3651,8 +3637,7 @@ networkGetDHCPLeases(virNetworkPtr network,
VIR_FREE(custom_lease_file);
virJSONValueFree(leases_array);
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
return rv;
@@ -4124,8 +4109,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
error:
@@ -4327,8 +4311,7 @@ networkNotifyActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
error:
@@ -4477,8 +4460,7 @@
[libvirt] [PATCH v3 07/15] virNetworkObjList: Derive from virObjectLockableClass
Later we can turn APIs to lock the object if needed instead of
relying on caller to mutually exclude itself (probably done by
locking a big lock anyway).
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f677e3c..399c372 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -52,7 +52,7 @@
#define CLASS_ID_BITMAP_SIZE (116)
struct _virNetworkObjList {
-virObject parent;
+virObjectLockable parent;
virHashTablePtr objs;
};
@@ -92,7 +92,7 @@ static int virNetworkObjOnceInit(void)
virNetworkObjDispose)))
return -1;
-if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
+if (!(virNetworkObjListClass = virClassNew(virClassForObjectLockable(),
virNetworkObjList,
sizeof(virNetworkObjList),
virNetworkObjListDispose)))
@@ -153,7 +153,7 @@ virNetworkObjListPtr virNetworkObjListNew(void)
if (virNetworkObjInitialize() 0)
return NULL;
-if (!(nets = virObjectNew(virNetworkObjListClass)))
+if (!(nets = virObjectLockableNew(virNetworkObjListClass)))
return NULL;
if (!(nets-objs = virHashCreate(50, virNetworkObjListDataFree))) {
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 08/15] network_conf: Introduce locked versions of lookup functions
This is going to be needed later, when some functions already
have the virNetworkObjList object already locked and need to
lookup a object to work on. As an example of such function is
virNetworkAssignDef(). The other use case might be in
virNetworkObjListForEach() callback.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 34 ++
src/conf/network_conf.h | 4
src/libvirt_private.syms | 2 ++
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 399c372..be5cf0e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -164,8 +164,9 @@ virNetworkObjListPtr virNetworkObjListNew(void)
return nets;
}
-virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
- const unsigned char *uuid)
+virNetworkObjPtr
+virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid)
{
virNetworkObjPtr ret = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -178,6 +179,18 @@ virNetworkObjPtr
virNetworkObjFindByUUID(virNetworkObjListPtr nets,
return ret;
}
+virNetworkObjPtr
+virNetworkObjFindByUUID(virNetworkObjListPtr nets,
+const unsigned char *uuid)
+{
+virNetworkObjPtr ret;
+
+virObjectLock(nets);
+ret = virNetworkObjFindByUUIDLocked(nets, uuid);
+virObjectUnlock(nets);
+return ret;
+}
+
static int
virNetworkObjSearchName(const void *payload,
const void *name ATTRIBUTE_UNUSED,
@@ -193,8 +206,9 @@ virNetworkObjSearchName(const void *payload,
return want;
}
-virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
- const char *name)
+virNetworkObjPtr
+virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name)
{
virNetworkObjPtr ret = NULL;
@@ -204,6 +218,18 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
return ret;
}
+virNetworkObjPtr
+virNetworkObjFindByName(virNetworkObjListPtr nets,
+const char *name)
+{
+virNetworkObjPtr ret;
+
+virObjectLock(nets);
+ret = virNetworkObjFindByNameLocked(nets, name);
+virObjectUnlock(nets);
+return ret;
+}
+
bool
virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index e0ed714..3e926f7 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -300,8 +300,12 @@ virNetworkObjIsActive(const virNetworkObj *net)
virNetworkObjListPtr virNetworkObjListNew(void);
+virNetworkObjPtr virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid);
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid);
+virNetworkObjPtr virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name);
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name);
bool virNetworkObjTaint(virNetworkObjPtr obj,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5fbe094..64808ce 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,7 +565,9 @@ virNetworkLoadAllState;
virNetworkObjAssignDef;
virNetworkObjEndAPI;
virNetworkObjFindByName;
+virNetworkObjFindByNameLocked;
virNetworkObjFindByUUID;
+virNetworkObjFindByUUIDLocked;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 06/15] parallels_network: Use virNetworkObjEndAPI
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/parallels/parallels_network.c | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 86038bf..1bcd2d3 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
goto cleanup;
net-active = 1;
net-autostart = 1;
-virObjectUnlock(net);
return net;
cleanup:
@@ -241,7 +240,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
static virNetworkObjPtr
parallelsAddRoutedNetwork(parallelsConnPtr privconn)
{
-virNetworkObjPtr net;
+virNetworkObjPtr net = NULL;
virNetworkDefPtr def;
if (VIR_ALLOC(def) 0)
@@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
net-active = 1;
net-autostart = 1;
-virObjectUnlock(net);
return net;
@@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
-virNetworkObjPtr net;
+virNetworkObjPtr net = NULL;
int ret = -1;
int count;
size_t i;
@@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr
privconn)
net = parallelsLoadNetwork(privconn, jobj2);
if (!net)
goto cleanup;
+else
+virNetworkObjEndAPI(net);
}
-if (!parallelsAddRoutedNetwork(privconn))
+if (!(net = parallelsAddRoutedNetwork(privconn)))
goto cleanup;
ret = 0;
cleanup:
virJSONValueFree(jobj);
+virNetworkObjEndAPI(net);
return ret;
}
@@ -444,8 +445,7 @@ static virNetworkPtr
parallelsNetworkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, network-def-name, network-def-uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -468,8 +468,7 @@ static virNetworkPtr
parallelsNetworkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, network-def-name, network-def-uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -495,8 +494,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(network-def, flags);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
@@ -516,8 +514,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
return ret;
}
@@ -537,8 +534,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(obj);
return ret;
}
@@ -562,8 +558,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(network);
return ret;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/15] Drop network driver lock
Well, this is interesting. I've turned the virNetworkObjList into using a virHash. And even though the performance in my testing (using the very same test program as in previous versions of the patch set) increased. Without these patches, the test program took 56s to finish. With these patches (that is with virHash table) it takes 46s, which is better. But previously, with network objects stored in flat array I was able to get somewhere around 23s! I wonder what may be the cause. NB, nearly all patches in the series have been ACKed already, but since I'm switching the virNetworkObjList internals, I'm posting them again. Michal Privoznik (15): virNetworkObjListPtr: Turn list into a hash table network_conf: Make virNetworkObj actually virObject network_conf: Introduce virNetworkObjEndAPI bridge_driver: Use virNetworkObjEndAPI test_driver: Use virNetworkObjEndAPI parallels_network: Use virNetworkObjEndAPI virNetworkObjList: Derive from virObjectLockableClass network_conf: Introduce locked versions of lookup functions virNetworkObjListPtr: Make APIs self-locking virNetworkObjFindBy*: Return an reference to found object struct _virNetworkDriverState: Annotate items bridge_driver: Drop networkDriverLock() from everywhere test_driver: Drop testDriverLock() from almost everywhere parallels_network: Drop parallelsDriverLock() from everywhere. bridge_driver: Use more of networkObjFromNetwork cfg.mk | 2 - src/conf/network_conf.c | 545 +++ src/conf/network_conf.h | 13 +- src/libvirt_private.syms | 7 +- src/network/bridge_driver.c | 197 +++-- src/network/bridge_driver_platform.h | 5 +- src/parallels/parallels_network.c| 62 +--- src/test/test_driver.c | 108 ++- tests/networkxml2conftest.c | 4 +- tests/objectlocking.ml | 4 - 10 files changed, 441 insertions(+), 506 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 14/15] parallels_network: Drop parallelsDriverLock() from everywhere.
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 v5 4/5] qemu: Add support to pin IOThreads to specific CPU
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.
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'?
+ * so check if we can reset setting.
+ */
+if (virBitmapIsAllSet(pcpumap))
+doReset = true;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+
+if (priv-iothreadpids == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(IOThread affinity is not supported));
+goto endjob;
+}
+
+if (iothread_id priv-niothreadpids) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(iothread value out of range %d %d),
+ iothread_id, priv-niothreadpids);
+goto endjob;
+}
+
+if (vm-def-cputune.iothreadspin) {
+/* The VcpuPinDefCopy works for IOThreads too */
Maybe it should be renamed to something like virDomainPinDefCopy then?
Perhaps, but that seems outside the scope of these changes.
Yes, that one belongs to a separate patch.
The 'reuse'
of virDomainVcpuPinDefPtr by the IOThreads code was an optimization
that could also then be generalized I suppose (eg change from
virDomainVcpuPinDefPtr to virDomainPinDefPtr), but that's a much more
invasive change which would seemingly require change the structure
vcpuid value to just id.
It's just a name change. And they are generalized already, it's just the
naming that's behind :)
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 13/15] test_driver: Drop testDriverLock() from almost everywhere
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);
Re: [libvirt] [PATCH v3 3/5] conf: Add 'family' attribute to graphics 'listen' element
On Mon, Mar 09, 2015 at 08:04:59PM -0400, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4. As Laine mentioned in his reply to v1: https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html This is intended to be run only on networks with one address. With more addresses, you cannot control which one to use. If you want to listen on IPv6, don't configure an IPv4 address on the network and vice versa. This attribute does not seem that useful to me. The original bug https://bugzilla.redhat.com/show_bug.cgi?id=1192318 complained about 'no usable address' I think the bug here is not treating an ipv6 address as an IP address, not that we cannot choose the attribute by family. Jan The graphics XML may look like this after this commit: graphics type='spice' port='5900' autoport='yes' listen type='network' address='192.168.0.1' network='vepa-net' family='ipv4'/ /graphics The address used to listen will be the first address of the family type found for the network. Added two new tests with the new attribute in place. Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 14 - docs/schemas/domaincommon.rng | 8 + src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 10 +++ .../qemuxml2argv-graphics-listen-network-ipv4.xml | 35 ++ .../qemuxml2argv-graphics-listen-network-ipv6.xml | 35 ++ tests/qemuxml2xmltest.c| 2 ++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 335763f..262c576 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4553,7 +4553,7 @@ qemu-kvm -net nic,model=? /dev/null lt;graphics type='rdp' autoport='yes' multiUser='yes' /gt; lt;graphics type='desktop' fullscreen='yes'/gt; lt;graphics type='spice'gt; - lt;listen type='network' network='rednet'/gt; + lt;listen type='network' network='rednet' family='ipv4'/gt; lt;/graphicsgt; lt;/devicesgt; .../pre @@ -4793,6 +4793,18 @@ qemu-kvm -net nic,model=? /dev/null the first forward dev will be used. /dd /dl +dl + dtcodefamily/code/dt + ddif codetype='network'/code, the codefamily/code +attribute may contain the IP family. The codefamily/code +can be set to either codeipv4/code or codeipv6/code. +This advises the graphics device which IP address family +to use as listen address for the network. The listen address +used will be the first found address of the codefamily/code +type defined for the host. +span class=sinceSince 1.2.14/span + /dd +/dl h4a name=elementsVideoVideo devices/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 56ea6a4..87582cc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2926,6 +2926,14 @@ ref name=addrIPorName/ /attribute /optional +optional + attribute name=family +choice + valueipv4/value + valueipv6/value +/choice + /attribute +/optional /group /choice /element diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc8616b..70a3525 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -522,6 +522,12 @@ VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, address, network) +VIR_ENUM_IMPL(virDomainGraphicsListenFamily, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST, + none, + ipv4, + ipv6) + VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST, default, @@ -9547,6 +9553,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *address = virXMLPropString(node, address); char *network = virXMLPropString(node, network); char *fromConfig = virXMLPropString(node, fromConfig); +char *family =
[libvirt] [PATCH v3 15/15] bridge_driver: Use more of networkObjFromNetwork
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 c6957c3..ec6948b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -120,7 +120,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,
@@ -2971,12 +2970,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;
@@ -3037,12 +3032,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;
@@ -3191,13 +3182,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;
@@ -3223,13 +3209,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;
@@ -3340,13 +3321,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 v3 11/15] struct _virNetworkDriverState: Annotate items
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 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network/bridge_driver_platform.h
b/src/network/bridge_driver_platform.h
index b7492e6..d9cf6a8 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -34,8 +34,10 @@
struct _virNetworkDriverState {
virMutex lock;
+/* Immutable pointer, self-locking APIs */
virNetworkObjListPtr networks;
+/* Immutable pointer, Immutable objects */
char *networkConfigDir;
char *networkAutostartDir;
char *stateDir;
@@ -44,6 +46,7 @@ struct _virNetworkDriverState {
char *radvdStateDir;
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
[libvirt] [PATCH v3 10/15] virNetworkObjFindBy*: Return an reference to found object
This patch turns both virNetworkObjFindByUUID() and
virNetworkObjFindByName() to return an referenced object so that
even if caller unlocks it, it's for sure that object won't
disappear meanwhile. Especially if the object (in general) is
locked and unlocked during the caller run.
Moreover, this commit is nicely small, since the object unrefing
can be done in virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 15 ++-
src/network/bridge_driver.c | 18 +-
src/test/test_driver.c | 10 --
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 98342f9..a2aa4dc 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -143,6 +143,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
return;
virObjectUnlock(*net);
+virObjectUnref(*net);
*net = NULL;
}
@@ -175,7 +176,7 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
ret = virHashLookup(nets-objs, uuidstr);
if (ret)
-virObjectLock(ret);
+virObjectRef(ret);
return ret;
}
@@ -188,6 +189,8 @@ virNetworkObjFindByUUID(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByUUIDLocked(nets, uuid);
virObjectUnlock(nets);
+if (ret)
+virObjectLock(ret);
return ret;
}
@@ -214,7 +217,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
ret = virHashSearch(nets-objs, virNetworkObjSearchName, name);
if (ret)
-virObjectLock(ret);
+virObjectRef(ret);
return ret;
}
@@ -227,6 +230,8 @@ virNetworkObjFindByName(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByNameLocked(nets, name);
virObjectUnlock(nets);
+if (ret)
+virObjectLock(ret);
return ret;
}
@@ -491,6 +496,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
virObjectLock(nets);
if ((network = virNetworkObjFindByNameLocked(nets, def-name))) {
virObjectUnlock(nets);
+virObjectLock(network);
virNetworkObjAssignDef(network, def, live);
return network;
}
@@ -507,13 +513,13 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
network-def = def;
network-persistent = !live;
+virObjectRef(network);
virObjectUnlock(nets);
return network;
error:
virObjectUnlock(nets);
-virObjectUnlock(network);
-virObjectUnref(network);
+virNetworkObjEndAPI(network);
return NULL;
}
@@ -687,7 +693,6 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
virObjectLock(nets);
virObjectLock(net);
virHashRemoveEntry(nets-objs, uuidstr);
-virObjectUnlock(net);
virObjectUnlock(nets);
virObjectUnref(net);
}
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 97fda3f..5ef9910 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2939,7 +2939,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn,
const char *xml)
if (networkStartNetwork(network) 0) {
virNetworkRemoveInactive(driver-networks,
network);
-network = NULL;
goto cleanup;
}
@@ -2988,7 +2987,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn,
const char *xml)
if (virNetworkSaveConfig(driver-networkConfigDir, def) 0) {
if (!virNetworkObjIsActive(network)) {
virNetworkRemoveInactive(driver-networks, network);
-network = NULL;
goto cleanup;
}
/* if network was active already, just undo new persistent
@@ -3053,11 +3051,8 @@ networkUndefine(virNetworkPtr net)
VIR_INFO(Undefining network '%s', network-def-name);
if (!active) {
-if (networkRemoveInactive(network) 0) {
-network = NULL;
+if (networkRemoveInactive(network) 0)
goto cleanup;
-}
-network = NULL;
} else {
/* if the network still exists, it was active, and we need to make
@@ -3314,13 +3309,10 @@ static int networkDestroy(virNetworkPtr net)
VIR_NETWORK_EVENT_STOPPED,
0);
-if (!network-persistent) {
-if (networkRemoveInactive(network) 0) {
-network = NULL;
-ret = -1;
-goto cleanup;
-}
-network = NULL;
+if (!network-persistent
+networkRemoveInactive(network) 0) {
+ret = -1;
+goto cleanup;
}
cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 72f40ed..90af80a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn)
goto error;
}
netobj-active = 1;
-virObjectUnlock(netobj);
+virNetworkObjEndAPI(netobj);
if (!(interfacedef =
[libvirt] [PATCH v3 12/15] bridge_driver: Drop networkDriverLock() from everywhere
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,
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,
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,
conn);
-networkDriverUnlock();
return nactive;
}
@@ -2548,12 +2511,10 @@ static int networkConnectListNetworks(virConnectPtr
conn, char **const names, in
if (virConnectListNetworksEnsureACL(conn) 0)
return -1;
-networkDriverLock();
got = virNetworkObjListGetNames(driver-networks,
true, names, nnames,
virConnectListNetworksCheckACL,
conn);
-networkDriverUnlock();
return got;
}
[libvirt] [PATCH v3 09/15] virNetworkObjListPtr: Make APIs self-locking
Every API that touches internal structure of the object must lock
the object first. Not every API that has the object as an
argument needs to do that though. Some APIs just pass the object
to lower layers which, however, must lock the object then. Look
at the code, you'll get my meaning soon.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index be5cf0e..98342f9 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -488,13 +488,17 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
virNetworkObjPtr network;
char uuidstr[VIR_UUID_STRING_BUFLEN];
-if ((network = virNetworkObjFindByName(nets, def-name))) {
+virObjectLock(nets);
+if ((network = virNetworkObjFindByNameLocked(nets, def-name))) {
+virObjectUnlock(nets);
virNetworkObjAssignDef(network, def, live);
return network;
}
-if (!(network = virNetworkObjNew()))
+if (!(network = virNetworkObjNew())) {
+virObjectUnlock(nets);
return NULL;
+}
virObjectLock(network);
virUUIDFormat(def-uuid, uuidstr);
@@ -503,9 +507,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
network-def = def;
network-persistent = !live;
+virObjectUnlock(nets);
return network;
error:
+virObjectUnlock(nets);
virObjectUnlock(network);
virObjectUnref(network);
return NULL;
@@ -676,8 +682,14 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(net-def-uuid, uuidstr);
+virObjectRef(net);
virObjectUnlock(net);
+virObjectLock(nets);
+virObjectLock(net);
virHashRemoveEntry(nets-objs, uuidstr);
+virObjectUnlock(net);
+virObjectUnlock(nets);
+virObjectUnref(net);
}
/* return ips[index], or NULL if there aren't enough ips */
@@ -3195,7 +3207,9 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
virNetworkObjPtr obj;
struct virNetworkBridgeInUseHelperData data = {bridge, skipname};
+virObjectLock(nets);
obj = virHashSearch(nets-objs, virNetworkBridgeInUseHelper, data);
+virObjectUnlock(nets);
return obj != NULL;
}
@@ -4395,6 +4409,7 @@ virNetworkObjListExport(virConnectPtr conn,
int ret = -1;
struct virNetworkObjListData data = { conn, NULL, filter, flags, 0, false};
+virObjectLock(netobjs);
if (nets VIR_ALLOC_N(data.nets, virHashSize(netobjs-objs) + 1) 0)
goto cleanup;
@@ -4412,6 +4427,7 @@ virNetworkObjListExport(virConnectPtr conn,
ret = data.nnets;
cleanup:
+virObjectUnlock(netobjs);
while (data.nets data.nnets)
virObjectUnref(data.nets[--data.nnets]);
@@ -4443,7 +4459,9 @@ virNetworkObjListForEachHelper(void *payload,
* @opaque: pointer to pass to the @callback
*
* Function iterates over the list of network objects and calls
- * passed callback over each one of them.
+ * passed callback over each one of them. You should avoid
+ * calling those virNetworkObjList APIs, which lock the list
+ * again in favor of their virNetworkObj*Locked variants.
*
* Returns: 0 on success, -1 otherwise.
*/
@@ -4453,7 +4471,9 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
void *opaque)
{
struct virNetworkObjListForEachHelperData data = {callback, opaque, 0};
+virObjectLock(nets);
virHashForEach(nets-objs, virNetworkObjListForEachHelper, data);
+virObjectUnlock(nets);
return data.ret;
}
@@ -4515,7 +4535,9 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
struct virNetworkObjListGetHelperData data = {
conn, filter, names, nnames, active, 0, false};
+virObjectLock(nets);
virHashForEach(nets-objs, virNetworkObjListGetHelper, data);
+virObjectUnlock(nets);
if (data.error)
goto cleanup;
@@ -4538,7 +4560,9 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
struct virNetworkObjListGetHelperData data = {
conn, filter, NULL, -1, active, 0, false};
+virObjectLock(nets);
virHashForEach(nets-objs, virNetworkObjListGetHelper, data);
+virObjectUnlock(nets);
return data.got;
}
@@ -4576,5 +4600,7 @@ virNetworkObjListPrune(virNetworkObjListPtr nets,
{
struct virNetworkObjListPruneHelperData data = {flags};
+virObjectLock(nets);
virHashRemoveSet(nets-objs, virNetworkObjListPruneHelper, data);
+virObjectUnlock(nets);
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 02/15] network_conf: Make virNetworkObj actually virObject
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
cfg.mk| 2 -
src/conf/network_conf.c | 110 --
src/conf/network_conf.h | 8 ++-
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 +--
src/parallels/parallels_network.c | 16 +++---
src/test/test_driver.c| 32 +--
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 116 insertions(+), 118 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 07a794a..6885f9e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -160,7 +160,6 @@ useless_free_options = \
--name=virNWFilterRuleDefFree\
--name=virNWFilterRuleInstFree \
--name=virNetworkDefFree \
- --name=virNetworkObjFree \
--name=virNodeDeviceDefFree \
--name=virNodeDeviceObjFree \
--name=virObjectUnref \
@@ -249,7 +248,6 @@ useless_free_options = \
# y virNetworkDefFree
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
-# y virNetworkObjFree
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ae29c47..609990c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -79,11 +79,19 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
hook-script);
+static virClassPtr virNetworkObjClass;
static virClassPtr virNetworkObjListClass;
+static void virNetworkObjDispose(void *obj);
static void virNetworkObjListDispose(void *obj);
static int virNetworkObjOnceInit(void)
{
+if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
+ virNetworkObj,
+ sizeof(virNetworkObj),
+ virNetworkObjDispose)))
+return -1;
+
if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
virNetworkObjList,
sizeof(virNetworkObjList),
@@ -99,7 +107,33 @@ static void
virNetworkObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
{
virNetworkObjPtr obj = payload;
-virNetworkObjFree(obj);
+virObjectUnref(obj);
+}
+
+virNetworkObjPtr
+virNetworkObjNew(void)
+{
+virNetworkObjPtr net;
+
+if (virNetworkObjInitialize() 0)
+return NULL;
+
+if (!(net = virObjectLockableNew(virNetworkObjClass)))
+return NULL;
+
+if (!(net-class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+goto error;
+
+/* The first three class IDs are already taken */
+ignore_value(virBitmapSetBit(net-class_id, 0));
+ignore_value(virBitmapSetBit(net-class_id, 1));
+ignore_value(virBitmapSetBit(net-class_id, 2));
+
+return net;
+
+ error:
+virObjectUnref(net);
+return NULL;
}
virNetworkObjListPtr virNetworkObjListNew(void)
@@ -130,7 +164,7 @@ virNetworkObjPtr
virNetworkObjFindByUUID(virNetworkObjListPtr nets,
ret = virHashLookup(nets-objs, uuidstr);
if (ret)
-virNetworkObjLock(ret);
+virObjectLock(ret);
return ret;
}
@@ -142,10 +176,10 @@ virNetworkObjSearchName(const void *payload,
virNetworkObjPtr net = (virNetworkObjPtr) payload;
int want = 0;
-virNetworkObjLock(net);
+virObjectLock(net);
if (STREQ(net-def-name, (const char *)data))
want = 1;
-virNetworkObjUnlock(net);
+virObjectUnlock(net);
return want;
}
@@ -156,7 +190,7 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
ret = virHashSearch(nets-objs, virNetworkObjSearchName, name);
if (ret)
-virNetworkObjLock(ret);
+virObjectLock(ret);
return ret;
}
@@ -318,18 +352,14 @@ virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def);
}
-void virNetworkObjFree(virNetworkObjPtr net)
+static void
+virNetworkObjDispose(void *obj)
{
-if (!net)
-return;
+virNetworkObjPtr net = obj;
virNetworkDefFree(net-def);
virNetworkDefFree(net-newDef);
virBitmapFree(net-class_id);
-
-virMutexDestroy(net-lock);
-
-VIR_FREE(net);
}
static void
@@ -427,35 +457,21 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
return network;
}
-if (VIR_ALLOC(network) 0)
+if (!(network = virNetworkObjNew()))
return NULL;
-if (virMutexInit(network-lock) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-
[libvirt] [PATCH v3 01/15] virNetworkObjListPtr: Turn list into a hash table
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 382 ++--
1 file changed, 237 insertions(+), 145 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9c1d578..ae29c47 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -54,8 +54,7 @@
struct _virNetworkObjList {
virObject parent;
-size_t count;
-virNetworkObjPtr *objs;
+virHashTablePtr objs;
};
VIR_ENUM_IMPL(virNetworkForward,
@@ -96,6 +95,13 @@ static int virNetworkObjOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virNetworkObj)
+static void
+virNetworkObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
+{
+virNetworkObjPtr obj = payload;
+virNetworkObjFree(obj);
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -106,37 +112,52 @@ virNetworkObjListPtr virNetworkObjListNew(void)
if (!(nets = virObjectNew(virNetworkObjListClass)))
return NULL;
+if (!(nets-objs = virHashCreate(50, virNetworkObjListDataFree))) {
+virObjectUnref(nets);
+return NULL;
+}
+
return nets;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
-size_t i;
+virNetworkObjPtr ret = NULL;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
-for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
-if (!memcmp(nets-objs[i]-def-uuid, uuid, VIR_UUID_BUFLEN))
-return nets-objs[i];
-virNetworkObjUnlock(nets-objs[i]);
-}
+virUUIDFormat(uuid, uuidstr);
-return NULL;
+ret = virHashLookup(nets-objs, uuidstr);
+if (ret)
+virNetworkObjLock(ret);
+return ret;
+}
+
+static int
+virNetworkObjSearchName(const void *payload,
+const void *name ATTRIBUTE_UNUSED,
+const void *data)
+{
+virNetworkObjPtr net = (virNetworkObjPtr) payload;
+int want = 0;
+
+virNetworkObjLock(net);
+if (STREQ(net-def-name, (const char *)data))
+want = 1;
+virNetworkObjUnlock(net);
+return want;
}
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
{
-size_t i;
+virNetworkObjPtr ret = NULL;
-for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
-if (STREQ(nets-objs[i]-def-name, name))
-return nets-objs[i];
-virNetworkObjUnlock(nets-objs[i]);
-}
-
-return NULL;
+ret = virHashSearch(nets-objs, virNetworkObjSearchName, name);
+if (ret)
+virNetworkObjLock(ret);
+return ret;
}
bool
@@ -315,12 +336,8 @@ static void
virNetworkObjListDispose(void *obj)
{
virNetworkObjListPtr nets = obj;
-size_t i;
-for (i = 0; i nets-count; i++)
-virNetworkObjFree(nets-objs[i]);
-
-VIR_FREE(nets-objs);
+virHashFree(nets-objs);
}
/*
@@ -403,6 +420,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
bool live)
{
virNetworkObjPtr network;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
if ((network = virNetworkObjFindByName(nets, def-name))) {
virNetworkObjAssignDef(network, def, live);
@@ -419,8 +437,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
}
virNetworkObjLock(network);
-if (VIR_APPEND_ELEMENT_COPY(nets-objs, nets-count, network) 0 ||
-!(network-class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+if (!(network-class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+goto error;
+
+virUUIDFormat(def-uuid, uuidstr);
+if (virHashAddEntry(nets-objs, uuidstr, network) 0)
goto error;
/* The first three class IDs are already taken */
@@ -600,20 +621,11 @@ virNetworkConfigChangeSetup(virNetworkObjPtr network,
unsigned int flags)
void virNetworkRemoveInactive(virNetworkObjListPtr nets,
virNetworkObjPtr net)
{
-size_t i;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(net-def-uuid, uuidstr);
virNetworkObjUnlock(net);
-for (i = 0; i nets-count; i++) {
-virNetworkObjLock(nets-objs[i]);
-if (nets-objs[i] == net) {
-virNetworkObjUnlock(nets-objs[i]);
-virNetworkObjFree(nets-objs[i]);
-
-VIR_DELETE_ELEMENT(nets-objs, i, nets-count);
-break;
-}
-virNetworkObjUnlock(nets-objs[i]);
-}
+virHashRemoveEntry(nets-objs, uuidstr);
}
/* return ips[index], or NULL if there aren't enough ips */
@@ -3102,23 +3114,39 @@ char *virNetworkConfigFile(const char *dir,
return ret;
}
+struct virNetworkBridgeInUseHelperData {
+const char *bridge;
+const char *skipname;
+};
+
+static int
+virNetworkBridgeInUseHelper(const void *payload,
+const void *name ATTRIBUTE_UNUSED,
+
[libvirt] [PATCH] domain_conf: fix crash in virDomainObjListFindByUUIDInternal
If a domain object is being removed and looked up concurrently we must
ensure we unlock the object before unreferencing it, since the latter
might free the object.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cc8616b..ce0e173 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1068,9 +1068,9 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr
doms,
if (obj) {
virObjectLock(obj);
if (obj-removing) {
+virObjectUnlock(obj);
if (ref)
virObjectUnref(obj);
-virObjectUnlock(obj);
obj = NULL;
}
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] {domain, network}_conf: disable autostart when deleting config
Undefining a running, autostarted domain removes the autostart link, but
dom-autostart is not cleared. If the domain is subsequently redefined,
libvirt thinks it is already autostarted and will not create the link
even if requested:
# virsh dominfo example | grep Autostart
Autostart: enable
# ls /etc/libvirt/qemu/autostart/example.xml
/etc/libvirt/qemu/autostart/example.xml
# virsh undefine example
Domain example has been undefined
# virsh define example.xml
Domain example defined from example.xml
# virsh dominfo example | grep Autostart
Autostart: enable
# virsh autostart example
Domain example marked as autostarted
# ls /etc/libvirt/qemu/autostart/example.xml
ls: cannot access /etc/libvirt/qemu/autostart/example.xml: No such file or
directory
This commit ensures dom-autostart is cleared whenever the config and
autostart link (if present) are removed.
The bridge network driver cleared this flag itself in networkUndefine.
This commit moves this into virNetworkDeleteConfig for symmetry with
virDomainDeleteConfig, and to ensure it is not missed in future network
drivers.
Signed-off-by: Michael Chapman m...@very.puzzling.org
---
src/conf/domain_conf.c | 1 +
src/conf/network_conf.c | 1 +
src/network/bridge_driver.c | 1 -
3 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cc8616b..a8f4ce2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20998,6 +20998,7 @@ virDomainDeleteConfig(const char *configDir,
/* Not fatal if this doesn't work */
unlink(autostartLink);
+dom-autostart = 0;
if (unlink(configFile) 0
errno != ENOENT) {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9c1d578..779a08a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3077,6 +3077,7 @@ int virNetworkDeleteConfig(const char *configDir,
/* Not fatal if this doesn't work */
unlink(autostartLink);
+net-autostart = 0;
if (unlink(configFile) 0) {
virReportSystemError(errno,
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5752acb..5158078 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3050,7 +3050,6 @@ networkUndefine(virNetworkPtr net)
driver-networkAutostartDir,
network) 0)
goto cleanup;
-network-autostart = 0;
event = virNetworkEventLifecycleNew(network-def-name,
network-def-uuid,
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns
-Original Message-
From: Richard Weinberger [mailto:richard.weinber...@gmail.com]
Sent: Wednesday, March 11, 2015 4:56 AM
To: Chen, Hanxiao/陈 晗霄
Cc: libvir-list@redhat.com; Daniel P. Berrange; Gao feng
Subject: Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable
userns
but disable netns
On Mon, Jul 14, 2014 at 12:01 PM, Chen Hanxiao
chenhanx...@cn.fujitsu.com wrote:
kernel commit 7dc5dbc879bd0779924b5132a48b731a0bc04a1e
forbid us doing a fresh mount for sysfs
when enable userns but disable netns.
This patch will create a bind mount in this senario.
Sorry for exhuming an already merged patch but today I ran into a
nasty issue caused by it.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_container.c | 44 +---
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d89677..8a27215 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -815,10 +815,13 @@ static int lxcContainerSetReadOnly(void)
}
-static int lxcContainerMountBasicFS(bool userns_enabled)
+static int lxcContainerMountBasicFS(bool userns_enabled,
+bool netns_disabled)
{
size_t i;
int rc = -1;
+char* mnt_src = NULL;
+int mnt_mflags;
VIR_DEBUG(Mounting basic filesystems);
@@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
bool bindOverReadonly;
virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
+/* When enable userns but disable netns, kernel will
+ * forbid us doing a new fresh mount for sysfs.
+ * So we had to do a bind mount for sysfs instead.
+ */
+if (userns_enabled netns_disabled
+STREQ(mnt-src, sysfs)) {
+if (VIR_STRDUP(mnt_src, /sys) 0) {
+goto cleanup;
+}
This is clearly broken and looks very untested to me.
It's broken now.
But when I submitted this patch last year, it's not.
It will issue this mount call:
mount(/sys, /sys, sysfs, MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_BIND, NULL)
because the code runs after pivot_root(2).
i.e, /sys will be still empty after that and no sysfs at all there.
As libvirt will later remount /sys readonly creating a container will
fail with the most useless error message:
Error: internal error: guest failed to start: Unable to create
directory /sys/fs/: Read-only file system
or
Error: internal error: guest failed to start: Unable to create
directory /sys/fs/cgroup: Read-only file system
Please note that changing /sys to /.oldroot/sys will not solve the
issue as this code runs already in the new
namespace and therefore the old mount tree is locked, thus MS_BIND is
not allowed.
This brings me to the question, why do you handle the netns_disabled
case anyway?
Please check the discussion at:
http://lists.linux-foundation.org/pipermail/containers/2014-July/034721.html
If in the XML file no network is specified just create a new and empty
network namespace.
Bindmounting /sys into the container is a security issue. This is why
mounting sysfs without a netns
was disabled to begin with.
Yes, I tried to propose enable netns by default,
but Dan thought that we should allow containers sharing the host's network:
http://www.redhat.com/archives/libvir-list/2013-August/msg01025.html
So we should allow user create containers without netns,
they should know what they do if they read libvirt's docs
See docs patch describe security considerations:
http://www.redhat.com/archives/libvir-list/2013-September/msg00562.html
Regards,
- Chen
P.S: Sorry for the grumpy mail, I've wasted almost the whole day with
debugging that issue.
--
Thanks,
//richard
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] CfP 10th Workshop on Virtualization in High-Performance Cloud Computing (VHPC '15)
= CALL FOR PAPERS 10th Workshop on Virtualization in High-Performance Cloud Computing (VHPC '15) held in conjunction with Euro-Par 2015, August 24-28, Vienna, Austria (Springer LNCS) = Date: August 25, 2015 Workshop URL: http://vhpc.org Paper Submission Deadline: May 22, 2015 CALL FOR PAPERS Virtualization technologies constitute a key enabling factor for flexible resource management in modern data centers, cloud environments, and increasingly in HPC as well. Providers need to dynamically manage complex infrastructures in a seamless fashion for varying workloads and hosted applications, independently of the customers deploying software or users submitting highly dynamic and heterogeneous workloads. Thanks to virtualization, we have the ability to manage vast computing and networking resources dynamically and close to the marginal cost of providing the services, which is unprecedented in the history of scientific and commercial computing. Various virtualization technologies contribute to the overall picture in different ways: machine virtualization, with its capability to enable consolidation of multiple under-utilized servers with heterogeneous software and operating systems (OSes), and its capability to live-migrate a fully operating virtual machine (VM) with a very short downtime, enables novel and dynamic ways to manage physical servers; OS-level virtualization, with its capability to isolate multiple user-space environments and to allow for their co-existence within the same OS kernel, promises to provide many of the advantages of machine virtualization with high levels of responsiveness and performance; I/O Virtualization allows physical network adapters to take traffic from multiple VMs; network virtualization, with its capability to create logical network overlays that are independent of the underlying physical topology and IP addressing, provides the fundamental ground on top of which evolved network services can be realized with an unprecedented level of dynamicity and flexibility; These technologies have to be inter-mixed and integrated in an intelligent way, to support workloads that are increasingly demanding in terms of absolute performance, responsiveness and interactivity, and have to respect well-specified Service- Level Agreements (SLAs), as needed for industrial-grade provided services. Indeed, among emerging and increasingly interesting application domains for virtualization, we can find big-data application workloads in cloud infrastructures, interactive and real-time multimedia services in the cloud, including real-time big-data streaming platforms such as used in real-time analytics supporting nowadays a plethora of application domains. Distributed cloud infrastructures promise to offer unprecedented responsiveness levels for hosted applications, but that is only possible if the underlying virtualization technologies can overcome most of the latency impairments typical of current virtualized infrastructures (e.g., far worse tail-latency). The Workshop on Virtualization in High-Performance Cloud Computing (VHPC) aims to bring together researchers and industrial practitioners facing the challenges posed by virtualization in order to foster discussion, collaboration, mutual exchange of knowledge and experience, enabling research to ultimately provide novel solutions for virtualized computing systems of tomorrow. The workshop will be one day in length, composed of 20 min paper presentations, each followed by 10 min discussion sections, and lightning talks, limited to 5 minutes. Presentations may be accompanied by interactive demonstrations. TOPICS Topics of interest include, but are not limited to: - Virtualization in supercomputing environments, HPC clusters, cloud HPC and grids - Optimizations of virtual machine monitor platforms, hypervisors and OS-level virtualization - Hypervisor and network virtualization QoS and SLAs - Cloud based network and system management for SDN and NFV - Management, deployment and monitoring of virtualized environments - Performance measurement, modelling and monitoring of virtualized/cloud workloads - Programming models for virtualized environments - Cloud reliability, fault-tolerance, high-availability and security - Heterogeneous virtualized environments, virtualized accelerators, GPUs and co-processors - Optimized communication libraries/protocols in the cloud and for HPC in the cloud - Topology management and optimization for distributed virtualized applications - Cluster provisioning in the cloud and cloud bursting - Adaptation of emerging HPC technologies (high performance networks, RDMA, etc..) - I/O and storage virtualization, virtualization aware file systems - Job scheduling/control/policy in virtualized environments - Checkpointing and migration of
[libvirt] [PATCH 00/12] qemu: Refactor block stats gathering and implement summary stats
Peter Krempa (12): qemu: Use macro to set block stats typed parameters qemu: monitor: Drop parsing of 'errs' from block info qemu: blockstats: Switch to caller allocated hash table test: qemu: Fix qemu monitor test utils to allow testing HMP qemu: monitor: Implement HMP version for listing all block device stats qemu: monitor: Convert common code to a macro qemu: monitor: Count block stats fields in qemuMonitorGetAllBlockStatsInfo qemu: Split out working code qemuDomainBlockStats qemu: blockstats: Add support for totalled block statistics qemu: blockstats: Refactor qemuDomainBlockStatsFlags test: qemu: json: Avoid using the now obsolete functions qemu: monitor: Kill qemuMonitorGetBlockStats(Info,ParamsNumber) src/qemu/qemu_driver.c | 299 --- src/qemu/qemu_monitor.c | 106 ++- src/qemu/qemu_monitor.h | 15 --- src/qemu/qemu_monitor_json.c | 233 - src/qemu/qemu_monitor_json.h | 15 +-- src/qemu/qemu_monitor_text.c | 256 ++-- src/qemu/qemu_monitor_text.h | 16 +-- tests/Makefile.am| 8 +- tests/qemumonitorjsontest.c | 65 -- tests/qemumonitortest.c | 89 - tests/qemumonitortestutils.c | 3 +- tools/virsh.pod | 5 +- 12 files changed, 425 insertions(+), 685 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] qemu: Remove unnecessary virReportError on networkGetNetworkAddress return
On Mon, Mar 09, 2015 at 08:05:01PM -0400, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com Error messages are already set in all code paths returning -1 from networkGetNetworkAddress, so we don't want to overwrite them. Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_command.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) ACK, this one does not depend on the rest of the series. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-perl PATCHv2 0/2] Add virDomainGetIOThreads and virDomainPinIOThread bindings
v1:
http://www.redhat.com/archives/libvir-list/2015-March/msg00352.html
Since I essentially copied the vcpuinfo.pl example to create the
iothreadsinfo.pl example, I figured I'd resend the patches again
with the changes that ended up working which are slightly different
than the review comment. The major difference being I needed to limit
the unpack bits format. Instead of b*, I found b$nodeinfo-{cpus}
would print the just the cpu count bits.
Made the same change to the vcpuinfo.pl example. I didn't modify the
Virt.xs Safefree(info) check although it seems it would be possible
if desired.
John Ferlan (2):
Add virDomainGetIOThreads and virDomainPinIOThread bindings
Update the vcpuinfo.pl example
Changes | 3 ++-
Virt.xs | 42 ++
examples/iothreadsinfo.pl | 33 +
examples/vcpuinfo.pl | 6 ++
lib/Sys/Virt/Domain.pm| 17 +
5 files changed, 96 insertions(+), 5 deletions(-)
create mode 100644 examples/iothreadsinfo.pl
--
2.1.0
--
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
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.
Is your objection the comment? Should it be striken or restated?
+ * so check if we can reset setting.
+ */
+if (virBitmapIsAllSet(pcpumap))
+doReset = true;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+
+if (priv-iothreadpids == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(IOThread affinity is not supported));
+goto endjob;
+}
+
+if (iothread_id priv-niothreadpids) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(iothread value out of range %d %d),
+ iothread_id, priv-niothreadpids);
+goto endjob;
+}
+
+if (vm-def-cputune.iothreadspin) {
+/* The VcpuPinDefCopy works for IOThreads too */
Maybe it should be renamed to something like virDomainPinDefCopy then?
Perhaps, but that seems outside the scope of these changes. The 'reuse'
of virDomainVcpuPinDefPtr by the IOThreads code was an optimization
that could also then be generalized I suppose (eg change from
virDomainVcpuPinDefPtr to virDomainPinDefPtr), but that's a much more
invasive change which would seemingly require change the structure
vcpuid value to just id.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] qemu: Split out working code qemuDomainBlockStats
Extract the code to look up the disk alias and return the block stats
struct so that it can be reused later in qemuDomainBlockStatsFlags.
---
src/qemu/qemu_driver.c | 122 +
1 file changed, 82 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 06168b1..e280cdf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10480,6 +10480,80 @@ qemuDomainBlockResize(virDomainPtr dom,
return ret;
}
+
+/**
+ * qemuDomainBlocksStatsGather:
+ * @driver: driver object
+ * @vm: domain object
+ * @path: to gather the statistics for
+ * @retstats: returns pointer to structure holding the stats
+ *
+ * Gathers the block statistics for use in qemuDomainBlockStats* APIs.
+ *
+ * Returns -1 on error; number of filled block statistics on success.
+ */
+static int
+qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+const char *path,
+qemuBlockStatsPtr *retstats)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virDomainDiskDefPtr disk;
+virHashTablePtr blockstats = NULL;
+qemuBlockStatsPtr stats;
+int nstats;
+char *diskAlias = NULL;
+int ret = -1;
+
+if (*path) {
+int idx;
+
+if ((idx = virDomainDiskIndexByName(vm-def, path, false)) 0) {
+virReportError(VIR_ERR_INVALID_ARG, _(invalid path: %s), path);
+goto cleanup;
+}
+disk = vm-def-disks[idx];
+
+if (!disk-info.alias) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(missing disk device alias name for %s),
disk-dst);
+goto cleanup;
+}
+
+if (VIR_STRDUP(diskAlias, disk-info.alias) 0)
+goto cleanup;
+} else {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(summary statistics are not supported yet));
+goto cleanup;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+nstats = qemuMonitorGetAllBlockStatsInfo(priv-mon, blockstats, false);
+if (qemuDomainObjExitMonitor(driver, vm) 0 || nstats 0)
+goto cleanup;
+
+if (VIR_ALLOC(*retstats) 0)
+goto cleanup;
+
+if (!(stats = virHashLookup(blockstats, diskAlias))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(cannot find statistics for device '%s'), diskAlias);
+goto cleanup;
+}
+
+**retstats = *stats;
+
+ret = nstats;
+
+ cleanup:
+VIR_FREE(diskAlias);
+virHashFree(blockstats);
+return ret;
+}
+
+
/* This uses the 'info blockstats' monitor command which was
* integrated into both qemu kvm in late 2007. If the command is
* not supported we detect this and return the appropriate error.
@@ -10490,18 +10564,9 @@ qemuDomainBlockStats(virDomainPtr dom,
virDomainBlockStatsPtr stats)
{
virQEMUDriverPtr driver = dom-conn-privateData;
-int idx;
+qemuBlockStatsPtr blockstats = NULL;
int ret = -1;
virDomainObjPtr vm;
-virDomainDiskDefPtr disk = NULL;
-qemuDomainObjPrivatePtr priv;
-char *diskAlias = NULL;
-
-if (!*path) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
- _(summary statistics are not supported yet));
-return ret;
-}
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -10518,47 +10583,24 @@ qemuDomainBlockStats(virDomainPtr dom,
goto endjob;
}
-if ((idx = virDomainDiskIndexByName(vm-def, path, false)) 0) {
-virReportError(VIR_ERR_INVALID_ARG,
- _(invalid path: %s), path);
+if (qemuDomainBlocksStatsGather(driver, vm, path, blockstats) 0)
goto endjob;
-}
-disk = vm-def-disks[idx];
-
-if (!disk-info.alias) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(missing disk device alias name for %s), disk-dst);
-goto endjob;
-}
-
-if (VIR_STRDUP(diskAlias, disk-info.alias) 0)
-goto endjob;
-
-priv = vm-privateData;
+stats-rd_req = blockstats-rd_req;
+stats-rd_bytes = blockstats-rd_bytes;
+stats-wr_req = blockstats-wr_req;
+stats-wr_bytes = blockstats-wr_bytes;
/* qemu doesn't report the error count */
stats-errs = -1;
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorGetBlockStatsInfo(priv-mon,
- diskAlias,
- stats-rd_req,
- stats-rd_bytes,
- NULL,
- stats-wr_req,
- stats-wr_bytes,
- NULL,
- NULL,
- NULL);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-
[libvirt] [PATCH 07/12] qemu: monitor: Count block stats fields in qemuMonitorGetAllBlockStatsInfo
Our virDomainBlockStatsFlags API uses the old approach where, when it's
called without the typed parameter array, returs the count of parameters
supported by qemu.
The supported parameter count is obtained via separate monitor calls
which is a waste since we can calculate it when gathering the data.
This patch adds code to the qemuMonitorGetAllBlockStatsInfo workers that
allows to track the count of supported fields reported by qemu and will
allow to remove the old duplicate code.
---
src/qemu/qemu_monitor.c | 26 +++---
src/qemu/qemu_monitor_json.c | 15 +++
src/qemu/qemu_monitor_text.c | 10 +-
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 149e743..e4ff06e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1848,14 +1848,24 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
}
-/* Creates a hash table in 'ret_stats' with all block stats.
- * Returns 0 on error, 0 on success.
+/**
+ * qemuMonitorGetAllBlockStatsInfo:
+ * @mon: monitor object
+ * @ret_stats: pointer that is filled with a hash table containing the stats
+ * @backingChain: recurse into the bakcing chain of devices
+ *
+ * Creates a hash table in @ret_stats with block stats of all devices. In case
+ * @backingChain is true @ret_stats will additionally contain stats for
+ * backing chain members of block devices.
+ *
+ * Returns 0 on error, count of supported block stats fields on success.
*/
int
qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
virHashTablePtr *ret_stats,
bool backingChain)
{
+int ret = -1;
VIR_DEBUG(mon=%p ret_stats=%p, backing=%d, mon, ret_stats, backingChain);
if (!mon-json) {
@@ -1868,8 +1878,8 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
goto error;
if (mon-json) {
-if (qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain)
0)
-goto error;
+ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats,
+ backingChain);
} else {
if (backingChain) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
@@ -1878,11 +1888,13 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
goto error;
}
- if (qemuMonitorTextGetAllBlockStatsInfo(mon, *ret_stats) 0)
- goto error;
+ ret = qemuMonitorTextGetAllBlockStatsInfo(mon, *ret_stats);
}
-return 0;
+if (ret 0)
+goto error;
+
+return ret;
error:
virHashFree(*ret_stats);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 52e1ad6..76baaf6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1773,6 +1773,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,
qemuBlockStatsPtr bstats = NULL;
virJSONValuePtr stats;
int ret = -1;
+int nstats = 0;
char *entry_name = qemuDomainStorageAlias(dev_name, depth);
virJSONValuePtr backing;
@@ -1791,6 +1792,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,
#define QEMU_MONITOR_JSON_BLOCK_STAT(NAME, VAR, MANDATORY)
\
if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) {
\
+nstats++;
\
if (virJSONValueObjectGetNumberLong(stats, NAME, VAR) 0) {
\
virReportError(VIR_ERR_INTERNAL_ERROR,
\
_(cannot read %s statistic), NAME);
\
@@ -1820,7 +1822,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,
hash, true) 0)
goto cleanup;
-ret = 0;
+ret = nstats;
cleanup:
VIR_FREE(bstats);
VIR_FREE(entry_name);
@@ -1834,6 +1836,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
bool backingChain)
{
int ret = -1;
+int nstats = 0;
int rc;
size_t i;
virJSONValuePtr cmd;
@@ -1874,13 +1877,17 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
goto cleanup;
}
-if (qemuMonitorJSONGetOneBlockStatsInfo(dev, dev_name, 0, hash,
-backingChain) 0)
+rc = qemuMonitorJSONGetOneBlockStatsInfo(dev, dev_name, 0, hash,
+ backingChain);
+
+if (rc 0)
goto cleanup;
+if (rc nstats)
+nstats = rc;
}
-ret = 0;
+ret = nstats;
cleanup:
virJSONValueFree(cmd);
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 8b2ef90..203859c 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -854,6 +854,8 @@
[libvirt] [libvirt-perl PATCHv2 2/2] Update the vcpuinfo.pl example
Based on the review of my iothreadsinfo.pl example which I took from
vcpuinfo.pl, this patch updates the vcpuinfo.pl to follow the changes
made in iothreadsinfo.pl
Signed-off-by: John Ferlan jfer...@redhat.com
---
Changes | 1 +
examples/vcpuinfo.pl | 6 ++
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/Changes b/Changes
index 88f648c..124b798 100644
--- a/Changes
+++ b/Changes
@@ -3,6 +3,7 @@ Revision history for perl module Sys::Virt
1.2.14 2015-00-00
- Add virDomainGetIOThreads and virDomainPinIOThread API bindings
+ - Update the vcpuinfo.pl example to use unpack
1.2.13 2015-03-05
diff --git a/examples/vcpuinfo.pl b/examples/vcpuinfo.pl
index 6b07433..79dcf89 100644
--- a/examples/vcpuinfo.pl
+++ b/examples/vcpuinfo.pl
@@ -21,10 +21,8 @@ foreach my $dom (sort { $a-get_id = $b-get_id }
$con-list_all_domains) {
foreach (sort { $a cmp $b } keys %{$info}) {
if ($_ eq affinity) {
print , $_, : ;
- my @mask = split //, $info-{$_};
- for (my $p = 0 ; $p $nodeinfo-{cpus} ; $p++) {
- print ((ord($mask[$p/8]) (1 ($p % 8))) ? 1 : 0);
- }
+my @mask = split(//, unpack(b$nodeinfo-{cpus},
$info-{$_}));
+print join (, @mask), \n;
print \n;
} else {
print , $_, : , $info-{$_}, \n;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-perl PATCHv2 1/2] Add virDomainGetIOThreads and virDomainPinIOThread bindings
Test results in the following output:
$ perl examples/iothreadsinfo.pl
Addr
VMM type: QEMU
...
Domain: {
ID: 2 'f18iothr'
UUID: fb9f7826-b5d7-4f74-b962-7181ef3fc9ec
IOThread: {
affinity: 0010
number: 1
}
IOThread: {
affinity: 0001
number: 2
}
IOThread: {
affinity: 1100
number: 3
}
}
Signed-off-by: John Ferlan jfer...@redhat.com
---
Changes | 2 +-
Virt.xs | 42 ++
examples/iothreadsinfo.pl | 33 +
lib/Sys/Virt/Domain.pm| 17 +
4 files changed, 93 insertions(+), 1 deletion(-)
create mode 100644 examples/iothreadsinfo.pl
diff --git a/Changes b/Changes
index b62ee24..88f648c 100644
--- a/Changes
+++ b/Changes
@@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt
1.2.14 2015-00-00
- - XXX
+ - Add virDomainGetIOThreads and virDomainPinIOThread API bindings
1.2.13 2015-03-05
diff --git a/Virt.xs b/Virt.xs
index f9ec7a4..56e143d 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -5014,6 +5014,48 @@ get_emulator_pin_info(dom, flags=0)
RETVAL
+void
+get_iothread_info(dom, flags=0)
+ virDomainPtr dom;
+ unsigned int flags;
+ PREINIT:
+ virDomainIOThreadInfoPtr *iothrinfo;
+ int niothreads;
+ int i;
+ PPCODE:
+ if ((niothreads = virDomainGetIOThreadsInfo(dom, iothrinfo,
+ flags)) 0)
+ _croak_error();
+
+ EXTEND(SP, niothreads);
+ for (i = 0 ; i niothreads ; i++) {
+ HV *rec = newHV();
+ (void)hv_store(rec, number, 6,
+ newSViv(iothrinfo[i]-iothread_id), 0);
+ (void)hv_store(rec, affinity, 8,
+ newSVpvn((char*)iothrinfo[i]-cpumap,
+ iothrinfo[i]-cpumaplen), 0);
+ PUSHs(newRV_noinc((SV *)rec));
+ }
+
+ Safefree(iothrinfo);
+
+
+void
+pin_iothread(dom, iothread_id, mask, flags=0)
+ virDomainPtr dom;
+ unsigned int iothread_id;
+ SV *mask;
+ unsigned int flags;
+PREINIT:
+ STRLEN masklen;
+ unsigned char *maps;
+ PPCODE:
+ maps = (unsigned char *)SvPV(mask, masklen);
+ if (virDomainPinVcpuFlags(dom, iothread_id, maps, masklen, flags) 0)
+ _croak_error();
+
+
int
num_of_snapshots(dom, flags=0)
virDomainPtr dom;
diff --git a/examples/iothreadsinfo.pl b/examples/iothreadsinfo.pl
new file mode 100644
index 000..97c5eb6
--- /dev/null
+++ b/examples/iothreadsinfo.pl
@@ -0,0 +1,33 @@
+# -*- perl -*-
+use strict;
+use warnings;
+use Sys::Virt;
+
+my $addr = @ARGV ? shift @ARGV : ;
+print Addr $addr\n;
+my $con = Sys::Virt-new(address = $addr, readonly = 1);
+
+print VMM type: , $con-get_type(), \n;
+
+foreach my $dom (sort { $a-get_id = $b-get_id } $con-list_all_domains) {
+print Domain: {\n;
+print ID: , $dom-get_id(), ' , $dom-get_name(), '\n;
+print UUID: , $dom-get_uuid_string(), \n;
+my $nodeinfo = $con-get_node_info;
+my @info = $dom-get_iothread_info(Sys::Virt::Domain::AFFECT_CONFIG);
+
+foreach my $info (@info) {
+ print IOThread: {\n;
+ foreach (sort { $a cmp $b } keys %{$info}) {
+ if ($_ eq affinity) {
+ print , $_, : ;
+my @bits = split(//, unpack(b$nodeinfo-{cpus},
$info-{$_}));
+print join (, @bits), \n;
+ } else {
+ print , $_, : , $info-{$_}, \n;
+ }
+ }
+ print }\n;
+}
+print }\n;
+}
diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index 5c8ef47..062c012 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -1216,6 +1216,23 @@ the physical CPUa, 8 cpus per character. To create a
suitable
bitstring, use the Cvec function with a value of C1 for the
CBITS parameter.
+=item @iothreadinfo = $dom-get_iothread_info($flags=0)
+
+Obtain information about the state of all IOThreads in a running
+guest domain. The returned list will have one element for each IOThread,
+where each elements contains a hash reference. The keys in the hash
+are, Cnumber the IOThread number and Caffinity giving the allowed
+schedular placement. The value for Caffinity is a
+string representing a bitmask against physical CPUs, 8 cpus per
+character. To extract the bits use the Cunpack function with
+the Cb* template.
+
+=item $dom-pin_iothread($iothread, $mask)
+
+Pin the IOThread given by index C$iothread to physical CPUs
+given by C$mask. The C$mask is a string representing a bitmask
+against physical CPUs, 8 cpus per character.
+
=item my @stats = $dom-get_cpu_stats($startCpu, $numCpus, $flags=0)
Requests the guests host physical CPU usage statistics, starting
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] test: qemu: Fix qemu monitor test utils to allow testing HMP
qemu HMP commands sent by libvirt are terminated just by a '\r'. The
fake monitor used in tests wasn't prepared to handle this and the
communication would hang on an attempt to do a HMP conversation.
Add a special case for handling commands separated by \r in case HMP is
used.
---
tests/qemumonitortestutils.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 8155a69..3d34942 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -251,7 +251,8 @@ qemuMonitorTestIO(virNetSocketPtr sock,
* if so, handle that command
*/
t1 = test-incoming;
-while ((t2 = strstr(t1, \n))) {
+while ((t2 = strstr(t1, \n)) ||
+(!test-json (t2 = strstr(t1, \r {
*t2 = '\0';
if (qemuMonitorTestProcessCommand(test, t1) 0) {
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/12] qemu: monitor: Convert common code to a macro
The function that is extracting block stats data from the QMP monitor
reply contains a lot of repeated code. Since I'd be changing each of the
copies in the next patch, lets convert it to a macro right away.
---
src/qemu/qemu_monitor_json.c | 77 ++--
1 file changed, 17 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c88c7c3..52e1ad6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1789,66 +1789,23 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,
goto cleanup;
}
-if (virJSONValueObjectGetNumberLong(stats, rd_bytes,
-bstats-rd_bytes) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- rd_bytes);
-goto cleanup;
-}
-if (virJSONValueObjectGetNumberLong(stats, rd_operations,
-bstats-rd_req) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- rd_operations);
-goto cleanup;
-}
-if (virJSONValueObjectHasKey(stats, rd_total_time_ns)
-(virJSONValueObjectGetNumberLong(stats, rd_total_time_ns,
- bstats-rd_total_times) 0)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- rd_total_time_ns);
-goto cleanup;
-}
-if (virJSONValueObjectGetNumberLong(stats, wr_bytes,
-bstats-wr_bytes) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- wr_bytes);
-goto cleanup;
-}
-if (virJSONValueObjectGetNumberLong(stats, wr_operations,
-bstats-wr_req) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- wr_operations);
-goto cleanup;
-}
-if (virJSONValueObjectHasKey(stats, wr_total_time_ns)
-(virJSONValueObjectGetNumberLong(stats, wr_total_time_ns,
- bstats-wr_total_times) 0)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- wr_total_time_ns);
-goto cleanup;
-}
-if (virJSONValueObjectHasKey(stats, flush_operations)
-(virJSONValueObjectGetNumberLong(stats, flush_operations,
- bstats-flush_req) 0)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- flush_operations);
-goto cleanup;
-}
-if (virJSONValueObjectHasKey(stats, flush_total_time_ns)
-(virJSONValueObjectGetNumberLong(stats, flush_total_time_ns,
- bstats-flush_total_times) 0)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot read %s statistic),
- flush_total_time_ns);
-goto cleanup;
-}
+#define QEMU_MONITOR_JSON_BLOCK_STAT(NAME, VAR, MANDATORY)
\
+if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) {
\
+if (virJSONValueObjectGetNumberLong(stats, NAME, VAR) 0) {
\
+virReportError(VIR_ERR_INTERNAL_ERROR,
\
+ _(cannot read %s statistic), NAME);
\
+goto cleanup;
\
+}
\
+}
+ QEMU_MONITOR_JSON_BLOCK_STAT(rd_bytes, bstats-rd_bytes, true);
+ QEMU_MONITOR_JSON_BLOCK_STAT(wr_bytes, bstats-wr_bytes, true);
+ QEMU_MONITOR_JSON_BLOCK_STAT(rd_operations, bstats-rd_req, true);
+ QEMU_MONITOR_JSON_BLOCK_STAT(wr_operations, bstats-wr_req, true);
+ QEMU_MONITOR_JSON_BLOCK_STAT(rd_total_time_ns, bstats-rd_total_times,
false);
+ QEMU_MONITOR_JSON_BLOCK_STAT(wr_total_time_ns, bstats-wr_total_times,
false);
+ QEMU_MONITOR_JSON_BLOCK_STAT(flush_operations, bstats-flush_req,
false);
+ QEMU_MONITOR_JSON_BLOCK_STAT(flush_total_time_ns,
bstats-flush_total_times, false);
+#undef QEMU_MONITOR_JSON_BLOCK_STAT
/* it's ok to not have this information here. Just skip silently. */
qemuMonitorJSONDevGetBlockExtent(dev, bstats-wr_highest_offset);
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] qemu: Use macro to set block stats typed parameters
All the setters are the same code except for parameter name and
variable, so they can be converted to a macro to save a ton of
duplicated code.
---
src/qemu/qemu_driver.c | 78 +++---
1 file changed, 16 insertions(+), 62 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ceceafa..e94275f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10575,7 +10575,6 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
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;
-virTypedParameterPtr param;
char *diskAlias = NULL;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -10656,73 +10655,28 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
tmp = 0;
ret = -1;
-if (tmp *nparams wr_bytes != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,
-VIR_TYPED_PARAM_LLONG, wr_bytes) 0)
-goto endjob;
-tmp++;
+/* the following macro shall not be used with side effects statements */
+#define QEMU_BLOCK_STAT(VAR, NAME)\
+if (tmp *nparams (VAR) != -1) { \
+if (virTypedParameterAssign(params + tmp, NAME, VIR_TYPED_PARAM_LLONG,\
+(VAR)) 0) \
+goto endjob; \
+tmp++;\
}
-if (tmp *nparams wr_req != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,
-VIR_TYPED_PARAM_LLONG, wr_req) 0)
-goto endjob;
-tmp++;
-}
+QEMU_BLOCK_STAT(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES);
+QEMU_BLOCK_STAT(wr_req, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ);
-if (tmp *nparams rd_bytes != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES,
-VIR_TYPED_PARAM_LLONG, rd_bytes) 0)
-goto endjob;
-tmp++;
-}
+QEMU_BLOCK_STAT(rd_bytes, VIR_DOMAIN_BLOCK_STATS_READ_BYTES);
+QEMU_BLOCK_STAT(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ);
-if (tmp *nparams rd_req != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ,
-VIR_TYPED_PARAM_LLONG, rd_req) 0)
-goto endjob;
-tmp++;
-}
+QEMU_BLOCK_STAT(flush_req, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ);
-if (tmp *nparams flush_req != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ,
-VIR_TYPED_PARAM_LLONG, flush_req) 0)
-goto endjob;
-tmp++;
-}
+QEMU_BLOCK_STAT(wr_total_times, VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES);
+QEMU_BLOCK_STAT(rd_total_times, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES);
+QEMU_BLOCK_STAT(flush_total_times,
VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES);
-if (tmp *nparams wr_total_times != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES,
-VIR_TYPED_PARAM_LLONG, wr_total_times) 0)
-goto endjob;
-tmp++;
-}
-
-if (tmp *nparams rd_total_times != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES,
-VIR_TYPED_PARAM_LLONG, rd_total_times) 0)
-goto endjob;
-tmp++;
-}
-
-if (tmp *nparams flush_total_times != -1) {
-param = params[tmp];
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES,
-VIR_TYPED_PARAM_LLONG,
-flush_total_times) 0)
-goto endjob;
-tmp++;
-}
+#undef QEMU_BLOCK_STAT
/* Field 'errs' is meaningless for QEMU, won't set it. */
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] test: qemu: json: Avoid using the now obsolete functions
Use the new single function instead of calling
qemuMonitorJSONGetBlockStatsInfo and
qemuMonitorJSONGetBlockStatsParamsNumber. This will allow to delete the
functions later while still maintaining coverage.
---
tests/qemumonitorjsontest.c | 62 +++--
1 file changed, 21 insertions(+), 41 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index da9cd6c..1380df1 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1435,11 +1435,9 @@
testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data)
{
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+virHashTablePtr blockstats = NULL;
+qemuBlockStatsPtr stats;
int ret = -1;
-long long rd_req, rd_bytes, rd_total_times;
-long long wr_req, wr_bytes, wr_total_times;
-long long flush_req, flush_total_times;
-int nparams;
unsigned long long extent;
const char *reply =
@@ -1537,22 +1535,24 @@
testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data)
if (qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
-qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
-qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
-qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
qemuMonitorTestAddItem(test, query-blockstats, reply) 0)
goto cleanup;
#define CHECK0(var, value) \
-if (var != value) { \
+if (stats-var != value) { \
virReportError(VIR_ERR_INTERNAL_ERROR, \
Invalid #var value: %lld, expected %d, \
- var, value); \
+ stats-var, value); \
goto cleanup; \
}
-#define CHECK(RD_REQ, RD_BYTES, RD_TOTAL_TIMES, WR_REQ, WR_BYTES,
WR_TOTAL_TIMES, \
- FLUSH_REQ, FLUSH_TOTAL_TIMES) \
+#define CHECK(NAME, RD_REQ, RD_BYTES, RD_TOTAL_TIMES, WR_REQ, WR_BYTES,
\
+ WR_TOTAL_TIMES, FLUSH_REQ, FLUSH_TOTAL_TIMES)
\
+if (!(stats = virHashLookup(blockstats, NAME))) {
\
+virReportError(VIR_ERR_INTERNAL_ERROR,
\
+ block stats for device '%s' is missing, NAME);
\
+goto cleanup;
\
+}
\
CHECK0(rd_req, RD_REQ) \
CHECK0(rd_bytes, RD_BYTES) \
CHECK0(rd_total_times, RD_TOTAL_TIMES) \
@@ -1562,41 +1562,20 @@
testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data)
CHECK0(flush_req, FLUSH_REQ) \
CHECK0(flush_total_times, FLUSH_TOTAL_TIMES)
-if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test),
virtio-disk0,
- rd_req, rd_bytes, rd_total_times,
- wr_req, wr_bytes, wr_total_times,
- flush_req, flush_total_times) 0)
-goto cleanup;
-
-CHECK(1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0)
-
-if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test),
virtio-disk1,
- rd_req, rd_bytes, rd_total_times,
- wr_req, wr_bytes, wr_total_times,
- flush_req, flush_total_times) 0)
+if (qemuMonitorGetAllBlockStatsInfo(qemuMonitorTestGetMonitor(test),
+blockstats, false) 0)
goto cleanup;
-CHECK(85, 348160, 8232156, 0, 0, 0, 0, 0)
-
-if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test),
ide0-1-0,
- rd_req, rd_bytes, rd_total_times,
- wr_req, wr_bytes, wr_total_times,
- flush_req, flush_total_times) 0)
-goto cleanup;
-
-CHECK(16, 49250, 1004952, 0, 0, 0, 0, 0)
-
-if
(qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorTestGetMonitor(test),
- nparams) 0)
-goto cleanup;
-
-if (nparams != 8) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- Invalid number of stats: %d, expected 8,
- nparams);
+if (!blockstats) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ qemuMonitorJSONGetBlockStatsInfo didn't return stats);
goto cleanup;
}
+CHECK(virtio-disk0, 1279, 28505088, 640616474, 174, 2845696, 530699221,
0, 0)
+CHECK(virtio-disk1, 85, 348160, 8232156, 0, 0, 0, 0, 0)
+
[libvirt] [PATCH 03/12] qemu: blockstats: Switch to caller allocated hash table
Allocate the hash table in the monitor wrapper function instead of the
worker itself so that the text monitor impl that will be added in the
next patch doesn't have to duplicate it.
---
src/qemu/qemu_monitor.c | 13 -
src/qemu/qemu_monitor_json.c | 14 +-
src/qemu/qemu_monitor_json.h | 2 +-
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 24e87b7..95a6989 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1864,7 +1864,18 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
return -1;
}
-return qemuMonitorJSONGetAllBlockStatsInfo(mon, ret_stats, backingChain);
+if (!(*ret_stats = virHashCreate(10, virHashValueFree)))
+goto error;
+
+if (qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain) 0)
+goto error;
+
+return 0;
+
+ error:
+virHashFree(*ret_stats);
+*ret_stats = NULL;
+return -1;
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 612553b..c88c7c3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1695,7 +1695,10 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
if (flush_total_times)
*flush_total_times = -1;
-if (qemuMonitorJSONGetAllBlockStatsInfo(mon, blockstats, false) 0)
+if (!(blockstats = virHashCreate(10, virHashValueFree)))
+goto cleanup;
+
+if (qemuMonitorJSONGetAllBlockStatsInfo(mon, blockstats, false) 0)
goto cleanup;
if (!(stats = virHashLookup(blockstats, dev_name))) {
@@ -1870,7 +1873,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,
int
qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
-virHashTablePtr *ret_stats,
+virHashTablePtr hash,
bool backingChain)
{
int ret = -1;
@@ -1879,14 +1882,10 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
virJSONValuePtr devices;
-virHashTablePtr hash = NULL;
if (!(cmd = qemuMonitorJSONMakeCommand(query-blockstats, NULL)))
return -1;
-if (!(hash = virHashCreate(10, virHashValueFree)))
-goto cleanup;
-
if ((rc = qemuMonitorJSONCommand(mon, cmd, reply)) 0)
goto cleanup;
@@ -1924,12 +1923,9 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
}
-*ret_stats = hash;
-hash = NULL;
ret = 0;
cleanup:
-virHashFree(hash);
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 23589cf..0fcb0c0 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -82,7 +82,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
long long *flush_req,
long long *flush_total_times);
int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
-virHashTablePtr *ret_stats,
+virHashTablePtr hash,
bool backingChain);
int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon,
virHashTablePtr stats,
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] qemu: blockstats: Refactor qemuDomainBlockStatsFlags
Now that qemuDomainBlocksStatsGather provides functions of both
qemuMonitorGetBlockStatsParamsNumber and qemuMonitorGetBlockStatsInfo we
can reuse it and kill a lot of code.
Additionally as a bonus qemuDomainBlockStatsFlags will now support
summary statistics so add a statement to the virsh man page about that.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1142636
man
---
src/qemu/qemu_driver.c | 82 +++---
tools/virsh.pod| 5 +--
2 files changed, 21 insertions(+), 66 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4458c52..c35637b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10637,21 +10637,14 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
unsigned int flags)
{
virQEMUDriverPtr driver = dom-conn-privateData;
-int idx;
-int tmp, ret = -1;
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;
-char *diskAlias = NULL;
+qemuBlockStatsPtr blockstats = NULL;
+int nstats;
+int ret = -1;
-virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+VIR_DEBUG(params=%p, flags=%x, params, flags);
-if (!*path) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
- _(summary statistics are not supported yet));
-return ret;
-}
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
/* We don't return strings, and thus trivially support this flag. */
flags = ~VIR_TYPED_PARAM_STRING_OKAY;
@@ -10671,64 +10664,26 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
goto endjob;
}
-if (*nparams != 0) {
-virDomainDiskDefPtr disk = NULL;
-if ((idx = virDomainDiskIndexByName(vm-def, path, false)) 0) {
-virReportError(VIR_ERR_INVALID_ARG,
- _(invalid path: %s), path);
-goto endjob;
-}
-disk = vm-def-disks[idx];
-
-if (!disk-info.alias) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
-_(missing disk device alias name for %s),
-disk-dst);
- goto endjob;
-}
-if (VIR_STRDUP(diskAlias, disk-info.alias) 0)
-goto endjob;
-}
-
-priv = vm-privateData;
-VIR_DEBUG(priv=%p, params=%p, flags=%x, priv, params, flags);
-
-qemuDomainObjEnterMonitor(driver, vm);
-tmp = *nparams;
-ret = qemuMonitorGetBlockStatsParamsNumber(priv-mon, nparams);
-
-if (tmp == 0 || ret 0) {
-ignore_value(qemuDomainObjExitMonitor(driver, vm));
+if ((nstats = qemuDomainBlocksStatsGather(driver, vm, path,
+ blockstats)) 0)
goto endjob;
-}
-
-ret = qemuMonitorGetBlockStatsInfo(priv-mon,
- diskAlias,
- rd_req,
- rd_bytes,
- rd_total_times,
- wr_req,
- wr_bytes,
- wr_total_times,
- flush_req,
- flush_total_times);
-
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-ret = -1;
-if (ret 0)
+/* return count of supported stats */
+if (*nparams == 0) {
+*nparams = nstats;
+ret = 0;
goto endjob;
+}
-tmp = 0;
-ret = -1;
+nstats = 0;
/* the following macro shall not be used with side effects statements */
#define QEMU_BLOCK_STAT(VAR, NAME)\
-if (tmp *nparams (VAR) != -1) { \
-if (virTypedParameterAssign(params + tmp, NAME, VIR_TYPED_PARAM_LLONG,\
-(VAR)) 0) \
+if (nstats *nparams (blockstats-VAR) != -1) { \
+if (virTypedParameterAssign(params + nstats, NAME,\
+VIR_TYPED_PARAM_LLONG, (blockstats-VAR))
0) \
goto endjob; \
-tmp++;\
+nstats++; \
}
QEMU_BLOCK_STAT(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES);
@@ -10748,13 +10703,12 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
/* Field 'errs' is meaningless for QEMU, won't set it. */
ret = 0;
-*nparams = tmp;
+*nparams = nstats;
endjob:
qemuDomainObjEndJob(driver, vm);
cleanup:
-VIR_FREE(diskAlias);
qemuDomObjEndAPI(vm);
return ret;
}
diff --git
[libvirt] [PATCH 09/12] qemu: blockstats: Add support for totalled block statistics
In the LXC driver, if the disk path is not provided the API returns
total statistics for all disks of the domain. With the new text monitor
implementation this can be now done in the qemu driver too.
Add code that wil total the stats for all disks if the path is not
provided.
---
src/qemu/qemu_driver.c | 44 ++--
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e280cdf..4458c52 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10481,6 +10481,30 @@ qemuDomainBlockResize(virDomainPtr dom,
}
+static void
+qemuDomainBlockStatsGatherTotals(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+qemuBlockStatsPtr data = payload;
+qemuBlockStatsPtr total = opaque;
+
+#define QEMU_BLOCK_STAT_TOTAL(NAME)
\
+if (data-NAME 0)
\
+total-NAME += data-NAME
+
+QEMU_BLOCK_STAT_TOTAL(wr_bytes);
+QEMU_BLOCK_STAT_TOTAL(wr_req);
+QEMU_BLOCK_STAT_TOTAL(rd_bytes);
+QEMU_BLOCK_STAT_TOTAL(rd_req);
+QEMU_BLOCK_STAT_TOTAL(flush_req);
+QEMU_BLOCK_STAT_TOTAL(wr_total_times);
+QEMU_BLOCK_STAT_TOTAL(rd_total_times);
+QEMU_BLOCK_STAT_TOTAL(flush_total_times);
+#undef QEMU_BLOCK_STAT_TOTAL
+}
+
+
/**
* qemuDomainBlocksStatsGather:
* @driver: driver object
@@ -10523,10 +10547,6 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
if (VIR_STRDUP(diskAlias, disk-info.alias) 0)
goto cleanup;
-} else {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
- _(summary statistics are not supported yet));
-goto cleanup;
}
qemuDomainObjEnterMonitor(driver, vm);
@@ -10537,13 +10557,17 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
if (VIR_ALLOC(*retstats) 0)
goto cleanup;
-if (!(stats = virHashLookup(blockstats, diskAlias))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot find statistics for device '%s'), diskAlias);
-goto cleanup;
-}
+if (diskAlias) {
+if (!(stats = virHashLookup(blockstats, diskAlias))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(cannot find statistics for device '%s'),
diskAlias);
+goto cleanup;
+}
-**retstats = *stats;
+**retstats = *stats;
+} else {
+virHashForEach(blockstats, qemuDomainBlockStatsGatherTotals,
*retstats);
+}
ret = nstats;
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns
On Mon, Jul 14, 2014 at 12:01 PM, Chen Hanxiao
chenhanx...@cn.fujitsu.com wrote:
kernel commit 7dc5dbc879bd0779924b5132a48b731a0bc04a1e
forbid us doing a fresh mount for sysfs
when enable userns but disable netns.
This patch will create a bind mount in this senario.
Sorry for exhuming an already merged patch but today I ran into a
nasty issue caused by it.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_container.c | 44 +---
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d89677..8a27215 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -815,10 +815,13 @@ static int lxcContainerSetReadOnly(void)
}
-static int lxcContainerMountBasicFS(bool userns_enabled)
+static int lxcContainerMountBasicFS(bool userns_enabled,
+bool netns_disabled)
{
size_t i;
int rc = -1;
+char* mnt_src = NULL;
+int mnt_mflags;
VIR_DEBUG(Mounting basic filesystems);
@@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
bool bindOverReadonly;
virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
+/* When enable userns but disable netns, kernel will
+ * forbid us doing a new fresh mount for sysfs.
+ * So we had to do a bind mount for sysfs instead.
+ */
+if (userns_enabled netns_disabled
+STREQ(mnt-src, sysfs)) {
+if (VIR_STRDUP(mnt_src, /sys) 0) {
+goto cleanup;
+}
This is clearly broken and looks very untested to me.
It will issue this mount call:
mount(/sys, /sys, sysfs, MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_BIND, NULL)
because the code runs after pivot_root(2).
i.e, /sys will be still empty after that and no sysfs at all there.
As libvirt will later remount /sys readonly creating a container will
fail with the most useless error message:
Error: internal error: guest failed to start: Unable to create
directory /sys/fs/: Read-only file system
or
Error: internal error: guest failed to start: Unable to create
directory /sys/fs/cgroup: Read-only file system
Please note that changing /sys to /.oldroot/sys will not solve the
issue as this code runs already in the new
namespace and therefore the old mount tree is locked, thus MS_BIND is
not allowed.
This brings me to the question, why do you handle the netns_disabled
case anyway?
If in the XML file no network is specified just create a new and empty
network namespace.
Bindmounting /sys into the container is a security issue. This is why
mounting sysfs without a netns
was disabled to begin with.
P.S: Sorry for the grumpy mail, I've wasted almost the whole day with
debugging that issue.
--
Thanks,
//richard
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] parallels: prevent domain define only if NUMA is really specified
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 5a3969e..4ec9161 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1816,7 +1816,15 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom,
virDomainDefPtr def)
return -1;
}
-if (def-numa) {
+
+/*
+ * Though we don't support NUMA configuration at the moment
+ * virDomainDefPtr always contain non zero NUMA configuration
+ * So, just make sure this configuration does't differ from auto generated.
+ */
+if ((virDomainNumatuneGetMode(def-numa, -1) !=
+ VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
+ virDomainNumatuneHasPerNodeBinding(def-numa)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(numa parameters are not supported
by parallels driver));
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] parallels: add VIR_ARCH_I686 capability to parallels driver
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; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: don't fill in nicindexes for session mode libvirtd
Commit 4bbe1029f fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the niceindex array would be filled in for macvtap as well as tap
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by if
(cfg-privileged). The result was that the fixed libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:
Unable to open control socket: Operation not permitted
To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg-privileged.
This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244
---
If someone (Rich?) needs this pushed before I am awake, please feel
free to push it. (also push to the 1.2.13-maint branch if you do)
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1510797..3d1483e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7861,6 +7861,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
int actualType = virDomainNetGetActualType(net);
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7936,7 +7937,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* network and bridge use a tap device, and direct uses a
* macvtap device
*/
-if (nicindexes nnicindexes net-ifname) {
+if (cfg-privileged nicindexes nnicindexes net-ifname) {
if (virNetDevGetIndex(net-ifname, nicindex) 0 ||
VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) 0)
goto cleanup;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] parallels: set cpu mode when applying xml configuration
From: Mikhail Feoktistov mfeoktis...@parallels.com
Otherwise exporting existing domain config and defining a new one like this:
virsh -c parallels:///system dumpxml instance01 my.xml
virsh -c parallels:///system define my.xml
leads to an error because PCS default x64 mode turns to x32.
Thus, we need to set correct cpuMode in prlsdkDoApplyConfig() explicitly.
Signed-off-by: Mikhail Feoktistov mfeoktis...@parallels.com
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fec145d..9a2d658 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2869,6 +2869,19 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
pret = PrlVmCfg_SetCpuCount(sdkdom, def-vcpus);
prlsdkCheckRetGoto(pret, error);
+switch (def-os.arch) {
+case VIR_ARCH_X86_64:
+pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_64);
+break;
+case VIR_ARCH_I686:
+pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_32);
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown CPU mode: %X),
def-os.arch);
+goto error;
+}
+prlsdkCheckRetGoto(pret, error);
+
if (prlsdkClearDevices(sdkdom) 0)
goto error;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] parallels: fixes and cleanups
Maxim Nestratov (3): parallels: don't forget to unlock domain if unregister fails parallels: fix home directory for VMs parallels: minor cleanup Mikhail Feoktistov (1): parallels: set cpu mode when applying xml configuration -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] xenapi: Resolve Coverity NO_EFFECT
Coverity points out that check (def-uuid) has no effect since it's not
a pointer, rather an array of characters. Just remove the extranous check.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/xenapi/xenapi_utils.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index 91bc649..ce09dfe 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -455,11 +455,9 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr
def,
*record = xen_vm_record_alloc();
if (VIR_STRDUP((*record)-name_label, def-name) 0)
goto error;
-if (def-uuid) {
-virUUIDFormat(def-uuid, uuidStr);
-if (VIR_STRDUP((*record)-uuid, uuidStr) 0)
-goto error;
-}
+virUUIDFormat(def-uuid, uuidStr);
+if (VIR_STRDUP((*record)-uuid, uuidStr) 0)
+goto error;
if (STREQ(def-os.type, hvm)) {
char *boot_order = NULL;
if (VIR_STRDUP((*record)-hvm_boot_policy, BIOS order) 0)
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] xenapi: Resolve Coverity REVERSE_INULL
Coverity notes in xenapiDomainGetXMLDesc that 'vms' is dereferenced a few times before a if (vms) xen_vm_set_free(vms); call is made. Since we'd exit out much sooner if the fetch of the vms failed, just remove the unnecessary if (vms) check. Signed-off-by: John Ferlan jfer...@redhat.com --- src/xenapi/xenapi_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 148ff9b..8eb8d73 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1563,8 +1563,7 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) } xen_vif_set_free(vif_set); } -if (vms) -xen_vm_set_free(vms); +xen_vm_set_free(vms); xml = virDomainDefFormat(defPtr, flags); virDomainDefFree(defPtr); return xml; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Coverity fixes
Some Coverity patches - the first one has been showing up in my dailies for a few days... The other 5 I don't see, but that's because I don't generally build with xenapi in my Coverity environment. So for those, I'm hoping someone with that environment could check them out. John Ferlan (6): conf: Resolve Coverity RESOURCE_LEAK xenapi: Resolve Coverity FORWARD_NULL xenapi: Resolve Coverity NO_EFFECT xenapi: Resolve Coverity NULL_RETURNS xenapi: Resolve Coverity REVERSE_INULL xenapi: Resolve Coverity REVERSE_INULL src/conf/node_device_conf.c | 1 + src/xenapi/xenapi_driver.c | 13 - src/xenapi/xenapi_utils.c | 33 ++--- 3 files changed, 27 insertions(+), 20 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] xenapi: Resolve Coverity NULL_RETURNS
Coverity points out that the return from virDomainDefParseString is
not checked in xenapiDomainCreateXML like it should be which could
end up in a pointer dereference
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/xenapi/xenapi_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 821e9d9..148ff9b 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -560,6 +560,8 @@ xenapiDomainCreateXML(virConnectPtr conn,
priv-caps, priv-xmlopt,
1 VIR_DOMAIN_VIRT_XEN,
parse_flags);
+if (!defPtr)
+return NULL;
createVMRecordFromXml(conn, defPtr, record, vm);
virDomainDefFree(defPtr);
if (record) {
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] xenapi: Resolve Coverity REVERSE_INULL
Coverity complains that net_set is compared to NULL before calling
xen_network_set_free, but used rather liberally before that. While
I was looking at the code I also noted that if the virAsprintfQuiet
fails, then we leak our structures - so I added those too.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/xenapi/xenapi_utils.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index ce09dfe..0c13745 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -399,14 +399,14 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int
device,
xen_network_set *net_set = NULL;
xen_network_record *net_rec = NULL;
int cnt = 0;
-if (xen_network_get_all(session, net_set)) {
-for (cnt = 0; cnt net_set-size; cnt++) {
-if (xen_network_get_record(session, net_rec,
net_set-contents[cnt])) {
-if (STREQ(net_rec-bridge, bridge)) {
-break;
-} else {
-xen_network_record_free(net_rec);
-}
+if (!xen_network_get_all(session, net_set))
+return -1
+for (cnt = 0; cnt net_set-size; cnt++) {
+if (xen_network_get_record(session, net_rec, net_set-contents[cnt]))
{
+if (STREQ(net_rec-bridge, bridge)) {
+break;
+} else {
+xen_network_record_free(net_rec);
}
}
}
@@ -425,8 +425,13 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
vif_record-other_config = xen_string_string_map_alloc(0);
vif_record-runtime_properties = xen_string_string_map_alloc(0);
vif_record-qos_algorithm_params = xen_string_string_map_alloc(0);
-if (virAsprintfQuiet(vif_record-device, %d, device) 0)
+if (virAsprintfQuiet(vif_record-device, %d, device) 0) {
+xen_vif_free(vif);
+xen_vif_record_free(vif_record);
+xen_network_record_free(net_rec);
+xen_network_set_free(net_set);
return -1;
+}
xen_vif_create(session, vif, vif_record);
if (!vif) {
xen_vif_free(vif);
@@ -438,7 +443,7 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
xen_vif_record_free(vif_record);
xen_network_record_free(net_rec);
}
-if (net_set != NULL) xen_network_set_free(net_set);
+xen_network_set_free(net_set);
return -1;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] conf: Resolve Coverity RESOURCE_LEAK
Commit id 'c9027d8f' added parsing of the CapNet for offload SRIOV NIC discovery, but forgot to free the nodes Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f9c9b6f..d309bc6 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1002,6 +1002,7 @@ virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt, ret = 0; out: ctxt-node = orignode; +VIR_FREE(nodes); return ret; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] xenapi: Resolve Coverity FORWARD_NULL
Since inception. Coverity complains that the code checks (record ==
NULL !session-ok), but doesn't check (record != NULL) before
dereferencing at record-is_a_template
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/xenapi/xenapi_driver.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index afb6d6c..821e9d9 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1646,9 +1646,11 @@ xenapiConnectNumOfDefinedDomains(virConnectPtr conn)
xen_vm_set_free(result);
return -1;
}
-if (record-is_a_template == 0)
-DomNum++;
-xen_vm_record_free(record);
+if (record) {
+if (record-is_a_template == 0)
+DomNum++;
+xen_vm_record_free(record);
+}
}
xen_vm_set_free(result);
return DomNum;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] qemu: Remove unnecessary virReportError on networkGetNetworkAddress return
On 03/10/2015 10:58 AM, Ján Tomko wrote: On Mon, Mar 09, 2015 at 08:05:01PM -0400, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com Error messages are already set in all code paths returning -1 from networkGetNetworkAddress, so we don't want to overwrite them. Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_command.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) ACK, this one does not depend on the rest of the series. Jan Tks - this one patch is pushed now. I believe this will cause a change to the message in: https://bugzilla.redhat.com/show_bug.cgi?id=1192318 from error: XML error: listen network 'ipv6' had no usable address to: ...Unable to get IPv4 address on this platform But I have no empirical evidence. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] parallels: minor cleanup
indentation is fixed, unnecessary error message removed,
unnecessary job freeing removed
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index e0c5895..0c9837a 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -702,7 +702,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
goto cleanup;
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen);
-prlsdkCheckRetGoto(pret, cleanup);
+prlsdkCheckRetGoto(pret, cleanup);
if (isCt netAdapterIndex == (PRL_UINT32) -1) {
/* venet devices don't have mac address and
@@ -1352,7 +1352,6 @@ prlsdkLoadDomains(parallelsConnPtr privconn)
error:
PrlHandle_Free(result);
-PrlHandle_Free(job);
return -1;
}
@@ -1732,8 +1731,6 @@ prlsdkDomainChangeState(virDomainPtr domain,
pdom = dom-privateData;
pret = chstate(privconn, pdom-sdkdom);
-virReportError(VIR_ERR_OPERATION_FAILED,
- _(Can't change domain state: %d), pret);
if (PRL_FAILED(pret)) {
virResetLastError();
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] parallels: fix home directory for VMs
Failures of parallelsStorageOpen occured because we incorrectly treated
path to VM' configuration file as a directory. Now initialization of
parallels VM domains home directory is fixed.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 9a2d658..e0c5895 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1247,6 +1247,14 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
pret = PrlVmCfg_GetHomePath(sdkdom, pdom-home, buflen);
prlsdkCheckRetGoto(pret, error);
+/* For VMs pdom-home is actually /directory/config.pvs */
+if (!IS_CT(def)) {
+/* Get rid of /config.pvs in path string */
+char *s = strrchr(pdom-home, '/');
+if (s)
+*s = '\0';
+}
+
if (olddom) {
/* assign new virDomainDef without any checks */
/* we can't use virDomainObjAssignDef, because it checks
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] parallels: bridge network support enhancement and other network fixes
Maxim Nestratov (7): parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set network adapter device status to connected parallels: make E1000 network adapter type default parallels: switch off offline management feature parallels: don't prevent domain start if VIR_DOMAIN_NET_TYPE_BRIDGE -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] parallels: better bridge network interface support
In order to support 'bridge' network adapters in parallels
driver we need to plug our veth devices into corresponding
linux bridges.
We are going to do this by reusing our abstraction of
Virtual Networks in terms of PCS. On a domain creation, we
create a new Virtual Network naming it with the same name
as a source bridge for each network interface.
Having done this, we plug PCS veth interfaces created with names of
target dev into specified bridges using our standard PCS procedures
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 101 +++--
1 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index c31439f..71a64a1 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -692,9 +692,6 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
/* use device name, shown by prlctl as target device
* for identifying network adapter in virDomainDefineXML */
-pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex);
-prlsdkCheckRetGoto(pret, cleanup);
-
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, buflen);
prlsdkCheckRetGoto(pret, cleanup);
@@ -704,6 +701,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen);
prlsdkCheckRetGoto(pret, cleanup);
+pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex);
+prlsdkCheckRetGoto(pret, cleanup);
+
if (isCt netAdapterIndex == (PRL_UINT32) -1) {
/* venet devices don't have mac address and
* always up */
@@ -741,6 +741,16 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
net-data.network.name,
buflen);
prlsdkCheckRetGoto(pret, cleanup);
+
+/*
+ * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
+ * except those whose Virtual Network Id differ from Parallels
+ * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
+ * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
+ */
+if (!STREQ(net-data.network.name,
PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME))
+net-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
}
pret = PrlVmDev_IsConnected(netAdapter, isConnected);
@@ -2616,10 +2626,12 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac,
char *macstr)
return macstr;
}
-static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net)
+static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn,
virDomainNetDefPtr net)
{
PRL_RESULT pret;
PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
+PRL_HANDLE vnet = PRL_INVALID_HANDLE;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
int ret = -1;
char macstr[PRL_MAC_STRING_BUFNAME];
@@ -2644,10 +2656,39 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
virDomainNetDefPtr net)
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
-pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
-if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
+if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
+prlsdkCheckRetGoto(pret, cleanup);
+} else if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetVirtualNetworkId(sdknet,
net-data.network.name);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+} else if (net-type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+/*
+ * For this type of adapter we create a new
+ * Virtual Network assuming that bridge with given name exists
+ * Failing creating this means domain creation failure
+ */
+pret = PrlVirtNet_Create(vnet);
prlsdkCheckRetGoto(pret, cleanup);
-} else {
+
+pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
+job = PrlSrv_AddVirtualNetwork(privconn-server, vnet, 0);
+if (PRL_FAILED(pret = waitJob(job, privconn-jobTimeout)))
+goto cleanup;
+
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+prlsdkCheckRetGoto(pret, cleanup);
+
pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name);
prlsdkCheckRetGoto(pret, cleanup);
}
@@ -2660,10
[libvirt] [PATCH 2/7] parallels: fix parallelsLoadNetworks
Don't fail initialization of parallels driver if
parallelsLoadNetwork fails for optional networks.
This can happen when some of them are added manually
and configured incompletely. PCS requires only two networks
created automatically (named Host-Only and Bridged), others
are optional and their incompletenes can be ignored.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_network.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 26767c0..24fdb70 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr
def, const char *name)
return ret;
}
-static virNetworkObjPtr
+static int
parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
{
+int ret = -1;
virNetworkObjPtr net;
virNetworkDefPtr def;
const char *tmp;
@@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_BRIDGE;
-if (parallelsGetBridgedNetInfo(def, jobj) 0)
+if (parallelsGetBridgedNetInfo(def, jobj) 0) {
+
+/* Only mandatory networks required to be configured completely */
+if (!STREQ(def-name, PARALLELS_REQUIRED_BRIDGED_NETWORK))
+ret = 0;
+
goto cleanup;
+}
} else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_NONE;
-if (parallelsGetHostOnlyNetInfo(def, def-name) 0)
+if (parallelsGetHostOnlyNetInfo(def, def-name) 0) {
+
+/* Only mandatory networks required to be configured completely */
+if (!STREQ(def-name, PARALLELS_REQUIRED_HOSTONLY_NETWORK))
+ret = 0;
+
goto cleanup;
+}
} else {
parallelsParseError();
goto cleanup;
@@ -231,11 +244,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
net-active = 1;
net-autostart = 1;
virNetworkObjUnlock(net);
-return net;
+ret = 0;
+return ret;
cleanup:
virNetworkDefFree(def);
-return NULL;
+return ret;
}
static virNetworkObjPtr
@@ -277,7 +291,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
-virNetworkObjPtr net;
int ret = -1;
int count;
size_t i;
@@ -302,8 +315,8 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
goto cleanup;
}
-net = parallelsLoadNetwork(privconn, jobj2);
-if (!net)
+ret = parallelsLoadNetwork(privconn, jobj2);
+if (!ret)
goto cleanup;
}
--
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 if unregister fails
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_driver.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 650b790..623b122 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -943,6 +943,7 @@ parallelsDomainUndefineFlags(virDomainPtr domain,
{
parallelsConnPtr privconn = domain-conn-privateData;
virDomainObjPtr dom = NULL;
+int ret;
virCheckFlags(0, -1);
@@ -952,7 +953,11 @@ parallelsDomainUndefineFlags(virDomainPtr domain,
return -1;
}
-return prlsdkUnregisterDomain(privconn, dom);
+ret = prlsdkUnregisterDomain(privconn, dom);
+if (ret)
+ virObjectUnlock(dom);
+
+return ret;
}
static int
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] parallels: make E1000 network adapter type default
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index f0777bd..e17a941 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2656,6 +2656,10 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
parallelsConnPtr privconn, virDomainN
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
+/* Other alternatives: PNT_VIRTIO, PNT_RTL */
+pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000);
+prlsdkCheckRetGoto(pret, cleanup);
+
if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (STREQ(net-data.network.name,
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] parallels: switch off offline management feature
which is on by default when a new VM/CT is created. We should do this because this feature can't be controlled by libvirt now and it sets up some iptables rules. So it's better to do this to avoid potential conflict of different set of rules or to avoid unexpected behavior. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index e17a941..fb65ff9 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3043,6 +3043,9 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, PVS_GUEST_VER_LIN_REDHAT, 0); prlsdkCheckRetGoto(pret, cleanup); +pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); +prlsdkCheckRetGoto(pret, cleanup); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] parallels: set network adapter device status to connected
when a new network adapter device is added
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 71a64a1..f0777bd 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2644,7 +2644,7 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
parallelsConnPtr privconn, virDomainN
pret = PrlVmDev_SetEnabled(sdknet, 1);
prlsdkCheckRetGoto(pret, cleanup);
-pret = PrlVmDev_SetConnected(sdknet, net-linkstate);
+pret = PrlVmDev_SetConnected(sdknet, 1);
prlsdkCheckRetGoto(pret, cleanup);
if (net-ifname) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] parallels: don't prevent domain start if VIR_DOMAIN_NET_TYPE_BRIDGE
network adapter is used
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fb65ff9..c94e70a 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2237,7 +2237,8 @@ static int
prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr)
static int prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net)
{
-if (net-type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+if (net-type != VIR_DOMAIN_NET_TYPE_NETWORK
+net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Specified network adapter type is not
supported by Parallels Cloud Server.));
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_network.c |6 +++---
src/parallels/parallels_sdk.c |6 +++---
src/parallels/parallels_utils.h |8 +++-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 62ed268..26767c0 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
goto cleanup;
}
-if (STREQ(tmp, bridged)) {
+if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_BRIDGE;
if (parallelsGetBridgedNetInfo(def, jobj) 0)
goto cleanup;
-} else if (STREQ(tmp, host-only)) {
+} else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
def-forward.type = VIR_NETWORK_FORWARD_NONE;
if (parallelsGetHostOnlyNetInfo(def, def-name) 0)
@@ -249,7 +249,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
def-forward.type = VIR_NETWORK_FORWARD_ROUTE;
-if (VIR_STRDUP(def-name, PARALLELS_ROUTED_NETWORK_NAME) 0)
+if (VIR_STRDUP(def-name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0)
goto cleanup;
if (virUUIDParse(PARALLELS_ROUTED_NETWORK_UUID, def-uuid) 0) {
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fec145d..c31439f 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -709,7 +709,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
* always up */
net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
if (VIR_STRDUP(net-data.network.name,
- PARALLELS_ROUTED_NETWORK_NAME) 0)
+ PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0)
goto cleanup;
return 0;
}
@@ -728,7 +728,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr
net, bool isCt)
if (emulatedType == PNA_ROUTED) {
if (VIR_STRDUP(net-data.network.name,
- PARALLELS_ROUTED_NETWORK_NAME) 0)
+ PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0)
goto cleanup;
} else {
pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter, NULL, buflen);
@@ -2644,8 +2644,8 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
virDomainNetDefPtr net)
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
-if (STREQ(net-data.network.name, PARALLELS_ROUTED_NETWORK_NAME)) {
pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
+if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
prlsdkCheckRetGoto(pret, cleanup);
} else {
pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name);
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index ead5586..a1c7c60 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -47,7 +47,13 @@
_(no domain with matching uuid '%s'), uuidstr); \
} while (0)
-# define PARALLELS_ROUTED_NETWORK_NAME Routed
+# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME Routed
+# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME Bridged
+
+# define PARALLELS_REQUIRED_HOSTONLY_NETWORK Host-Only
+# define PARALLELS_HOSTONLY_NETWORK_TYPE host-only
+# define PARALLELS_REQUIRED_BRIDGED_NETWORK Bridged
+# define PARALLELS_BRIDGED_NETWORK_TYPE bridged
struct _parallelsConn {
virMutex lock;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't fill in nicindexes for session mode libvirtd
On Tue, Mar 10, 2015 at 02:32:04AM -0400, Laine Stump wrote:
Commit 4bbe1029f fixed a problem in commit f7afeddc by moving the call
to virNetDevGetIndex() to a location common to all interface types (so
that the niceindex array would be filled in for macvtap as well as tap
interfaces), but the location was *too* common, as the original call
to virNetDevGetIndex() had been in a section qualified by if
(cfg-privileged). The result was that the fixed libvirtd would try
to call virNetDevGetIndex() even for session mode libvirtd, and end up
failing with the log message:
Unable to open control socket: Operation not permitted
To remedy that, this patch qualifies the call to virNetDevGetIndex()
in its new location with cfg-privileged.
This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1198244
---
If someone (Rich?) needs this pushed before I am awake, please feel
free to push it. (also push to the 1.2.13-maint branch if you do)
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1510797..3d1483e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7861,6 +7861,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
int actualType = virDomainNetGetActualType(net);
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7936,7 +7937,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
/* network and bridge use a tap device, and direct uses a
* macvtap device
*/
-if (nicindexes nnicindexes net-ifname) {
+if (cfg-privileged nicindexes nnicindexes net-ifname) {
if (virNetDevGetIndex(net-ifname, nicindex) 0 ||
VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) 0)
goto cleanup;
Looks sensible, ACK.
As Laine says, please make sure this gets into 1.2.13-maint because it
currently affects all 1.2.13 / Rawhide users.
Also, why isn't there a regression test that would have picked this
up? A trivial reproducer is:
$ guestfish -a /dev/null --network run
but any test case that launches a guest with a network interface as
non-root would have caught this.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 00/10] Network enhancements and other fixes
03.03.2015 23:32, Maxim Nestratov пишет: Maxim Nestratov (9): parallels: don't forget to unlock domain if unregister fails parallels: fix home directory for VMs parallels: minor cleanup parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set network adapter device status to connected parallels: make E1000 network adapter type default parallels: switch off offline management feature Mikhail Feoktistov (1): parallels: set cpu mode when applying xml configuration Please disgregard this series. I've split it and resent in different ones: [PATCH 0/4] parallels: fixes and cleanups and [PATCH 0/7] parallels: bridge network support enhancement and other network fixes -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
