Re: [libvirt] [PATCH 00/13] Network disk improvements (NBD libiscsi)
Il 28/02/2013 06:10, Eric Blake ha scritto: It's too late for me tonight to do any more reviewing, but I also hope to check tomorrow whether any of 2-6 also make sense during the freeze. Patches 7-13 should wait until after 1.0.3 is out, since they are definitely new material and not bug fixes. No, I don't think any of 2-13 belong in 1.0.3. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] qemu: fix use-after-free when parsing NBD disk
Il 28/02/2013 06:03, Eric Blake ha scritto: diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index 0a7d4ec..f8f3ade 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -8832,11 +8832,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, if (VIR_ALLOC(disk-hosts) 0) goto no_memory; disk-nhosts = 1; -disk-hosts-name = host; +disk-hosts-name = disk-src; +disk-src = NULL; disk-hosts-port = strdup(port); if (!disk-hosts-port) goto no_memory; -disk-src = NULL; break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: /* old-style CEPH_ARGS env variable is parsed later */ ACK Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4]typo fix and codes improvement in generator.py
This four patches try to fix various typoes, fd leaks and optimize codes in generator.py script. This is the first round. Guannan Ren(4) [PATCH 1/4] python: global variable and debugging improvement for [PATCH 2/4] python: fix typoes and repeated global vars references [PATCH 3/4] python: optimize SAX xml parsing event handler [PATCH 4/4] python: fix fd leak in generator.py python/generator.py | 158 ++ 1 file changed, 70 insertions(+), 88 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] python: optimize SAX xml parsing event handler
close(), getmethodname(), cdata() are not standard methods from
parent class ContentHandler and not being used anywhere in codes,
so remove them.
In docParser, actually, we are overloading three parent methods
startElement(), endElement() and characters(), so I rename back
three of these method names for example from start()
to startElement() which makes it simpler to read.
make other improvements in codes.
---
python/generator.py | 53 -
1 file changed, 16 insertions(+), 37 deletions(-)
diff --git a/python/generator.py b/python/generator.py
index 39e654b..7ccd471 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -30,41 +30,24 @@ def getparser(debug):
target = docParser(debug)
parser = make_parser()
parser.setContentHandler(target)
-return parser, target
+return parser
class docParser(ContentHandler):
def __init__(self, debug = False):
self.debug = debug;
-self._methodname = None
-self._data = []
+self.data = []
self.in_function = 0
-self.startElement = self.start
-self.endElement = self.end
-self.characters = self.data
-
-def close(self):
-if self.debug:
-print close
-
-def getmethodname(self):
-return self._methodname
-
-def data(self, text):
-if self.debug:
-print data %s % text
-self._data.append(text)
-
-def cdata(self, text):
+def characters(self, text):
if self.debug:
print data %s % text
-self._data.append(text)
+self.data.append(text)
-def start(self, tag, attrs):
+def startElement(self, tag, attrs):
if self.debug:
print start %s, %s % (tag, attrs)
if tag == 'function':
-self._data = []
+self.data = []
self.in_function = 1
self.function = None
self.function_cond = None
@@ -80,9 +63,9 @@ class docParser(ContentHandler):
if attrs.has_key('module'):
self.function_module= attrs['module']
elif tag == 'cond':
-self._data = []
+self.data = []
elif tag == 'info':
-self._data = []
+self.data = []
elif tag == 'arg':
if self.in_function == 1:
self.function_arg_name = None
@@ -117,7 +100,7 @@ class docParser(ContentHandler):
elif attrs['file'] == libvirt-qemu:
qemu_enum(attrs['type'],attrs['name'],attrs['value'])
-def end(self, tag):
+def endElement(self, tag):
if self.debug:
print end %s % tag
if tag == 'function':
@@ -167,17 +150,13 @@ class docParser(ContentHandler):
self.function_return_info,
self.function_return_field]
elif tag == 'info':
-str = ''
-for c in self._data:
-str = str + c
if self.in_function == 1:
-self.function_descr = str
+text = ''.join(self.data)
+self.function_descr = text
elif tag == 'cond':
-str = ''
-for c in self._data:
-str = str + c
if self.in_function == 1:
-self.function_cond = str
+text = ''.join(self.data)
+self.function_cond = text
def function(name, desc, ret, args, file, module, cond):
@@ -792,14 +771,14 @@ def buildStubs(module, stubs_buiding_debug = False,
xml_parsing_debug = False):
try:
f = open(os.path.join(srcPref,api_xml))
data = f.read()
-(parser, target) = getparser(xml_parsing_debug)
+parser = getparser(xml_parsing_debug)
parser.feed(data)
parser.close()
except IOError, msg:
try:
f = open(os.path.join(srcPref,..,docs,api_xml))
data = f.read()
-(parser, target) = getparser(xml_parsing_debug)
+parser = getparser(xml_parsing_debug)
parser.feed(data)
parser.close()
except IOError, msg:
@@ -816,7 +795,7 @@ def buildStubs(module, stubs_buiding_debug = False,
xml_parsing_debug = False):
try:
f = open(os.path.join(srcPref, override_api_xml))
data = f.read()
-(parser, target) = getparser(xml_parsing_debug)
+parser = getparser(xml_parsing_debug)
parser.feed(data)
parser.close()
except IOError, msg:
--
1.7.11.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] python: fix typoes and repeated global vars references
--- python/generator.py | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/python/generator.py b/python/generator.py index 246767c..39e654b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -781,11 +781,11 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): elif module == libvirt-lxc: funcs = lxc_functions funcs_failed = lxc_functions_failed -funcs_skipped = functions_skipped +funcs_skipped = lxc_functions_skipped elif module == libvirt-qemu: funcs = qemu_functions funcs_failed = qemu_functions_failed -funcs_skipped = functions_skipped +funcs_skipped = qemu_functions_skipped api_xml = %s-api.xml % module @@ -1205,12 +1205,9 @@ def buildWrappers(module): global function_classes global classes_type global classes_list -global converter_type global primary_classes -global converter_type global classes_ancestor global converter_type -global primary_classes global classes_destructors global functions_noexcept -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] python: fix fd leak in generator.py
--- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/generator.py b/python/generator.py index 7ccd471..4b62f83 100755 --- a/python/generator.py +++ b/python/generator.py @@ -771,6 +771,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref,api_xml)) data = f.read() +f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() @@ -778,6 +779,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref,..,docs,api_xml)) data = f.read() +f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() @@ -795,6 +797,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref, override_api_xml)) data = f.read() +f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release candidate 2 of 1.0.3 is available
On 02/27/13 15:26, Michal Privoznik wrote: On 27.02.2013 12:20, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are at the usual place too: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests. Please give it more testing and let's keep change to git to bug fixes to minimize risks for 1.0.3. If everything goes well I will push 1.0.3 on Friday, thanks ! Daniel Maybe we want to postpone the release a bit; I was trying to find out how drop of qemu driver lock speeds up parallel startup of multiple domains, however - it is not working. I've created small test program - test4.c and run it against current HEAD. It takes arguments - domain names - which it spawns a separate thread per each domain and repeatedly starts and destroys domains. See code for more. Then, I've created 20 copies of diskless domain (attached as well): test1..test20 and run my test4 program over them: ./test4 $(for ((i=1; i21; i++)); do echo -n test$i ; done) I had to tweak max_client_requests=20 to really run those 20 requests in parallel. However, what I am getting instead of nice measuring of speedup is: [9722] Starting test1 (round 20) ...[9725] Starting test4 (round 20) ...[9724] Starting test3 (round 20) ...[9723] Starting test2 (round 20) ...[9726] Starting test5 (round 20) ...[9727] Starting test6 (round 20) ...[9728] Starting test7 (round 20) ...[9729] Starting test8 (round 20) ...[9730] Starting test9 (round 20) ...[9731] Starting test10 (round 20) ...[9732] Starting test11 (round 20) ...[9733] Starting test12 (round 20) ...[9734] Starting test13 (round 20) ...[9735] Starting test14 (round 20) ...[9736] Starting test15 (round 20) ...[9737] Starting test16 (round 20) ...[9738] Starting test17 (round 20) ...[9741] Starting test20 (round 20) ...[9739] Starting test18 (round 20) ...[9740] Starting test19 (round 20) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test12libvir: error : An error occurred, but the cause is unknown Unable to start domain test14libvir: error : An error occurred, but the cause is unknown Unable to start domain test7libvir: error : An error occurred, but the cause is unknown Unable to start domain test1libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test12[9733] Destroying test12 ...[9728] Destroying test7 ...[9735] Destroying test14 ...[9733] Starting test12 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test7[9728] Starting test7 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test14[9735] Starting test14 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test18libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test1[9722] Destroying test1 ...[9722] Starting test1 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test8libvir: error : An error occurred, but the cause is unknown Unable to start domain test15[9725] Done I'm hitting this issue when I start the guest with the network interface in the original XML. If I delete it or change it to a regular (non-vepa) then everything works well. So the impact of this problem isn't that critical as originally reported. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] rename qemuGetNumadAdvice to virDomainGetNumadAdvice
On 2013/02/28 01:16, Daniel P. Berrange wrote: On Wed, Feb 27, 2013 at 05:11:06PM +, Daniel P. Berrange wrote: On Wed, Feb 27, 2013 at 04:09:35PM +0800, Gao feng wrote: qemuGetNumadAdvice will be used by LXC driver,rename it to virDomainGetNumaAdvice and move it to domain_conf.c Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- src/conf/domain_conf.c | 31 +++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 32 +--- 4 files changed, 35 insertions(+), 31 deletions(-) Ewww no. The domain_conf.c file is for XML configuration parsing formatting. Code dealing with numad has no business being there. We don't currently have any place for general NUMA related helper APIs, so I'd suggest you create a new pair of files for these methods src/util/virnuma.h and src/util/virnuma.c NB, you won't be able to pass in a virDomainDef when the code is located here. Instead make the API accept 2 params, the vcpus and memory values. Get it :) Thanks for your comments. will fix these problems in next version. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] a script for libvirt-llxc + systemd on Fedora18
On 2013/02/27 17:33, Kamezawa Hiroyuki wrote: Hi. At playing libvirt-lxc on Fedora18, I found that the user needs some workarounds to run /sbin/init as the root process of container. With Fedora15, I found this https://gist.github.com/peo3/1142202. And I know virt-sandbox-service have some clever way to understand RPM to make domain running with a service. Here is a script I wrote for F18 inspired by above two. Creating LXC domain running systemd as /init program. I'd like to get some feedbackand ...can't this kind of scripts be maintained by some virt-tools project ? Anyway, playing with lxc is fun. I'm glad if someone makes this script better :) == Thanks! I think this work that creating root filesystem/directories and xml configure files for container should be done by virt-install. Daniel,what's your idea? Should we implement the same feature in virt-install? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] Remove contiguous CPU indexes assumption
On 2013年02月28日 14:27, Li Zhang wrote:
On 2013年02月28日 07:13, Eric Blake wrote:
On 02/27/2013 05:13 AM, Li Zhang wrote:
Hi Eric,
This should belong to bug-fix, could it be pushed to 1.0.3?
Without any test case under 'make check' that exposes the failure, I'm a
bit worried that this might cause regressions on other setups. I'm not
seven sure I even understand the scope of the problem. Is it something
specific to running a qemu-system-ppc64 emulator (but can be reproduced
on any host architecture), or is it specific to running on a powerpc
host (but happens for any version of qemu-* target architecture), or is
it something else altogether?
So sorry to see the failure.
I tried to 'make check' with my patch on ppc64, all of these cases pass.
For x86, I tried it with on x86 server, and pulled the latest version
of libvirt.
Even without my patch, it also fails during execute ./virshtest and
the tests with cpuset.
+++ out2013-02-28 13:02:14.290794080 +0800
@@ -1,2 +1,3 @@
error: vcpupin: Invalid or missing vCPU number.
--- exp2013-02-28 13:02:14.321794080 +0800
+++ out2013-02-28 13:02:14.320794080 +0800
@@ -1,2 +1,3 @@
error: vcpupin: Invalid vCPU number.
FAIL: vcpupin
The above error is caused by the environment of my X86.
I haven't used it for a long time.
Now, I checked the latest version with my patch, there is no error.
All test cases pass. Would you help double check it?
Thanks. :)
I think this problem is specific ppc64.
I did some experiment on X86.
X86 machine: 4 CPUs (0~3)
I disable CPU1 and CPU2 by writing 0 to /sys/.../cpuX/online.
Only 0 and 3 are online.
Although only 2 cpus are online, but after executing query-cpus, it
can get all the information of them.
[{return: [{current: true, CPU: 0, pc: 4294967280, halted:
false, thread_id: 31831}, {current: false, CPU: 1, pc:
4294967280, halted: false, thread_id: 31832}, {current: false,
CPU: 2, pc: 4294967280, halted: false, thread_id: 31833},
{current: false, CPU: 3, pc: 4294967280, halted: false,
thread_id: 31834}], id: libvirt-3}]
Qemu can expose all of these CPUs offline for X86.
But for ppc64, it can't expose these offline CPUs. The following is
the return value from ppc64.
[{current: true, CPU: 0, nip: 256, halted: false, thread_id:
25964}, {current: false, CPU: 4, nip: 256, halted: true,
thread_id: 25965}, {current: false, CPU: 8, nip: 256,
halted: true, thread_id: 25966}, {current: false, CPU: 12,
nip: 256, halted: true, thread_id: 25967}], id: libvirt-3}
We have test framework in place to allow replaying of a QMP JSON
response and seeing how qemu_monitor_json.c deals with it; what I'd
really like to see is a side-by-side comparison of what the QMP
'query-cpus' command returns for a guest with multiple vcpus on a setup
where you are seeing the problem, when compared to a setup that does not
have the issue. You can get at this with virsh qemu-monitor-command
$dom '{execute:query-cpus}', if that helps.
Thanks a lot. Tried with ppc64, it is different from x86 even with
some CPUs disabled.
To help you out, here's what I got for a guest using 3 vcpus on my
x86_64 host machine and using the qemu-kvm binary:
# virsh qemu-monitor-command guest '{execute:query-cpus}'
{return:[{current:true,CPU:0,pc:1025829,halted:false,thread_id:5849},{current:false,CPU:1,pc:1005735,halted:true,thread_id:5850},{current:false,CPU:2,pc:1005735,halted:true,thread_id:5851}],id:libvirt-9}
That is, your patch might be right, but I have not yet been convinced
that it is right; and while things may currently be broken on ppc, it is
not a recent breakage, so being conservative and either proving the fix
is right (via testsuite addition) or deferring the fix until post 1.0.3
seems like safer alternatives. Or maybe someone else will chime in and
agree to take it now, without waiting for my desired burden of proof.
OK, I will tried to see the testsuite problems for x86 even without my
patch.
I couldn't think of what kind of problems will this patch cause now.
Really thank you, Eric for your review and suggestion.
I don't know whether others have suggestions about my patch.
Any suggestion is appreciated.
Thanks.
--Li
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: virConnectGetVersion returns bogus value
The unitialized local variable qemuVersion can cause an random value
to be returned for the hypervisor version, observable with virsh version.
Introduced by commit b46f7f4a0b96c2d2d01d64d960bd7bc90dc16b0c
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
src/qemu/qemu_driver.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f6a431..825babd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1382,7 +1382,7 @@ cleanup:
static int qemuGetVersion(virConnectPtr conn, unsigned long *version) {
virQEMUDriverPtr driver = conn-privateData;
int ret = -1;
-unsigned int qemuVersion;
+unsigned int qemuVersion = 0;
virCapsPtr caps = NULL;
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: virConnectGetVersion returns bogus value
On 02/28/13 11:11, Viktor Mihajlovski wrote: The unitialized local variable qemuVersion can cause an random value to be returned for the hypervisor version, observable with virsh version. Introduced by commit b46f7f4a0b96c2d2d01d64d960bd7bc90dc16b0c Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK and will be pushed soon. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release candidate 2 of 1.0.3 is available
On Thu, Feb 28, 2013 at 11:00:28AM +0100, Peter Krempa wrote: On 02/27/13 15:26, Michal Privoznik wrote: On 27.02.2013 12:20, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are at the usual place too: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests. Please give it more testing and let's keep change to git to bug fixes to minimize risks for 1.0.3. If everything goes well I will push 1.0.3 on Friday, thanks ! Daniel Maybe we want to postpone the release a bit; I was trying to find out how drop of qemu driver lock speeds up parallel startup of multiple domains, however - it is not working. I've created small test program - test4.c and run it against current HEAD. It takes arguments - domain names - which it spawns a separate thread per each domain and repeatedly starts and destroys domains. See code for more. Then, I've created 20 copies of diskless domain (attached as well): test1..test20 and run my test4 program over them: ./test4 $(for ((i=1; i21; i++)); do echo -n test$i ; done) I had to tweak max_client_requests=20 to really run those 20 requests in parallel. However, what I am getting instead of nice measuring of speedup is: [9722] Starting test1 (round 20) ...[9725] Starting test4 (round 20) ...[9724] Starting test3 (round 20) ...[9723] Starting test2 (round 20) ...[9726] Starting test5 (round 20) ...[9727] Starting test6 (round 20) ...[9728] Starting test7 (round 20) ...[9729] Starting test8 (round 20) ...[9730] Starting test9 (round 20) ...[9731] Starting test10 (round 20) ...[9732] Starting test11 (round 20) ...[9733] Starting test12 (round 20) ...[9734] Starting test13 (round 20) ...[9735] Starting test14 (round 20) ...[9736] Starting test15 (round 20) ...[9737] Starting test16 (round 20) ...[9738] Starting test17 (round 20) ...[9741] Starting test20 (round 20) ...[9739] Starting test18 (round 20) ...[9740] Starting test19 (round 20) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test12libvir: error : An error occurred, but the cause is unknown Unable to start domain test14libvir: error : An error occurred, but the cause is unknown Unable to start domain test7libvir: error : An error occurred, but the cause is unknown Unable to start domain test1libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test12[9733] Destroying test12 ...[9728] Destroying test7 ...[9735] Destroying test14 ...[9733] Starting test12 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test7[9728] Starting test7 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test14[9735] Starting test14 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test18libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test1[9722] Destroying test1 ...[9722] Starting test1 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test8libvir: error : An error occurred, but the cause is unknown Unable to start domain test15[9725] Done I'm hitting this issue when I start the guest with the network interface in the original XML. If I delete it or change it to a regular (non-vepa) then everything works well. So the impact of this problem isn't that critical as originally reported. That is still critical enough that we must fix it before release IMHO 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] a script for libvirt-llxc + systemd on Fedora18
On Thu, Feb 28, 2013 at 06:03:46PM +0800, Gao feng wrote: On 2013/02/27 17:33, Kamezawa Hiroyuki wrote: Hi. At playing libvirt-lxc on Fedora18, I found that the user needs some workarounds to run /sbin/init as the root process of container. With Fedora15, I found this https://gist.github.com/peo3/1142202. And I know virt-sandbox-service have some clever way to understand RPM to make domain running with a service. Here is a script I wrote for F18 inspired by above two. Creating LXC domain running systemd as /init program. I'd like to get some feedbackand ...can't this kind of scripts be maintained by some virt-tools project ? Anyway, playing with lxc is fun. I'm glad if someone makes this script better :) == Thanks! I think this work that creating root filesystem/directories and xml configure files for container should be done by virt-install. Daniel,what's your idea? Should we implement the same feature in virt-install? I have always tended to view this as something to be done in a new tool claled virt-bootstrap, since it is a very different approach to the installation of guests than virt-install. 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 1/1] Clear PIIX3/PIIX4_USB capabilities for non-X86 platforms
On Thu, Feb 28, 2013 at 10:39:56 +0800, Li Zhang wrote: On Thu, Feb 28, 2013 at 10:06 AM, Li Zhang zhlci...@gmail.com wrote: I also hope that QEMU capabilities depend on the binary by QMP. But the flags in virQEMUCapsObjectTypes are all set in virQEMUCapsInitQMP. virQEMUCapsInitQMP - virQEMUCapsProbeQMPObjects - virQEMUCapsProcessStringFlags(qemuCaps, ARRAY_CARDINALITY(virQEMUCapsObjectTypes), virQEMUCapsObjectTypes, nvalues, values); Yes, but that's fine, virQEMUCapsObjectTypes is just a lookup table between qom objects and our internal qemu capabilities flags. virQEMUCapsProcessStringFlags will only set flags corresponding to the qom objects reported by qemu. More information from QEMU: I tried to execute qom-list-types command, I get a lot of return values including X86 and other platforms. I think this is the real problem. QEMU binary emulating PPC architecture should not advertise x86-specific objects. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Don't fail to shutdown domains with unresponsive agent
On 28.02.2013 02:13, Eric Blake wrote: On 02/27/2013 03:30 AM, Michal Privoznik wrote: On 27.02.2013 00:22, Eric Blake wrote: On 02/26/2013 04:02 AM, Michal Privoznik wrote: Currently, qemuDomainShutdownFlags() chooses the agent method of shutdown whenever the agent is configured. However, this assumption is not enough as the guest agent may be unresponsive at the moment. So unless guest agent method has been explicitly requested, we should fall back to the ACPI method. --- diff to v1: - Rework some conditions as Eric suggested in v1 src/qemu/qemu_driver.c | 38 ++ 1 file changed, 22 insertions(+), 16 deletions(-) ACK. Do you think this one is safe to push now? It is a bug fix and I think the code is well understood. However, the issue it's fixing doesn't seem to be so usual to hit. Otherwise there would be a bug report much sooner than 2012-12-22, right? https://bugzilla.redhat.com/show_bug.cgi?id=889635 I've hit the bug myself, but never filed a BZ - it's quite annoying that the shutdown button in virt-manager fails to work if you have the guest agent wired up in the host XML, but not installed and running in the guest, at which point you can no longer shut down the guest using virt-manager. Yes, I think this is safe to push for 1.0.3. Okay. Pushed. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virnettlscontexttest test fails on Fedora Rawhide x86_64
Still investigating, but the log is attached. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) = libvirt 1.0.2: tests/test-suite.log = # TOTAL: 83 # PASS: 80 # SKIP: 2 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 .. contents:: :depth: 2 FAIL: virnettlscontexttest == TEST: virnettlscontexttest 1) TLS Context ... OK 2) TLS Context ... OK 3) TLS Context ... OK 4) TLS Context ... OK 5) TLS Context ... OK 6) TLS Context ... libvir: XML-RPC error : Our own certificate servercert.pem failed validation against cacert5.pem: The certificate is not trusted. FAILED 7) TLS Context ... libvir: XML-RPC error : The certificate cacert6.pem is missing basic constraints for a CA OK 8) TLS Context ... libvir: XML-RPC error : Certificate cacert7.pem usage does not permit certificate signing OK 9) TLS Context ... OK 10) TLS Context ... OK 11) TLS Context ... OK 12) TLS Context ... OK 13) TLS Context ... OK 14) TLS Context ... OK 15) TLS Context ... OK 16) TLS Context ... libvir: XML-RPC error : Certificate servercert8.pem usage does not permit digital signature OK 17) TLS Context ... libvir: XML-RPC error : Certificate servercert9.pem purpose does not allow use for with a TLS server OK 18) TLS Context ... libvir: XML-RPC error : Certificate servercert10.pem usage does not permit digital signature OK 19) TLS Context ... OK 20) TLS Context ... OK 21) TLS Context ... OK 22) TLS Context ... OK 23) TLS Context ... OK 24) TLS Context ... OK 25) TLS Context ... OK 26) TLS Context ... libvir: XML-RPC error : Certificate clientcert8.pem usage does not permit digital signature OK 27) TLS Context ... libvir: XML-RPC error : Certificate clientcert9.pem purpose does not allow use for with a TLS client OK 28) TLS Context ... libvir: XML-RPC error : Certificate clientcert10.pem usage does not permit digital signature OK 29) TLS Context ... libvir: XML-RPC error : The CA certificate cacert.pem has expired OK 30) TLS Context ... libvir: XML-RPC error : The server certificate servercert.pem has expired OK 31) TLS Context ... libvir: XML-RPC error : The client certificate clientcert.pem has expired OK 32) TLS Context ... libvir: XML-RPC error : The CA certificate cacert.pem is not yet active OK 33) TLS Context ... libvir: XML-RPC error : The server certificate servercert.pem is not yet active OK 34) TLS Context ... libvir: XML-RPC error : The client certificate clientcert.pem is not yet active OK 35) TLS Session ... OK 36) TLS Session ... libvir: XML-RPC error : authentication failed: Failed to verify peer's certificate OK 37) TLS Session ... OK 38) TLS Session ... OK 39) TLS Session
Re: [libvirt] [PATCH 1/1] Clear PIIX3/PIIX4_USB capabilities for non-X86 platforms
On 2013年02月28日 19:39, Jiri Denemark wrote: On Thu, Feb 28, 2013 at 10:39:56 +0800, Li Zhang wrote: On Thu, Feb 28, 2013 at 10:06 AM, Li Zhang zhlci...@gmail.com wrote: I also hope that QEMU capabilities depend on the binary by QMP. But the flags in virQEMUCapsObjectTypes are all set in virQEMUCapsInitQMP. virQEMUCapsInitQMP - virQEMUCapsProbeQMPObjects - virQEMUCapsProcessStringFlags(qemuCaps, ARRAY_CARDINALITY(virQEMUCapsObjectTypes), virQEMUCapsObjectTypes, nvalues, values); Yes, but that's fine, virQEMUCapsObjectTypes is just a lookup table between qom objects and our internal qemu capabilities flags. virQEMUCapsProcessStringFlags will only set flags corresponding to the qom objects reported by qemu. More information from QEMU: I tried to execute qom-list-types command, I get a lot of return values including X86 and other platforms. I think this is the real problem. QEMU binary emulating PPC architecture should not advertise x86-specific objects. It seems that command qom-list-types just give all architectures' list, not related with architecture. qemu-system-x86_64 also can get all information of other architectures. I think it is not right way to getting capabilities by qom-list-types. To fix this problem can be either way of the following: 1. Specify the architectures in Libvirt as my patch. 2. Specify the architectures in QEMU by modifying qom-list-types command. In my opinion, it may be better to fix it in libvirt. :) Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virnettlscontexttest test fails on Fedora Rawhide x86_64
On Thu, Feb 28, 2013 at 11:44:02AM +, Richard W.M. Jones wrote: Still investigating, but the log is attached. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) = libvirt 1.0.2: tests/test-suite.log = # TOTAL: 83 # PASS: 80 # SKIP: 2 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 .. contents:: :depth: 2 FAIL: virnettlscontexttest == TEST: virnettlscontexttest 1) TLS Context ... OK 2) TLS Context ... OK 3) TLS Context ... OK 4) TLS Context ... OK 5) TLS Context ... OK 6) TLS Context ... libvir: XML-RPC error : Our own certificate servercert.pem failed validation against cacert5.pem: The certificate is not trusted. FAILED In that test case we're creating a CA cert which has the key-usage policy set to digital signature instead of key signing. However we also set the flag non-critical so a failing key usage policy check should still result in a pass from cert validation. Sounds like gnutls3 isn't liking this. 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 1/1] Clear PIIX3/PIIX4_USB capabilities for non-X86 platforms
On Thu, Feb 28, 2013 at 19:47:47 +0800, Li Zhang wrote: On 2013年02月28日 19:39, Jiri Denemark wrote: I tried to execute qom-list-types command, I get a lot of return values including X86 and other platforms. I think this is the real problem. QEMU binary emulating PPC architecture should not advertise x86-specific objects. It seems that command qom-list-types just give all architectures' list, not related with architecture. qemu-system-x86_64 also can get all information of other architectures. I think it is not right way to getting capabilities by qom-list-types. Yeah, it's either bug in qemu or libvirt should use another way of probing this stuff. To fix this problem can be either way of the following: 1. Specify the architectures in Libvirt as my patch. No way. This won't work if you want to start PPC domain on an x86 host or x86 domain on a PPC host. 2. Specify the architectures in QEMU by modifying qom-list-types command. Perhaps. We definitely need to find the right way of probing the data we care about. And it's likely both libvirt and qemu will need to be modified. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] Clear PIIX3/PIIX4_USB capabilities for non-X86 platforms
On 2013年02月28日 19:47, Li Zhang wrote: On 2013年02月28日 19:39, Jiri Denemark wrote: On Thu, Feb 28, 2013 at 10:39:56 +0800, Li Zhang wrote: On Thu, Feb 28, 2013 at 10:06 AM, Li Zhang zhlci...@gmail.com wrote: It seems that command qom-list-types just give all architectures' list, not related with architecture. qemu-system-x86_64 also can get all information of other architectures. Sorry, qemu-syste-x86_64 didn't get list of other platforms. I just saw wrong information. I think it is not right way to getting capabilities by qom-list-types. To fix this problem can be either way of the following: 1. Specify the architectures in Libvirt as my patch. 2. Specify the architectures in QEMU by modifying qom-list-types command. In my opinion, it may be better to fix it in libvirt. :) So I will fix this in QEMU. Please ignore this patch. Thanks, Jiri. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/5] snapshot revert-and-create
On 11/21/12 01:36, Eric Blake wrote: v2 was here: https://www.redhat.com/archives/libvir-list/2012-November/msg00818.html One patch from v2 has already been committed. This patch additionally adds a qemu implementation for the new flag, and I have tested creation of offline branches (I still need to test creation of a branch from a disk-only or online external snapshot). I've also started documenting my plans for a new revert FOLLOW flag, which updates to the current state of external files (rather than reverting completely to the point where the snapshot was taken); once that is working, then implementing the combination of createXML(LIVE|BRANCH) will really be creating the branch, then farming out to revert(FOLLOW) to switch over to the new branch. Eric Blake (5): snapshot: add revert-and-create branching of external snapshots snapshot: prepare to parse new XML snapshot: actually compute branch definition from XML snapshot: support revert-and-create branching in qemu snapshot: add another revert API flag Hi Eric, this code has been buried long enough so that it doesn't apply. I'm reviving my snapshot patches and would like to integrate this functionality too. If you happen to have the branch still in you repo, could you please push it somewhere so git can help me while rebasing. Thanks Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: fix graphics port allocation
On 02/28/13 00:17, Eric Blake wrote:
On 02/27/2013 04:51 AM, Ján Tomko wrote:
Only release ports that have been allocated before.
This fixes these issues:
* trying to release ports when qemuProcessStart fails before port
allocation
* trying to release the SPICE TLS port if spice_tls is 0
* failing to release SPICE port with autoport=off (when only one
of them is -1)
---
v1: https://www.redhat.com/archives/libvir-list/2013-February/msg01464.html
Use a pair of booleans in domain private data instead of a new field
in the domain definition.
Closer, but where is this information saved across libvirtd restarts?
Hint: qemu_domain.c:qemuDomainObjPrivateXML{Format,Parse}()
We don't preserve the port allocator state across restarts, so saving
this would do nothing in the best case. (the worse cases being printing
an error or (possibly?) marking a used port as unused).
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix crash in QEMU auto-destroy with transient guests
From: Daniel P. Berrange berra...@redhat.com
When the auto-destroy callback runs it is supposed to return
NULL if the virDomainObjPtr is no longer valid. It was not
doing this for transient guests, so we tried to virObjectUnlock
a mutex which had been freed. This often led to a crash.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index db95d6e..1b9eede 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4629,8 +4629,10 @@ qemuProcessAutoDestroy(virQEMUDriverPtr driver,
if (!qemuDomainObjEndJob(driver, dom))
dom = NULL;
-if (dom !dom-persistent)
+if (dom !dom-persistent) {
qemuDomainRemoveInactive(driver, dom);
+dom = NULL;
+}
if (event)
qemuDomainEventQueue(driver, event);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] docs: Describe the dhcp 'enable' and 'relay' attributes
On Wed, Feb 27, 2013 at 09:38:33PM -0700, Eric Blake wrote: On 02/27/2013 09:29 PM, TJ wrote: [...] And a big THANK YOU for contributing to the community. Too often, I forget to express gratitude for new contributors braving the unknown waters of posting to a list where they are not sure of the conventions. Honestly, we aren't trying to scare you off :) Definitely ! Honnestly a rather good patchset for a first contribution :-) thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release candidate 2 of 1.0.3 is available
On Wed, Feb 27, 2013 at 03:26:24PM +0100, Michal Privoznik wrote: On 27.02.2013 12:20, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are at the usual place too: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests. Please give it more testing and let's keep change to git to bug fixes to minimize risks for 1.0.3. If everything goes well I will push 1.0.3 on Friday, thanks ! Daniel Maybe we want to postpone the release a bit; I was trying to find out how drop of qemu driver lock speeds up parallel startup of multiple domains, however - it is not working. I've created small test program - test4.c and run it against current HEAD. It takes arguments - domain names - which it spawns a separate thread per each domain and repeatedly starts and destroys domains. See code for more. Then, I've created 20 copies of diskless domain (attached as well): test1..test20 and run my test4 program over them: ./test4 $(for ((i=1; i21; i++)); do echo -n test$i ; done) I had to tweak max_client_requests=20 to really run those 20 requests in parallel. However, what I am getting instead of nice measuring of speedup is: [9722] Starting test1 (round 20) ...[9725] Starting test4 (round 20) ...[9724] Starting test3 (round 20) ...[9723] Starting test2 (round 20) ...[9726] Starting test5 (round 20) ...[9727] Starting test6 (round 20) ...[9728] Starting test7 (round 20) ...[9729] Starting test8 (round 20) ...[9730] Starting test9 (round 20) ...[9731] Starting test10 (round 20) ...[9732] Starting test11 (round 20) ...[9733] Starting test12 (round 20) ...[9734] Starting test13 (round 20) ...[9735] Starting test14 (round 20) ...[9736] Starting test15 (round 20) ...[9737] Starting test16 (round 20) ...[9738] Starting test17 (round 20) ...[9741] Starting test20 (round 20) ...[9739] Starting test18 (round 20) ...[9740] Starting test19 (round 20) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test12libvir: error : An error occurred, but the cause is unknown Unable to start domain test14libvir: error : An error occurred, but the cause is unknown Unable to start domain test7libvir: error : An error occurred, but the cause is unknown Unable to start domain test1libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test12[9733] Destroying test12 ...[9728] Destroying test7 ...[9735] Destroying test14 ...[9733] Starting test12 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test7[9728] Starting test7 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test14[9735] Starting test14 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test18libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test1[9722] Destroying test1 ...[9722] Starting test1 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test8libvir: error : An error occurred, but the cause is unknown Unable to start domain test15[9725] Done Do you have corresponding logs from libvirtd, and / or is there anything in /var/log/libvirt/qemu/$GUEST.log 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] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 16:42, Anthony Liguori ha scritto: There's such thing as list support in QemuOpts. The only way QemuOptsVisitor was able to implement it was to expose QemuOpts publicly via options_int.h and rely on a implementation detail. There are fixed types supported by QemuOpts. It just so happens that whenever qemu_opt_set() is called, instead of replacing the last instance, the value is either prepended or appended in order to implement a replace or set-if-unset behavior. Fair enough. Nobody said the implementation is pretty. If we want to have list syntax, we need to introduce first class support for it. Here's a simple example of how to do this. If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. No, I'm primarily motivated by the UI and am assuming that ya'll are arguing based on the implementation today. Your assumption is incorrect :) Repeating keys is quite mad. Here are some examples: qemu -numa node=1,node=2,cpus=2,cpus=3 What would a user expect that that would mean. I figure you mean -numa node,nodeid=1,nodeid=2,cpus=2,cpus=3 Parameter nodeid is int-valued, therefore repeating it is either an error, or the last one wins. Matter of taste. Currently, we do the latter. Parameter cpus is list-valued, therefore the values accumulate. We already do that. Taken together, this configures node 1 to use cpus 2 and 3. What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 With respect to escaping, for string lists (the only place where this point is even relevant), we can simply support escaping. But I'd like to hear a use-case for a string list first. There is no need for the syntactic warts you so cavalierly propose. Whenever you do key=X:Y, I can do key=X,key=Y. Whatever semantics you propose for the former, I'll propose for the latter. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virnettlscontexttest test fails on Fedora Rawhide x86_64
On Thu, Feb 28, 2013 at 11:53:12AM +, Daniel P. Berrange wrote: [...] In that test case we're creating a CA cert which has the key-usage policy set to digital signature instead of key signing. However we also set the flag non-critical so a failing key usage policy check should still result in a pass from cert validation. Sounds like gnutls3 isn't liking this. I've filed this bug, initially against libvirt although it probably needs to be reassigned to gnutls: https://bugzilla.redhat.com/show_bug.cgi?id=916603 I have also added a patch to the Rawhide libvirt package to disable this test for now. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 17:19, Eduardo Habkost ha scritto: If it is meant as a prototype only, and the final command-line syntax would be with repeated keys, that's okay. I think that Eduardo/Markus/I are focusing on the user interface, you're focusing in the implementation. In the meanwhile, however, it seems to me that Eduardo can use QemuOptsVisitor---which can also hide the details and provide type safety. Whatever I use to implement it, I still need to know how the command-line syntax will look like, because we need to tell libvirt developers how they should write the QEMU command-line. I don't think any syntax makes sense except cpus=A,cpus=B. How we implement it is another story. Agreed on both counts. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release candidate 2 of 1.0.3 is available
On 28.02.2013 13:42, Daniel P. Berrange wrote: On Wed, Feb 27, 2013 at 03:26:24PM +0100, Michal Privoznik wrote: On 27.02.2013 12:20, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are at the usual place too: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests. Please give it more testing and let's keep change to git to bug fixes to minimize risks for 1.0.3. If everything goes well I will push 1.0.3 on Friday, thanks ! Daniel Maybe we want to postpone the release a bit; I was trying to find out how drop of qemu driver lock speeds up parallel startup of multiple domains, however - it is not working. I've created small test program - test4.c and run it against current HEAD. It takes arguments - domain names - which it spawns a separate thread per each domain and repeatedly starts and destroys domains. See code for more. Then, I've created 20 copies of diskless domain (attached as well): test1..test20 and run my test4 program over them: ./test4 $(for ((i=1; i21; i++)); do echo -n test$i ; done) I had to tweak max_client_requests=20 to really run those 20 requests in parallel. However, what I am getting instead of nice measuring of speedup is: [9722] Starting test1 (round 20) ...[9725] Starting test4 (round 20) ...[9724] Starting test3 (round 20) ...[9723] Starting test2 (round 20) ...[9726] Starting test5 (round 20) ...[9727] Starting test6 (round 20) ...[9728] Starting test7 (round 20) ...[9729] Starting test8 (round 20) ...[9730] Starting test9 (round 20) ...[9731] Starting test10 (round 20) ...[9732] Starting test11 (round 20) ...[9733] Starting test12 (round 20) ...[9734] Starting test13 (round 20) ...[9735] Starting test14 (round 20) ...[9736] Starting test15 (round 20) ...[9737] Starting test16 (round 20) ...[9738] Starting test17 (round 20) ...[9741] Starting test20 (round 20) ...[9739] Starting test18 (round 20) ...[9740] Starting test19 (round 20) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test12libvir: error : An error occurred, but the cause is unknown Unable to start domain test14libvir: error : An error occurred, but the cause is unknown Unable to start domain test7libvir: error : An error occurred, but the cause is unknown Unable to start domain test1libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test12[9733] Destroying test12 ...[9728] Destroying test7 ...[9735] Destroying test14 ...[9733] Starting test12 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test7[9728] Starting test7 (round 19) ...libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test14[9735] Starting test14 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test18libvir: QEMU Driver error : Requested operation is not valid: domain is not running Unable to destroy domain test1[9722] Destroying test1 ...[9722] Starting test1 (round 19) ...libvir: error : An error occurred, but the cause is unknown Unable to start domain test8libvir: error : An error occurred, but the cause is unknown Unable to start domain test15[9725] Done Do you have corresponding logs from libvirtd, and / or is there anything in /var/log/libvirt/qemu/$GUEST.log Daniel Jirka uploaded them: http://people.redhat.com/jdenemar/libvirt/ Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: Il 27/02/2013 18:08, Anthony Liguori ha scritto: No, no, no. This makes ':' special, which means you can't have lists of anything containing ':'. Your cure is worse than the disease. Let go of that syntactic high-fructose corn syrup, stick to what we have and works just fine, thank you. Yes, there *must* be special syntax. If we're treating something special, then we should indicate to the user that it's special. Specifically, a list of integers should look distinctly different than overriding a previously specified integer. The solution is there is no way to override a previously specified key. Something like -device virtio-scsi-pci,num_queues=1,num_queues=2 now works, let's make it an error instead. That breaks compatibility. The above may seem silly but consider: qemu -device virtio-scsi-pci,num_queues=1,id=foo \ -set device.foo.num_queues=2 This is more common than you would think primarily as a way to override options that libvirt has set either via the qemu extra args tag or a script wrapper of qemu. Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Make sure qemuProcessStart is run within a job
qemuProcessStart expects to be run with a job already set and every
caller except for qemuMigrationPrepareAny use it correctly. This bug can
be observed in libvirtd logs during incoming migration as
warning : qemuDomainObjEnterMonitorInternal:979 : This thread seems
to be the async job owner; entering monitor without asking for a
nested job is dangerous
---
src/qemu/qemu_domain.c| 35 ---
src/qemu/qemu_domain.h| 4
src/qemu/qemu_migration.c | 11 +++
3 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index eca85fc..0e56596 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -881,6 +881,29 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
asyncJob);
}
+int
+qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = obj-privateData;
+
+if (asyncJob != priv-job.asyncJob) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unexpected async job %d), asyncJob);
+return -1;
+}
+
+if (priv-job.asyncOwner != virThreadSelfID()) {
+VIR_WARN(This thread doesn't seem to be the async job owner: %d,
+ priv-job.asyncOwner);
+}
+
+return qemuDomainObjBeginJobInternal(driver, obj,
+ QEMU_JOB_ASYNC_NESTED,
+ QEMU_ASYNC_JOB_NONE);
+}
+
/*
* obj must be locked before calling
@@ -955,17 +978,7 @@ qemuDomainObjEnterMonitorInternal(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = obj-privateData;
if (asyncJob != QEMU_ASYNC_JOB_NONE) {
-if (asyncJob != priv-job.asyncJob) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(unexpected async job %d), asyncJob);
-return -1;
-}
-if (priv-job.asyncOwner != virThreadSelfID())
-VIR_WARN(This thread doesn't seem to be the async job owner: %d,
- priv-job.asyncOwner);
-if (qemuDomainObjBeginJobInternal(driver, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE) 0)
+if (qemuDomainObjBeginNestedJob(driver, obj, asyncJob) 0)
return -1;
if (!virDomainObjIsActive(obj)) {
virReportError(VIR_ERR_OPERATION_FAILED, %s,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 30e6b97..e114f89 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -192,6 +192,10 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj,
enum qemuDomainAsyncJob asyncJob)
ATTRIBUTE_RETURN_CHECK;
+int qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+enum qemuDomainAsyncJob asyncJob)
+ATTRIBUTE_RETURN_CHECK;
bool qemuDomainObjEndJob(virQEMUDriverPtr driver,
virDomainObjPtr obj)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a58a79d..4c6d7e1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2118,6 +2118,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
goto endjob;
}
+if (qemuDomainObjBeginNestedJob(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_IN) 0)
+goto endjob;
+
/* Start the QEMU daemon, with the same command-line arguments plus
* -incoming $migrateFrom
*/
@@ -2126,9 +2130,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_QEMU_PROCESS_START_PAUSED |
VIR_QEMU_PROCESS_START_AUTODESTROY) 0) {
virDomainAuditStart(vm, migrated, false);
-/* Note that we don't set an error here because qemuProcessStart
- * should have already done that.
- */
+if (qemuDomainObjEndJob(driver, vm) 0)
+vm = NULL;
goto endjob;
}
@@ -2235,7 +2238,7 @@ stop:
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
endjob:
-if (!qemuMigrationJobFinish(driver, vm)) {
+if (vm !qemuMigrationJobFinish(driver, vm)) {
vm = NULL;
}
goto cleanup;
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Markus Armbruster arm...@redhat.com writes: Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. With QemuOpts, this is treated as two distinct sections with anonymous ids so whatever the code is to handle NUMA options would get called twice and it's up to that code to determine how it handles the conflict. (This is admittedly irrelevant to the discussion.) Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? Regards, Anthony Liguori With respect to escaping, for string lists (the only place where this point is even relevant), we can simply support escaping. But I'd like to hear a use-case for a string list first. There is no need for the syntactic warts you so cavalierly propose. Whenever you do key=X:Y, I can do key=X,key=Y. Whatever semantics you propose for the former, I'll propose for the latter. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix deadlock in QEMU close callback APIs
From: Daniel P. Berrange berra...@redhat.com
There is a lock ordering problem in the QEMU close callback
APIs.
When starting a guest we have a lock on the VM. We then
set a autodestroy callback, which acquires a lock on the
close callbacks.
When running auto-destroy, we obtain a lock on the close
callbacks, then run each callbacks - which obtains a lock
on the VM.
This causes deadlock if anyone tries to start a VM, while
autodestroy is taking place.
The fix is to do autodestroy in 2 phases. First obtain
all the callbacks and remove them from the list under
the close callback lock. Then invoke each callback
from outside the close callback lock.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_conf.c | 106 ++
src/util/virnetlink.c | 5 ++-
2 files changed, 92 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 1cd4b7c..f3b09cf 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -797,20 +797,37 @@ virQEMUCloseCallbacksGet(virQEMUCloseCallbacksPtr
closeCallbacks,
return cb;
}
+
+typedef struct _virQEMUCloseCallbacksListEntry virQEMUCloseCallbacksListEntry;
+typedef virQEMUCloseCallbacksListEntry *virQEMUCloseCallbacksListEntryPtr;
+struct _virQEMUCloseCallbacksListEntry {
+unsigned char uuid[VIR_UUID_BUFLEN];
+virQEMUCloseCallback callback;
+};
+
+
+typedef struct _virQEMUCloseCallbacksList virQEMUCloseCallbacksList;
+typedef virQEMUCloseCallbacksList *virQEMUCloseCallbacksListPtr;
+struct _virQEMUCloseCallbacksList {
+size_t nentries;
+virQEMUCloseCallbacksListEntryPtr entries;
+};
+
+
struct virQEMUCloseCallbacksData {
-virHashTablePtr list;
-virQEMUDriverPtr driver;
virConnectPtr conn;
+virQEMUCloseCallbacksListPtr list;
+bool oom;
};
+
static void
-virQEMUCloseCallbacksRunOne(void *payload,
+virQEMUCloseCallbacksGetOne(void *payload,
const void *key,
void *opaque)
{
struct virQEMUCloseCallbacksData *data = opaque;
qemuDriverCloseDefPtr closeDef = payload;
-virDomainObjPtr dom;
const char *uuidstr = key;
unsigned char uuid[VIR_UUID_BUFLEN];
@@ -823,35 +840,90 @@ virQEMUCloseCallbacksRunOne(void *payload,
if (data-conn != closeDef-conn || !closeDef-cb)
return;
-if (!(dom = virDomainObjListFindByUUID(data-driver-domains, uuid))) {
-VIR_DEBUG(No domain object with UUID %s, uuidstr);
+if (VIR_EXPAND_N(data-list-entries,
+ data-list-nentries, 1) 0) {
+data-oom = true;
return;
}
-dom = closeDef-cb(data-driver, dom, data-conn);
-if (dom)
-virObjectUnlock(dom);
+memcpy(data-list-entries[data-list-nentries - 1].uuid,
+ uuid, VIR_UUID_BUFLEN);
+data-list-entries[data-list-nentries - 1].callback = closeDef-cb;
+}
+
+
+static virQEMUCloseCallbacksListPtr
+virQEMUCloseCallbacksGetForConn(virQEMUCloseCallbacksPtr closeCallbacks,
+virConnectPtr conn)
+{
+virQEMUCloseCallbacksListPtr list = NULL;
+struct virQEMUCloseCallbacksData data;
+
+if (VIR_ALLOC(list) 0) {
+virReportOOMError();
+return NULL;
+}
+
+data.conn = conn;
+data.list = list;
+data.oom = false;
+
+virHashForEach(closeCallbacks-list, virQEMUCloseCallbacksGetOne, data);
-virHashRemoveEntry(data-list, uuid);
+if (data.oom) {
+VIR_FREE(list-entries);
+VIR_FREE(list);
+virReportOOMError();
+return NULL;
+}
+
+return list;
}
+
void
virQEMUCloseCallbacksRun(virQEMUCloseCallbacksPtr closeCallbacks,
virConnectPtr conn,
virQEMUDriverPtr driver)
{
-struct virQEMUCloseCallbacksData data = {
-.driver = driver,
-.conn = conn
-};
+virQEMUCloseCallbacksListPtr list;
+size_t i;
VIR_DEBUG(conn=%p, conn);
-virObjectLock(closeCallbacks);
-data.list = closeCallbacks-list;
-virHashForEach(data.list, virQEMUCloseCallbacksRunOne, data);
+/* We must not hold the lock while running the callbacks,
+ * so first we obtain the list of callbacks, then remove
+ * them all from the hash. At that point we can release
+ * the lock and run the callbacks safely. */
+virObjectLock(closeCallbacks);
+list = virQEMUCloseCallbacksGetForConn(closeCallbacks, conn);
+if (!list)
+return;
+
+for (i = 0 ; i list-nentries ; i++) {
+virHashRemoveEntry(closeCallbacks-list,
+ list-entries[i].uuid);
+}
virObjectUnlock(closeCallbacks);
+
+for (i = 0 ; i list-nentries ; i++) {
+virDomainObjPtr vm;
+
+if (!(vm = virDomainObjListFindByUUID(driver-domains,
+ list-entries[i].uuid))) {
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
[libvirt] [PATCH] Fix error report from nl_recvmsg
From: Daniel P. Berrange berra...@redhat.com
The nl_recvmsg does not always set errno. Instead it returns
its own custom set of error codes. Thus we were reporting the
wrong data.
---
src/util/virnetlink.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..8b47ede 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,8 +335,9 @@ virNetlinkEventCallback(int watch,
if (length == 0)
return;
if (length 0) {
-virReportSystemError(errno,
- %s, _(nl_recv returned with error));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nl_recv returned with error: %s),
+ nl_geterror(length));
return;
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Markus Armbruster arm...@redhat.com writes: Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? My fundamental problem here is that we have the same syntax with different meanings depending on the context. Going back to our original example: qemu -numa node,nodeid=2,cpus=4 This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. Regards, Anthony Liguori -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix deadlock in QEMU close callback APIs
On Thu, Feb 28, 2013 at 01:33:31PM +, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There is a lock ordering problem in the QEMU close callback
APIs.
When starting a guest we have a lock on the VM. We then
set a autodestroy callback, which acquires a lock on the
close callbacks.
When running auto-destroy, we obtain a lock on the close
callbacks, then run each callbacks - which obtains a lock
on the VM.
This causes deadlock if anyone tries to start a VM, while
autodestroy is taking place.
The fix is to do autodestroy in 2 phases. First obtain
all the callbacks and remove them from the list under
the close callback lock. Then invoke each callback
from outside the close callback lock.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_conf.c | 106
++
src/util/virnetlink.c | 5 ++-
2 files changed, 92 insertions(+), 19 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..8b47ede 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,8 +335,9 @@ virNetlinkEventCallback(int watch,
if (length == 0)
return;
if (length 0) {
-virReportSystemError(errno,
- %s, _(nl_recv returned with error));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nl_recv returned with error: %s),
+ nl_geterror(length));
return;
}
This chunk wasn't supposed to be here. I've sent it as a separate fix
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] Fix deadlock in QEMU close callback APIs
On Thu, Feb 28, 2013 at 13:33:31 +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com There is a lock ordering problem in the QEMU close callback APIs. When starting a guest we have a lock on the VM. We then set a autodestroy callback, which acquires a lock on the close callbacks. When running auto-destroy, we obtain a lock on the close callbacks, then run each callbacks - which obtains a lock on the VM. This causes deadlock if anyone tries to start a VM, while autodestroy is taking place. The fix is to do autodestroy in 2 phases. First obtain all the callbacks and remove them from the list under the close callback lock. Then invoke each callback from outside the close callback lock. Looks like we can just remove 568a6cda277f04ab9baaeb97490e548b7b608aa6 then, can't we? Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix deadlock in QEMU close callback APIs
On Thu, Feb 28, 2013 at 02:50:34PM +0100, Jiri Denemark wrote: On Thu, Feb 28, 2013 at 13:33:31 +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com There is a lock ordering problem in the QEMU close callback APIs. When starting a guest we have a lock on the VM. We then set a autodestroy callback, which acquires a lock on the close callbacks. When running auto-destroy, we obtain a lock on the close callbacks, then run each callbacks - which obtains a lock on the VM. This causes deadlock if anyone tries to start a VM, while autodestroy is taking place. The fix is to do autodestroy in 2 phases. First obtain all the callbacks and remove them from the list under the close callback lock. Then invoke each callback from outside the close callback lock. Looks like we can just remove 568a6cda277f04ab9baaeb97490e548b7b608aa6 then, can't we? Quite possibly. I'll investigate that further, and test what happens. 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] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 28/02/2013 14:32, Anthony Liguori ha scritto: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? For example because another kind of string list could have a colon inside, and that would in turn need escaping (in fact that's already the case for guestfwd); repeating the key is a syntax that is easily reusable (and indeed is already in use). Instead, the '-' is parsed within the NUMA code, and is completely opaque to QemuOpts. The NUMA code knows that the '-' will never need escaping, because it only deals with positive integers. A perhaps better question would have been if you're okay with making ',' be special syntax, why not ':'. And the answer is that indeed ',' already brings some problems, but likely they outweight the advantages of having say only a -set option. But adding a second escaped character is already much more debatable in my opinion. What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? What would the QemuOpts parsing code do? Do you have in mind bitmasks as a first-class QemuOpts type? If so, that would be an argument in favor of ':', but against the prototype patch you posted yesterday. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Il 28/02/2013 14:41, Anthony Liguori ha scritto: This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. This makes your point clear, but it sounds a bit artificial. 4 or 8 would never appear alone. You would likely have something like -numa node,nodeid=0,cpus=0,cpus=12 \ -numa node,nodeid=1,cpus=1,cpus=13 \ -numa node,nodeid=2,cpus=2,cpus=14 \ -numa node,nodeid=3,cpus=3,cpus=15 \ -numa node,nodeid=4,cpus=4,cpus=8 which would make the syntax much more obvious. Something like 4:8 would be rather unclear actually, because both numbes are even. Given -numa node,nodeid=2,cpus=4:8 out of context, I would guess that 4:8 is [4,8) where the upper-bound is excluded for some reason. Of course context would clear it up, but that also applies to cpus=foo,cpus=bar. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virnetdevmacvlan.c: Introduce mutex for macvlan creation
Currently, after we removed the qemu driver lock, it may happen
that two or more threads will start up a machine with macvlan and
race over virNetDevMacVLanCreateWithVPortProfile(). However,
there's a racy section in which we are generating a sequence of
possible device names and detecting if they exits. If we found
one which doesn't we try to create a device with that name.
However, the other thread is doing just the same. Assume it will
succeed and we must therefore fail. If this happens more than 5
times (which in massive parallel startup surely will) we return
-1 without any error reported. This patch is a simple hack to
both of these problems. It introduces a mutex, so only one thread
will enter the section, and if it runs out of possibilities,
error is reported. Moreover, the number of retries is raised to 20.
---
This is just a quick hack which aim is to be as small as possible in order to
be well understood and hence included in the release. After the release, this
should be totally dropped in flavour of virNetDevNameAllocator.
src/util/virnetdevmacvlan.c | 38 --
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index a74db1e..bf5e7e0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -31,6 +31,7 @@
#include virmacaddr.h
#include virutil.h
#include virerror.h
+#include virthread.h
#define VIR_FROM_THIS VIR_FROM_NET
@@ -71,6 +72,15 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIX macvlan
# define MACVLAN_NAME_PATTERN macvlan%d
+virMutex virNetDevMacVLanCreateMutex;
+
+static int virNetDevMacVLanCreateMutexOnceInit(void)
+{
+return virMutexInit(virNetDevMacVLanCreateMutex);
+}
+
+VIR_ONCE_GLOBAL_INIT(virNetDevMacVLanCreateMutex);
+
/**
* virNetDevMacVLanCreate:
*
@@ -829,7 +839,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
char ifname[IFNAMSIZ];
int retries, do_retry = 0;
uint32_t macvtapMode;
-const char *cr_ifname;
+const char *cr_ifname = NULL;
int ret;
int vf = -1;
@@ -870,23 +880,39 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
return -1;
} else {
create_name:
-retries = 5;
+if (virNetDevMacVLanCreateMutexInitialize() 0) {
+virReportSystemError(errno, %s, _(unable to init mutext));
+return -1;
+}
+
+retries = 20;
+virMutexLock(virNetDevMacVLanCreateMutex);
for (c = 0; c 8192; c++) {
snprintf(ifname, sizeof(ifname), pattern, c);
-if ((ret = virNetDevExists(ifname)) 0)
+if ((ret = virNetDevExists(ifname)) 0) {
+virMutexUnlock(virNetDevMacVLanCreateMutex);
return -1;
+}
if (!ret) {
rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
macvtapMode, do_retry);
-if (rc == 0)
+if (rc == 0) {
+cr_ifname = ifname;
break;
+}
if (do_retry --retries)
continue;
-return -1;
+break;
}
}
-cr_ifname = ifname;
+
+virMutexUnlock(virNetDevMacVLanCreateMutex);
+if (!cr_ifname) {
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+ _(Unable to create macvlan device));
+return -1;
+}
}
if (virNetDevVPortProfileAssociate(cr_ifname,
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error report from nl_recvmsg
On 02/28/2013 08:37 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The nl_recvmsg does not always set errno. Instead it returns
its own custom set of error codes. Thus we were reporting the
wrong data.
---
src/util/virnetlink.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..8b47ede 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,8 +335,9 @@ virNetlinkEventCallback(int watch,
if (length == 0)
return;
if (length 0) {
-virReportSystemError(errno,
- %s, _(nl_recv returned with error));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nl_recv returned with error: %s),
+ nl_geterror(length));
My recollection is that we specifically avoided calling nl_geterror()
because it isn't threadsafe.
I'll go take another look to verify.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error report from nl_recvmsg
On Thu, Feb 28, 2013 at 11:11:53AM -0500, Laine Stump wrote:
On 02/28/2013 08:37 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The nl_recvmsg does not always set errno. Instead it returns
its own custom set of error codes. Thus we were reporting the
wrong data.
---
src/util/virnetlink.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..8b47ede 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,8 +335,9 @@ virNetlinkEventCallback(int watch,
if (length == 0)
return;
if (length 0) {
-virReportSystemError(errno,
- %s, _(nl_recv returned with error));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nl_recv returned with error: %s),
+ nl_geterror(length));
My recollection is that we specifically avoided calling nl_geterror()
because it isn't threadsafe.
I'll go take another look to verify.
I did check this, but only for libnl3 which merely does a static
string table lookup:
const char *nl_geterror(int error)
{
error = abs(error);
if (error NLE_MAX)
error = NLE_FAILURE;
return errmsg[error];
}
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] Fix error report from nl_recvmsg
On Thu, Feb 28, 2013 at 04:16:37PM +, Daniel P. Berrange wrote:
On Thu, Feb 28, 2013 at 11:11:53AM -0500, Laine Stump wrote:
On 02/28/2013 08:37 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The nl_recvmsg does not always set errno. Instead it returns
its own custom set of error codes. Thus we were reporting the
wrong data.
---
src/util/virnetlink.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..8b47ede 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -335,8 +335,9 @@ virNetlinkEventCallback(int watch,
if (length == 0)
return;
if (length 0) {
-virReportSystemError(errno,
- %s, _(nl_recv returned with error));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nl_recv returned with error: %s),
+ nl_geterror(length));
My recollection is that we specifically avoided calling nl_geterror()
because it isn't threadsafe.
I'll go take another look to verify.
I did check this, but only for libnl3 which merely does a static
string table lookup:
Oh joy, it is worse than you could possibly imagine.
On libnl1 the return value is a valid -errno, while in libnl3
the return value is an error code of its own invention.
Further in libnl1 we can';t rely on the global errno, because
other calls libnl does may have overritten it with garbage.
We must use the return value from the function.
For yet more fun, libnl1's error handling is not threadsafe.
Whenever it hits an error with a syscall, it internally
runs __nl_error() which mallocs/frees a static global
variable containing the contents of strerror() which is
itself also not threadsafe :-(
Did I mention we should just throw out all versions of libnl
entirely and talk to the kernel ourselves. It has caused
us no end of pain in all its versions.
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] Fix error report from nl_recvmsg
On Thu, Feb 28, 2013 at 04:24:17PM +, Daniel P. Berrange wrote: On Thu, Feb 28, 2013 at 04:16:37PM +, Daniel P. Berrange wrote: [...] Oh joy, it is worse than you could possibly imagine. On libnl1 the return value is a valid -errno, while in libnl3 the return value is an error code of its own invention. Further in libnl1 we can';t rely on the global errno, because other calls libnl does may have overritten it with garbage. We must use the return value from the function. For yet more fun, libnl1's error handling is not threadsafe. Whenever it hits an error with a syscall, it internally runs __nl_error() which mallocs/frees a static global variable containing the contents of strerror() which is itself also not threadsafe :-( Did I mention we should just throw out all versions of libnl entirely and talk to the kernel ourselves. It has caused us no end of pain in all its versions. No chance of educating them instead ? We can't rewrite everything :) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix deadlock in QEMU close callback APIs
On Thu, Feb 28, 2013 at 01:33:31PM +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com There is a lock ordering problem in the QEMU close callback APIs. When starting a guest we have a lock on the VM. We then set a autodestroy callback, which acquires a lock on the close callbacks. When running auto-destroy, we obtain a lock on the close callbacks, then run each callbacks - which obtains a lock on the VM. This causes deadlock if anyone tries to start a VM, while autodestroy is taking place. The fix is to do autodestroy in 2 phases. First obtain all the callbacks and remove them from the list under the close callback lock. Then invoke each callback from outside the close callback lock. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_conf.c | 106 ++ src/util/virnetlink.c | 5 ++- 2 files changed, 92 insertions(+), 19 deletions(-) Incidentally this patch is also a huge performance win. Previously once autodestroy starts running it blocks all startup/shutdown of VMs. When a single client had created 1000 VMs, this blocked libvirtd for a very long time indeed. With this change we get full parallelism in auto-destroy since only 1 VM at a time is locked, and other VMs can continue to start/stop Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Revert hack for autodestroy in qemuProcessStop
From: Daniel P. Berrange berra...@redhat.com
This reverts the hack done in
commit 568a6cda277f04ab9baaeb97490e548b7b608aa6
Author: Jiri Denemark jdene...@redhat.com
Date: Fri Feb 15 15:11:47 2013 +0100
qemu: Avoid deadlock in autodestroy
since we now have a fix which avoids the deadlock scenario
entirely
---
src/qemu/qemu_conf.h| 4
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_process.c | 8 +---
3 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d547d97..6bb3dee 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -254,10 +254,6 @@ struct qemuDomainDiskInfo {
int io_status;
};
-/*
- * To avoid a certain deadlock this callback must never call any
- * virQEMUCloseCallbacks* API.
- */
typedef virDomainObjPtr (*virQEMUCloseCallback)(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 30e6b97..dacbb54 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -138,7 +138,6 @@ struct _qemuDomainObjPrivate {
bool gotShutdown;
bool beingDestroyed;
-bool autoDestroyed;
char *pidfile;
int nvcpupids;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1b9eede..9f1507a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4232,8 +4232,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainCleanupRun(driver, vm);
/* Stop autodestroy in case guest is restarted */
-if (!priv-autoDestroyed)
-qemuProcessAutoDestroyRemove(driver, vm);
+qemuProcessAutoDestroyRemove(driver, vm);
/* now that we know it's stopped call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
@@ -4614,13 +4613,8 @@ qemuProcessAutoDestroy(virQEMUDriverPtr driver,
VIR_DEBUG(Killing domain);
-/* We need to prevent qemuProcessStop from removing this function from
- * closeCallbacks since that would cause a deadlock.
- */
-priv-autoDestroyed = true;
qemuProcessStop(driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED,
VIR_QEMU_PROCESS_STOP_MIGRATED);
-priv-autoDestroyed = false;
virDomainAuditStop(dom, destroyed);
event = virDomainEventNewFromObj(dom,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Related: overwrite something you got from a config file on the command line. In both your example and mine, we have entirely separate options, and they have perfectly ordinary overwrite semantics: each option overwrites the given keys with the given values. The last key=value wins. This usage makes sense, and we obviously want to preserve it. Paolo's example is different only in that it's a silly. Preserving compatibility may mean that once we accepted silly usage, we can't ever reject it. Debatable. Personally, I disagree: I think we could outlaw repeating keys within the same option argument / configuration file section just fine. Finally, I don't think that we must have fancy-pants syntax to remind users that they're configuring a list. What's the chance of confusion there? What user would expect num_queues=1,num_queues=2 to make num_queues magically become a list? My fundamental problem here is that we have the same syntax with different meanings depending on the context. Going back to our original example: qemu -numa node,nodeid=2,cpus=4 This is certainly ambiguous. Does this mean that you have a single cpu for the node (VCPU 4) or does it mean the node have 4 cpus (presumably ranged 0-3). Root cause: the name cpus can be interpreted as number of cpus or as list of cpus. Fix (if it's worth fixing): use a better name. First one that crossed my mind: cpu. Given that ambiguity the following: qemu -numa node,nodeid=2,cpus=4,cpus=8 Does help the situation. A reasonable person could assume that cpus=8 overrides the previous cpus=4 (as it does elsewhere in QEMU) and I suspect a reasonable person is blissfully unaware of the fact that he can give the same key several times in a single option argument, let alone what happens when he does. And I still think we could outlaw such repetition if we cared. Besides the command line, there's also the config file. As Paolo explained, repeated key means list is established practice there. therefore assume they were creating a node with 8 CPUS (0-7) instead of two cpus. However: qemu -numa node,nodeid=2,cpus=4:8 Is much less ambiguous. Granted, it's not immediately obvious whether this is a range specification or a disjoint specification but it's more clear than the previous syntax. Does it mean CPU 4 and 8? CPU 4 to 8? 8 CPUs starting with 4? If it's less ambiguous, then probably because it's sufficiently greek to make people reach for the manual :) Moreover, no change, thus no improvement for your original example cpus=4, which you called certainly ambigous. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix crash in QEMU auto-destroy with transient guests
On 02/28/2013 05:19 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
When the auto-destroy callback runs it is supposed to return
NULL if the virDomainObjPtr is no longer valid. It was not
doing this for transient guests, so we tried to virObjectUnlock
a mutex which had been freed. This often led to a crash.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
ACK.
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index db95d6e..1b9eede 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4629,8 +4629,10 @@ qemuProcessAutoDestroy(virQEMUDriverPtr driver,
if (!qemuDomainObjEndJob(driver, dom))
dom = NULL;
-if (dom !dom-persistent)
+if (dom !dom-persistent) {
qemuDomainRemoveInactive(driver, dom);
+dom = NULL;
+}
if (event)
qemuDomainEventQueue(driver, event);
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Anthony Liguori anth...@codemonkey.ws writes: Paolo Bonzini pbonz...@redhat.com writes: What about: [numa] node=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node=1,cpus=1 I figure you mean [numa] nodeid=1 cpus=2 cpus=3 qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1 The config file configures node 1 to use cpus 2 and 3. The command line configures node 1 to use cpus 1. QemuOpts can optionally be made to merge options with the same ID. We use that in cases like -machine, where multiple options make no sense, but merging them does. With QemuOpts, this is treated as two distinct sections with anonymous ids so whatever the code is to handle NUMA options would get called twice and it's up to that code to determine how it handles the conflict. (This is admittedly irrelevant to the discussion.) Correct. Merging options doesn't make sense for -numa. Therefore, configuration file and command line are *not* merged. In particular, the two cpus lists are not merged. So, what we have is two conflicting bits of configuration for the same thing. That's not a new problem, we got that everywhere. Either treat as error, or let the last one win. Matter of taste. Currently, we do the latter. I'm pretty sure you won't be able to say without looking at the source code. I hereby certify that I did not look at the source code, only at help output. So there. Now what about ranges? I'm pretty sure that what a user really wants to say is: qemu -numa node=1,cpus=0-3:8-11 This is easy to do with the patch I sent. We can add range support integer lists by just adding a check for '-' before doing dispatch. That's a heck of a lot nicer than: qemu -numa node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11 Let me pick up the baby you just threw out with the bathwater for you: qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11 If you're okay with making '-' be special syntax, why are you not okay with ':' being special syntax? Fair question! What I assume your proposing is making cpus be a string list and then parsing within the NUMA code. Why not do it all in QemuOpts core code? Yes, and another fair question. I want common QemuOpts syntax used instead of ad hoc syntax whenever practical, because ad hoc syntax is bound to breed inconsistencies and special cases. QemuOpts was created to get us out of that pit; let's not jump back in without a really good cause. We already got lists in QemuOpts. We don't have intervals in QemuOpts. If we had, I'd certainly argue for using them here. If more uses of intervals turn up, I'll argue for putting intervals in QemuOpts. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix crash in QEMU auto-destroy with transient guests
On 02/28/2013 07:19 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
When the auto-destroy callback runs it is supposed to return
NULL if the virDomainObjPtr is no longer valid. It was not
doing this for transient guests, so we tried to virObjectUnlock
a mutex which had been freed. This often led to a crash.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index db95d6e..1b9eede 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4629,8 +4629,10 @@ qemuProcessAutoDestroy(virQEMUDriverPtr driver,
if (!qemuDomainObjEndJob(driver, dom))
dom = NULL;
-if (dom !dom-persistent)
+if (dom !dom-persistent) {
qemuDomainRemoveInactive(driver, dom);
+dom = NULL;
+}
if (event)
qemuDomainEventQueue(driver, event);
ACK. That looks correct (qemuDomainRemoveInactive requires that there be
no other references to the domain, and most other calls to it are
followed by setting the domain ptr to NULL), and just as important it
fixes the crash that I was seeing running Daniel's multi-threaded
transient domain torture program.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] network: Bridge - Add support for a DHCP Relay Agent
On 02/27/2013 11:32 PM, TJ wrote: One thing I'm not yet sure how to deal with is the difference between build and deployment systems when one or the other doesn't have access to 'dhcp-helper', since as I understand it, some distributions do not include it in their package archives (it is available in Debian/Ubuntu). Just copy what we do for other helper executables, such as dnsmasq. If the user pre-defines $DNSMASQ as part of calling configure (that is, ./configure DNSMASQ=/alternate/dnsmasq), that pre-configuration is used regardless of whether it exists at the time. Otherwise, if the executable is present at configure time, then that path is hardcoded into that build. If neither pre-configured nor path search finds anything, then the basename is used, for a runtime PATH lookup. Distro packagers are smart enough to do build requirements and either prepulate the variable or pre-install the packages in their build environment, so that the distro build of libvirt will always have the desired absolute path to the distro executable. Someone building libvirt from a tarball has total control over what to use, and the default of a PATH lookup during configure with a fallback to a PATH lookup at runtime is generally the right thing (except for distros, most people don't build on one machine and install on another). So your patch already did the right thing, by copying and pasting what existing examples we already had for DNSMASQ. That then leaves the issue of different DHCP relay agents requiring different command-line parameters. I was looking at the discussion about the option element and wondering if that could have a dual use here, especially in the name= variety, e.g: We have to deal with different command line syntax in the case of nc (netcat); but that's normally a problem we don't have to worry about. If someone actually hits the problem, they can provide patches at that time. For now, assuming that you are targetting the one and only dhcp-relay program with known syntax is sufficient. In other words, you're worrying too much. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error report from nl_recvmsg
On 02/28/2013 11:37 AM, Daniel P. Berrange wrote: On Fri, Mar 01, 2013 at 12:31:34AM +0800, Daniel Veillard wrote: On Thu, Feb 28, 2013 at 04:24:17PM +, Daniel P. Berrange wrote: On Thu, Feb 28, 2013 at 04:16:37PM +, Daniel P. Berrange wrote: [...] Oh joy, it is worse than you could possibly imagine. Heh. I didn't see this message before my followup, or I would have just replied here. On libnl1 the return value is a valid -errno, while in libnl3 the return value is an error code of its own invention. Ugh. Another indication that we can't use the return value. Further in libnl1 we can';t rely on the global errno, because other calls libnl does may have overritten it with garbage. We must use the return value from the function. Or call nl_geterror(), which your patch does. For yet more fun, libnl1's error handling is not threadsafe. Whenever it hits an error with a syscall, it internally runs __nl_error() which mallocs/frees a static global That static global (errbuf) has been turned into static __thread char *errbuf; by patches in RHEL6 and Fedora. variable containing the contents of strerror() which is itself also not threadsafe :-( I think at this point it's down to this one problem (not sure why we didn't fix that the last time we visited this problem) Did I mention we should just throw out all versions of libnl entirely and talk to the kernel ourselves. It has caused us no end of pain in all its versions. No chance of educating them instead ? We can't rewrite everything :) Sure, it has been getting better over time, but that doesn't help us for all existing distros, particular rhel-5 and rhel-6 which libvirt is going to be crash-prone due to unsolvable libnl design flaws in those versions. Except for libnl's calling strerror, the thread-safety of error handling in libnl1 has been avoided in the versions in RHEL6 and older Fedoras (F18 libvirt uses libnl3) by changing the static global you mention above (errbuf) to thread-local: static __thread char *errbuf; In RHEL5, neither netcf nor macvtap is supported (netcf originally wasn't supported in RHEL5 because there was no libnl, and macvtap isn't even in the kernel - it requires kernel 2.6.34, but RHEL5 is using 2.6.18). This means that libnl isn't compiled into libvirt on RHEL5, and also isn't indirectly used, since netcf isn't supported. So we don't need to worry about RHEL5. Still there is the fact that glibc's strerror is getting called :-( Looking at the code there are two basic sets of APIs we rely on nl_ nla_ The nl_XXX APis are basically just wrappers around the normal socket() based APIs, hiding a few bits about the AF_NETLINK socket type. It would be trivially to do all that work ourselves, since socket() handling is nothing special. These are the APIs which have caused us multiple thread safety crash problems over the years. The nla_XXX APIs are all about complex data formatting, and we wouldn't want to try todo that ourselves. Fortunately the nla_XXX APIs are not the ones that are causing us trouble - AFAICT those look pretty safe in what they do fro a thread POV, since they're all just working on the object instances you pass in, no global state. That is an interesting idea, though. (It would have been an even better idea 2-3 years ago, if we were only clairvoyant :-) Or maybe we could just push to get strerror replaced with strerror_r in libnl1 (and push that into the various libnl1-using distros, then cross our fingers for the next thread safety problem). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] conf: DHCP - add state for DHCP Relay and On/Off switch
On 02/27/2013 10:26 PM, TJ wrote: On 28/02/13 03:15, Eric Blake wrote: Does your series ever allow dnsmasq and dhcprelay to run at the same time, or can we use a single pid_t field that covers the mutually exclusive choice of which helper is running based on the rest of the config? When local DHCP server services are disabled dnsmasq is still launched since there are several non-DHCP settings in the generated config. Right. a DNS server is run by default on any network that has an IP address defined. In my test-bed for these patches dnsmasq and dhcp-helper will be started alongside each other. If this series is accepted I was intending to propose adding dns enable='(yes|no)'/ You can consider that one pre-approved, as we've already discussed it to death and decided it was a good idea; just nobody has gotten around to doing it that way yet :-) (in other words, go ahead and write it; you'll be getting it off the todo lists of at least two of us.) in the same style used for dhcp so that dnsmasq can be Actually the enable attribute is totally unnecessary for dhcp, since an absence of dhcp implies that it is disabled (and indeed that is what already happens), and in the case of dhcp relay='yes'/, it is also implied (since a local dhcp server and dhcp relay are mutually exclusive). So, if there is no dhcp (or if it's dhcp relay='yes'/) and there is dns enable='no'/, then dnsmasq won't be run. The only reason we need an enable attribute for DNS is that the original default (when there is no dns element) is to enable the dns server, and we must preserve backward compatibility. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] conf: Network - add pointers to enabled virNetworkIpDef DHCP settings
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
Having previously introduced DHCP enabled and relay state within the
virNetworkIpDef structure - which can be one of many on each network -
these pointers allow us to track and easily access the DHCP state for
IPv4 and IPv6 when setting up the network without having to iterate
every virNetworkIpDef to find the DHCP state.
I'm not sure I like this. Having these convenience pointers is, er,
convenient, but it also means that you must maintain them, for example
during virNetworkUpdate* (a series of calls to this could potentially
remove all dhcp info from one IP address, and add it into another IP
address). That means more potential for getting it out of synce due to
missing a change in some obscure place.
Signed-off-by: TJ li...@iam.tj
---
src/conf/network_conf.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 8400eab..1889c45 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -231,6 +231,8 @@ struct _virNetworkDef {
virPortGroupDefPtr portGroups;
virNetDevBandwidthPtr bandwidth;
virNetDevVlan vlan;
+virNetworkIpDefPtr ipv4_dhcp;
+virNetworkIpDefPtr ipv6_dhcp;
};
typedef struct _virNetworkObj virNetworkObj;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/10] conf: Network - add ability to read/write XML DHCP state
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
Maintain backwards XML compatibility by assuming existing default values
and only adding the additional XML properties if settings are not
default.
Signed-off-by: TJ li...@iam.tj
---
src/conf/network_conf.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 3fc01cf..259de0a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -828,6 +828,19 @@ virNetworkDHCPDefParseXML(const char *networkName,
{
xmlNodePtr cur;
+char *tmp = NULL;
+
+def-dhcp_enabled = true;
+if ((tmp = virXMLPropString(node, enabled))) {
+def-dhcp_enabled = strncmp(no, tmp, 2) == 0 ? false :
def-dhcp_enabled;
+VIR_FREE(tmp);
+}
See my earlier comment about the lack of a need for an enable attribute
for dhcp.
+
+def-dhcp_relay = false;
+if ((tmp = virXMLPropString(node, relay))) {
+def-dhcp_relay = strncmp(yes, tmp, 3) == 0 ? true :
def-dhcp_relay;
+ VIR_FREE(tmp);
+}
At the end of this function you should check for dhcp_relay and if it's
true, verify that nothing else was included in the dhcp element. It's
okay/desirable for the parser to ignore extra stuff if it just doesn't
understand it, or if it's unknown whether or not the backend driver
would accept it, but in this case it's certain that if you want a dhcp
relay, adding an ip element etc would be nonsensical, so we might as
well save the backend driver the trouble of checking for it.
cur = node-children;
while (cur != NULL) {
@@ -2180,12 +2193,19 @@ virNetworkIpDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, tftp root='%s' /\n,
def-tftproot);
}
-if ((def-nranges || def-nhosts)) {
+if ((def-nranges || def-nhosts) ||
+ !def-dhcp_enabled || def-dhcp_relay) {
int ii;
-virBufferAddLit(buf, dhcp\n);
+virBufferAddLit(buf, dhcp);
+if (!def-dhcp_enabled)
+ virBufferAddLit(buf, enabled='no');
+ if (def-dhcp_relay)
+ virBufferAddLit(buf, relay='yes');
+ virBufferAddLit(buf, \n);
+
Since relay='yes' necessarily mandates that there can be no subelements
(at least until such time as we figure out options we need to add for a
dhcp relay and add them in), you could be extra tidy by just closing the
element right here on a single line, then skipping the rest of the
function (the parser should have already validated that there was no
extra information in the case of relay='yes')
virBufferAdjustIndent(buf, 2);
-for (ii = 0 ; ii def-nranges ; ii++) {
+for (ii = 0 ; def-nranges ii def-nranges ; ii++) {
char *saddr = virSocketAddrFormat(def-ranges[ii].start);
if (!saddr)
goto error;
@@ -2199,7 +2219,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
VIR_FREE(saddr);
VIR_FREE(eaddr);
}
-for (ii = 0 ; ii def-nhosts ; ii++) {
+for (ii = 0 ; def-nhosts ii def-nhosts ; ii++) {
virBufferAddLit(buf, host );
if (def-hosts[ii].mac)
virBufferAsprintf(buf, mac='%s' , def-hosts[ii].mac);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] conf: DHCP - add state for DHCP Relay and On/Off switch
On 02/27/2013 09:18 PM, TJ wrote:
Signed-off-by: TJ li...@iam.tj
---
src/conf/network_conf.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index c509915..8400eab 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -144,6 +144,10 @@ struct _virNetworkIpDef {
char *bootfile;
virSocketAddr bootserver;
+/* when false no DHCP server is started */
+bool dhcp_enabled;
+/* when true ranges are ignored and a DHCP relay-agent started */
+bool dhcp_relay;
I think Eric mentioned elsewhere that your patches are *extremely*
small. At least both the data structure, parser, and formatter changes
should go into a single patch. For that matter, this functionality is
small enough that you could put the struct change, parser change,
implementation of the feature in bridge_driver, makefile and configure
changes, AND the documentation addition into a single patch.
};
typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
@@ -234,6 +238,7 @@ typedef virNetworkObj *virNetworkObjPtr;
struct _virNetworkObj {
virMutex lock;
+pid_t dhcprelayPid;
pid_t dnsmasqPid;
pid_t radvdPid;
unsigned int active : 1;
--
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 04/10] conf: Network - keep track of active DHCP stanza in virNetworkDef
On 02/27/2013 09:57 PM, TJ wrote: From: TJ li...@iam.tj To avoid iterating all virNetworkIpDef entries when determining DHCP state keep track of the first enabled DHCP stanza in the network definition itself, for both IPv4 and IPv6. A by-product of this change is it allows the XML to contain more than one IP-DHCP stanza. The active DHCP stanza is the first enabled DHCP stanza. Hmm. This is an interesting idea, but I can't think of any other place in libvirt config where we allow putting the config in place and setting it disabled, and wouldn't want to set that precedent if it didn't already exist. Any other opinions about that? All other stanzas are retained which adds flexibility when multiple interfaces and routes might come and go since an alternative DHCP stanza can be selected and a refresh operation performed without needing to destroy/edit/start the network. Well, the network would still need to be updated (virNetworkUpdateFlags), and that operation. Signed-off-by: TJ li...@iam.tj --- src/conf/network_conf.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 259de0a..c5eab01 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1882,6 +1882,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (ret 0) goto error; def-nips++; + /* use only the first enabled DHCP definition */ + if (!def-ipv4_dhcp def-ips[ii].dhcp_enabled + VIR_SOCKET_ADDR_IS_FAMILY(def-ips[ii].address, AF_INET)) + def-ipv4_dhcp = def-ips[ii]; + if (!def-ipv6_dhcp def-ips[ii].dhcp_enabled + VIR_SOCKET_ADDR_IS_FAMILY(def-ips[ii].address, AF_INET6)) + def-ipv6_dhcp = def-ips[ii]; } } VIR_FREE(ipNodes); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix deadlock in QEMU close callback APIs
On Thu, Feb 28, 2013 at 13:33:31 +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com There is a lock ordering problem in the QEMU close callback APIs. When starting a guest we have a lock on the VM. We then set a autodestroy callback, which acquires a lock on the close callbacks. When running auto-destroy, we obtain a lock on the close callbacks, then run each callbacks - which obtains a lock on the VM. This causes deadlock if anyone tries to start a VM, while autodestroy is taking place. The fix is to do autodestroy in 2 phases. First obtain all the callbacks and remove them from the list under the close callback lock. Then invoke each callback from outside the close callback lock. ACK (with the virnetlink hunk removed, of course). Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] network: Bridge - use IPv4 and IPv6 active DHCP stanza pointers
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
Rather than iterate through virNetworkIPDef arrays multiple times
use the new virNetworkDef ipv4_dhcp and ipv6_dhcp active stanza
pointers.
Signed-off-by: TJ li...@iam.tj
---
src/network/bridge_driver.c | 63
+++--
1 file changed, 15 insertions(+), 48 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0932cf8..8410c93 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -810,24 +810,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
}
}
-/* Find the first dhcp for both IPv4 and IPv6 */
-for (ii = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false;
Your refactoring has eliminated the initialization of ipv6SLAAC to false.
- (ipdef = virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, ii));
- ii++) {
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET)) {
-if (ipdef-nranges || ipdef-nhosts) {
-if (ipv4def) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(For IPv4, multiple DHCP definitions
- cannot be specified.));
-goto cleanup;
-} else {
-ipv4def = ipdef;
-}
-}
-}
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET6)) {
-if (ipdef-nranges || ipdef-nhosts) {
I'm starting to warm to the idea of a convenience pointer, but still
dislike the fact that it could get out of sync due to careless
programming. How about a convenience *function* instead. It would be
just as inefficient, but the code would look cleaner (and the
inefficiency isn't a big problem anyway, since it's very rarely done,
and takes a miniscule amount of time anyway).
+ipv4def = ipv6def = NULL;
+ipdef = network-def-ipv4_dhcp;
+if (ipdef (ipdef-nranges || ipdef-nhosts))
+ipv4def = ipdef;
+
+ipdef = network-def-ipv6_dhcp;
+if (ipdef) {
+if (ipdef-nranges || ipdef-nhosts) {
+ipv6def = ipdef;
+
if (!DNSMASQ_DHCPv6_SUPPORT(caps)) {
unsigned long version = dnsmasqCapsGetVersion(caps);
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -842,18 +834,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
DNSMASQ_DHCPv6_MINOR_REQD);
goto cleanup;
}
-if (ipv6def) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(For IPv6, multiple DHCP definitions
- cannot be specified.));
-goto cleanup;
-} else {
-ipv6def = ipdef;
-}
-} else {
-ipv6SLAAC = true;
-}
-}
+ }
+} else {
+ipv6SLAAC = true;
}
if (ipv6def ipv6SLAAC) {
@@ -1160,25 +1143,9 @@ networkRefreshDhcpDaemon(struct network_driver *driver,
if (!(dctx = dnsmasqContextNew(network-def-name, DNSMASQ_STATE_DIR)))
goto cleanup;
-/* Look for first IPv4 address that has dhcp defined.
- * We only support dhcp-host config on one IPv4 subnetwork
- * and on one IPv6 subnetwork.
- */
-ipv4def = NULL;
-for (ii = 0;
- (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET, ii));
- ii++) {
-if (!ipv4def (ipdef-nranges || ipdef-nhosts))
-ipv4def = ipdef;
-}
+ipv4def = network-def-ipv4_dhcp;
-ipv6def = NULL;
-for (ii = 0;
- (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, ii));
- ii++) {
-if (!ipv6def (ipdef-nranges || ipdef-nhosts))
-ipv6def = ipdef;
-}
+ipv6def = network-def-ipv6_dhcp;
if (ipv4def (networkBuildDnsmasqDhcpHostsList(dctx, ipv4def) 0))
goto cleanup;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] network: Bridge - Add support for a DHCP Relay Agent
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
A DHCP relay daemon will be started that will forward all DHCP/BOOTP
requests on the bridge network via the first declared forward
interface.
Okay, I think we've got our first candidate for something we might want
to configure. Picking the forward interface would probably be correct
95% of the time, but it seems reasonable someone might want to forward
it somewhere else. Also there's the fact that most people don't define
any forward interface for their networks, just leaving it up to the
host's IP routing to determine the appropriate egress for every packet.
(I don't really like what's done with the forward interface anyway - it
doesn't do anything about routing to force traffic egress via that
particular interface, just rejects any traffic that would have gone out
a different interface).
The relay is broadcast rather than routed so no IP address
is needed on the bridge.
The XML relay element's relay property of the active DHCP stanza
defaults to 'no'. Set it to 'yes' to enable the relay:
ip ...
dhcp relay='yes'/
/ip
The relay will not be started if the enable property is 'no':
ip ...
dhcp enable='no' relay='yes'/
I don't see the utility of that - it's the same as simply omitting the
dhcp element altogether.
/ip
Signed-off-by: TJ li...@iam.tj
---
src/network/bridge_driver.c | 146
1 file changed, 146 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8410c93..c02d3de 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -587,6 +587,145 @@ cleanup:
* which is later saved into a file
*/
+static virNetworkIpDefPtr
+networkGetActiveDhcp(virNetworkObjPtr network)
+{
+virNetworkIpDefPtr dhcp = NULL;
+
+if (network-def network-def-ipv4_dhcp)
+dhcp = network-def-ipv4_dhcp;
+
+if (!dhcp
+network-def network-def-ipv6_dhcp)
+dhcp = network-def-ipv6_dhcp;
+
+return dhcp;
+}
+
+static int
+networkBuildDhcpRelayArgv(virNetworkObjPtr network,
+const char *pidfile,
+virCommandPtr cmd)
+{
+int ret = -1;
+
+/* PID file */
+virCommandAddArgList(cmd, -r, pidfile, NULL);
+
+/* Listen for DHCP requests on the bridge interface */
+virCommandAddArgList(cmd, -i, network-def-bridge, NULL);
+
+/* Use the first forwarding device to broadcast to the upstream DHCP
server */
+if (network-def-forward.nifs 0) {
+virCommandAddArgList(cmd, -b,
network-def-forward.ifs[0].device.dev, NULL);
+ ret = 0;
+} else
+ virReportSystemError(VIR_ERR_INVALID_INTERFACE,
+ _(DHCP relay requires at least one network %s\n),
+ forward ... dev='eth?'/ or interface dev='eth?'/);
+
+return ret;
+}
+
+static int
+networkBuildDhcpRelayCommandLine(virNetworkObjPtr network, virCommandPtr
*cmdout,
+ char *pidfile)
+{
+virCommandPtr cmd = NULL;
+int ret = -1;
+
+cmd = virCommandNew(DHCPRELAY);
+if (networkBuildDhcpRelayArgv(network, pidfile, cmd) 0) {
+goto cleanup;
+}
+
+if (cmdout)
+*cmdout = cmd;
+ret = 0;
+cleanup:
+if (ret 0)
+virCommandFree(cmd);
+return ret;
+}
+
+static int
+networkStartDhcpRelayDaemon(struct network_driver *driver ATTRIBUTE_UNUSED,
+ virNetworkObjPtr network)
+{
+virCommandPtr cmd = NULL;
+virNetworkIpDefPtr ipdef = NULL;
+char *pidfile = NULL;
+char *tmp = NULL;
+int pid_len;
+int ret = 0;
+const char *dhcprelay = dhcprelay_;
+
+ipdef = networkGetActiveDhcp(network);
+/* Prepare for DHCP relay agent */
+if (ipdef ipdef-dhcp_enabled ipdef-dhcp_relay) {
+ ret = -1;
+
+if (virFileMakePath(NETWORK_PID_DIR) 0) {
+virReportSystemError(errno,
+ _(cannot create directory %s),
+ NETWORK_PID_DIR);
+goto cleanup;
+}
+
+pid_len = strlen(dhcprelay) + strlen(network-def-name);
+if ( VIR_ALLOC_N(tmp, pid_len+1) = 0) {
+ tmp = strcpy(tmp, dhcprelay);
+ tmp = strncat(tmp, network-def-name, pid_len);
+ if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, tmp))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+} else {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ret = networkBuildDhcpRelayCommandLine(network, cmd, pidfile);
+if (ret 0)
+ goto cleanup;
+
+ret = virCommandRun(cmd, NULL);
+if (ret 0)
+ goto cleanup;
+
+ret = virPidFileRead(NETWORK_PID_DIR, pidfile,
network-dhcprelayPid);
+if (ret 0)
+
Re: [libvirt] [PATCH] Revert hack for autodestroy in qemuProcessStop
On Thu, Feb 28, 2013 at 16:44:38 +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com This reverts the hack done in commit 568a6cda277f04ab9baaeb97490e548b7b608aa6 Author: Jiri Denemark jdene...@redhat.com Date: Fri Feb 15 15:11:47 2013 +0100 qemu: Avoid deadlock in autodestroy since we now have a fix which avoids the deadlock scenario entirely ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] network: Bridge - Add support for a DHCP Relay Agent
On 02/28/2013 01:32 AM, TJ wrote: That then leaves the issue of different DHCP relay agents requiring different command-line parameters. I was looking at the discussion about the option element and wondering if that could have a dual use here, especially in the name= variety, e.g: ip ... dhcp relay='yes' agent='some-agent-we-dont-know' option name=bridge value data=-b/ /option option name=interface value data=-i/ /option option name=pidfile value data=-r/ /option /dhcp /ip No, definitely not. the option element is for options in the dhcp request/response packets, as defined in RFC2132 (and others), other than coincidentally (because the dhcp options can be specified on the dnsmasq commandline) it has nothing to do with commandline parameters of any particular program. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/10] network: Bridge - don't offer dnsmasq DHCP services when DHCP relay is enabled
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
When dnsmasq's DNS services are required but the network is configured
to use a DHCP relay agent (other than dnsmasq's proxy services) the
configuration generated for dnsmasq should not enable DHCP services.
Signed-off-by: TJ li...@iam.tj
---
src/network/bridge_driver.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c02d3de..a4cd727 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -951,11 +951,12 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
ipv4def = ipv6def = NULL;
ipdef = network-def-ipv4_dhcp;
-if (ipdef (ipdef-nranges || ipdef-nhosts))
+if (ipdef ipdef-dhcp_enabled !ipdef-dhcp_relay
+(ipdef-nranges || ipdef-nhosts))
ipv4def = ipdef;
ipdef = network-def-ipv6_dhcp;
-if (ipdef) {
+if (ipdef ipdef-dhcp_enabled !ipdef-dhcp_relay) {
if (ipdef-nranges || ipdef-nhosts) {
ipv6def = ipdef;
@@ -1266,8 +1267,8 @@ static int
networkRefreshDhcpDaemon(struct network_driver *driver,
virNetworkObjPtr network)
{
-int ret = -1, ii;
You apparently removed the usage of ii in an earlier patch, but are
removing the definition of ii here. That would have cause a build
failure after applying the earlier patch. AS Eric mentioned, you must be
able to successfully complete an autogen.sh make check make
syntax-check after applying each patch in the series in sequence.
Otherwise, git bisect becomes unusable. (This is another argument for
integrating more into each patch).
-virNetworkIpDefPtr ipdef, ipv4def, ipv6def;
+int ret = -1;
+virNetworkIpDefPtr ipv4def, ipv6def;
dnsmasqContext *dctx = NULL;
/* if no IP addresses specified, nothing to do */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/10] configure: Add DHCPRELAY to the set of external program definitions
On 02/27/2013 09:57 PM, TJ wrote: From: TJ li...@iam.tj This variable should name the path to the system's DHCP relay daemon. At this time the expected daemon is dhcp-helper, a DHCP relay agent from Simon Kelly, author of dnsmasq. The supporting code, however, has been designed to work with any suitable DHCP relay agent. Later patches could allow configuration of the agent's command-line arguments. Currently the chosen agent must support: -b bridge_interface (the virtual network to listen on) -i physical_interface (the interface to broadcast on) -r /path/to/pid.file (the PID file of the daemon) Signed-off-by: TJ li...@iam.tj --- configure.ac | 4 1 file changed, 4 insertions(+) This should have come much earlier in the series, probably as just another part of a single larger patch. diff --git a/configure.ac b/configure.ac index e3a749a..b62170e 100644 --- a/configure.ac +++ b/configure.ac @@ -295,6 +295,8 @@ dnl External programs that we can use if they are available. dnl We will hard-code paths to these programs unless we cannot dnl detect them, in which case we'll search for the program dnl along the $PATH at runtime and fail if it's not there. +AC_PATH_PROG([DHCPRELAY], [dhcp-helper], [dhcp-helper], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([RADVD], [radvd], [radvd], @@ -314,6 +316,8 @@ AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], AC_PATH_PROG([SCRUB], [scrub], [scrub], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_DEFINE_UNQUOTED([DHCPRELAY], [$DHCPRELAY], + [Location or name of the dhcp relay-agent program]) AC_DEFINE_UNQUOTED([DNSMASQ],[$DNSMASQ], [Location or name of the dnsmasq program]) AC_DEFINE_UNQUOTED([RADVD],[$RADVD], -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VG backed storage pool start behavior.
This was the change that moved the lvm tools: https://projects.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/lvm2id=a54404dbc41bdf5a9dc60ac754311297938c0a4d I wouldn't know if libvirt needed to be changed to support finding them when configuring, but that sounds like it should happen if it isn't. On Wed, Feb 20, 2013 at 12:07 PM, Eric Blake ebl...@redhat.com wrote: On 02/20/2013 09:40 AM, Luke Varnadore wrote: It was a distro specific issue. Arch Linux has the lvm tools in /usr/sbin instead of /sbin. I made links for lvs, vgs, and vgchange and the issue cleared up. Reported a bug for the package. Is it also a case of upstream libvirt having a bug for not checking at configure time for the preferred location of these tools, but just using a hard-coded location for them? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Luke W Varnadore 727.697.7865 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt does not logout of iscsi targets, causing system hang on shutdown
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi guys, There's a quite old bug entry here: https://bugzilla.redhat.com/show_bug.cgi?id=700010 I just stumbled over that very issue on F18. Doing a little bit debugging of the shutdown sequence, it turns out that - at least on my F18 installation - libvirtd is shutdown *after* iscsid, which makes it impossible for libvirt to perform the logout of the iscsi session properly. My local fix (diff) is attached. It simply adds another startup dependancy on iscsid.service which in turn delays iscsid shutdown until after libvirtd has stopped. Having that applied, the system shuts down properly again. I was asked to post that here for discussion... So please consider this trivial change for the next update. Cheers -Fritz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEvwhoACgkQboM4mAMyprB9agCgpIkGbSBKpfB4e0q6aqSMVGam rMoAnAtBpp3nFbleWLxJTbCt9EKO9Yx3 =2hHf -END PGP SIGNATURE- --- /usr/lib/systemd/system/libvirtd.service 2013-01-28 21:35:32.0 +0100 +++ /etc/systemd/system/libvirtd.service 2013-02-28 06:34:41.341905146 +0100 @@ -7,6 +7,7 @@ Description=Virtualization daemon Before=libvirt-guests.service After=network.target +After=iscsid.service [Service] EnvironmentFile=-/etc/sysconfig/libvirtd libvirtd.service-iscsi-shutdown.diff.sig Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Make sure qemuProcessStart is run within a job
On 02/28/2013 06:22 AM, Jiri Denemark wrote: qemuProcessStart expects to be run with a job already set and every caller except for qemuMigrationPrepareAny use it correctly. This bug can be observed in libvirtd logs during incoming migration as warning : qemuDomainObjEnterMonitorInternal:979 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous --- src/qemu/qemu_domain.c| 35 --- src/qemu/qemu_domain.h| 4 src/qemu/qemu_migration.c | 11 +++ 3 files changed, 35 insertions(+), 15 deletions(-) ACK. Qualifies as a bug fix for 1.0.3. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] python: global variable and debugging improvement for generator.py
On 02/28/2013 03:03 AM, Guannan Ren wrote: * Put import clause in front of global variables * Sink __name__ == __main__ to the bottom of this script and support import generator * Remove quiet and debug global variables and use stubs_buiding_debug and xml_parsing_debug variable instead s/buiding/building/ --- python/generator.py | 105 +++- 1 file changed, 54 insertions(+), 51 deletions(-) -if not quiet: +if stubs_buiding_debug: and here, as well + +if buildStubs(libvirt, stubs_buiding_debug, xml_parsing_debug) 0: +sys.exit(1) +if buildStubs(libvirt-lxc, stubs_buiding_debug, xml_parsing_debug) 0: +sys.exit(1) +if buildStubs(libvirt-qemu, stubs_buiding_debug, xml_parsing_debug) 0: +sys.exit(1) And here. My python is too weak to say whether this makes sense, especially not whether it makes sense for 1.0.3 or whether it should wait until after the release. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] python: fix typoes and repeated global vars references
On 02/28/2013 03:03 AM, Guannan Ren wrote: --- python/generator.py | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) ACK. I don't have to understand python to see that this one makes sense :) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] python: optimize SAX xml parsing event handler
On 02/28/2013 03:03 AM, Guannan Ren wrote: close(), getmethodname(), cdata() are not standard methods from parent class ContentHandler and not being used anywhere in codes, so remove them. In docParser, actually, we are overloading three parent methods startElement(), endElement() and characters(), so I rename back three of these method names for example from start() to startElement() which makes it simpler to read. make other improvements in codes. Someone else will have to review this. Sorry. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] python: fix fd leak in generator.py
On 02/28/2013 03:03 AM, Guannan Ren wrote: --- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) At first, I was worried that this is a leak in the generated code. But it looks like it is only a leak in the generator, and that the generated code is safe. So it's not an essential bug fix, and more of a judgment call on whether to include in 1.0.3. At any rate, the fix appears obvious enough that I'm fine giving: ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [trivial PATCH 1/1] Fix a message typo
As pointed out in
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1034661
The sentence
The function of PCI device addresses must less than 8
does not quite make sense. Update that to read
The function of PCI device addresses must be less than 8
Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f8f3ade..201fac1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1743,7 +1743,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
if (info-addr.pci.function 7) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(The function of PCI device addresses must
- less than 8));
+ be less than 8));
return -1;
}
} else {
--
1.8.1.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [trivial PATCH 1/1] Fix a message typo
On 02/28/2013 03:08 PM, Serge Hallyn wrote: As pointed out in https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1034661 The sentence The function of PCI device addresses must less than 8 does not quite make sense. Update that to read The function of PCI device addresses must be less than 8 Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK and pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt does not logout of iscsi targets, causing system hang on shutdown
On 02/28/2013 01:46 PM, Fritz Elfert wrote: Hi guys, There's a quite old bug entry here: https://bugzilla.redhat.com/show_bug.cgi?id=700010 I just stumbled over that very issue on F18. Doing a little bit debugging of the shutdown sequence, it turns out that - at least on my F18 installation - libvirtd is shutdown *after* iscsid, which makes it impossible for libvirt to perform the logout of the iscsi session properly. My local fix (diff) is attached. It simply adds another startup dependancy on iscsid.service which in turn delays iscsid shutdown until after libvirtd has stopped. Having that applied, the system shuts down properly again. I was asked to post that here for discussion... So please consider this trivial change for the next update. +++ /etc/systemd/system/libvirtd.service 2013-02-28 06:34:41.341905146 +0100 @@ -7,6 +7,7 @@ Description=Virtualization daemon Before=libvirt-guests.service After=network.target +After=iscsid.service Makes sense to me. My biggest doubt was whether this would make a system that previously did not use iscsid now suddenly start to require a service. But if I understood 'man systemd.unit' correctly, adding an 'After=' without a 'Wants=' or 'Requires=' is valid, and merely means that _if_ iscsid is enabled, then systemd will enforce the ordering, but that libvirtd will manage just fine even when iscsid is disabled. I'm not much of a systemd expert, so I'll wait until morning before pushing, to give anyone else a chance to disagree with my analysis or provide a more kosher fix. Otherwise, you have my ACK, and I think this deserves to be in 1.0.3. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
On 02/27/2013 04:51 PM, Jim Fehlig wrote: With the apparmor security driver enabled, qemu instances fail to start # grep ^security_driver /etc/libvirt/qemu.conf security_driver = apparmor # virsh start test-kvm error: Failed to start domain test-kvm error: internal error security label already defined for VM The model field of virSecurityLabelDef object is always populated by virDomainDefGetSecurityLabelDef(), so remove the check for a NULL model when verifying if a label is already defined for the instance. Checking for a NULL model and populating it later in AppArmorGenSecurityLabel() has been left in the code to be consistent with virSecuritySELinuxGenSecurityLabel(). --- src/security/security_apparmor.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK; and safe for 1.0.3. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt does not logout of iscsi targets, causing system hang on shutdown
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/28/2013 11:38 PM, Eric Blake wrote: On 02/28/2013 01:46 PM, Fritz Elfert wrote: Hi guys, There's a quite old bug entry here: https://bugzilla.redhat.com/show_bug.cgi?id=700010 I just stumbled over that very issue on F18. Doing a little bit debugging of the shutdown sequence, it turns out that - at least on my F18 installation - libvirtd is shutdown *after* iscsid, which makes it impossible for libvirt to perform the logout of the iscsi session properly. My local fix (diff) is attached. It simply adds another startup dependancy on iscsid.service which in turn delays iscsid shutdown until after libvirtd has stopped. Having that applied, the system shuts down properly again. I was asked to post that here for discussion... So please consider this trivial change for the next update. +++ /etc/systemd/system/libvirtd.service 2013-02-28 06:34:41.341905146 +0100 @@ -7,6 +7,7 @@ Description=Virtualization daemon Before=libvirt-guests.service After=network.target +After=iscsid.service Makes sense to me. My biggest doubt was whether this would make a system that previously did not use iscsid now suddenly start to require a service. But if I understood 'man systemd.unit' correctly, adding an 'After=' without a 'Wants=' or 'Requires=' is valid, and merely means that _if_ iscsid is enabled, then systemd will enforce the ordering, but that libvirtd will manage just fine even when iscsid is disabled. I'm not much of a systemd expert, so I'll wait until morning before pushing, to give anyone else a chance to disagree with my analysis or provide a more kosher fix. Otherwise, you have my ACK, and I think this deserves to be in 1.0.3. I wasn't shure either, so I just tried it out adding another After=idontexist.service. System rebooted and started libvirtd without complaining about the missing entry. - -Fritz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEv4a4ACgkQboM4mAMyprCECQCdGdP6TN1Gp7iBCr9RroHPOMfo yagAoKgY0av/W4IIJopF7H9qdOz3bLqS =WyKi -END PGP SIGNATURE- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] conf: DHCP - add state for DHCP Relay and On/Off switch
On 28/02/13 19:45, Laine Stump wrote: [...sni[...] I think Eric mentioned elsewhere that your patches are *extremely* small. At least both the data structure, parser, and formatter changes should go into a single patch. For that matter, this functionality is small enough that you could put the struct change, parser change, implementation of the feature in bridge_driver, makefile and configure changes, AND the documentation addition into a single patch. Yes. Remember this an RFC so to some extend I'm making it easier to document (in commit messages as well as logical separation of the patches), the way I thought about the problem for myself as much as anyone else. When I'm developing a feature for some package that is foreign to me I prefer to break each focus of change into the actual hacking style I follow, which allows me to go back and change patches that have a single focus without changing related code (which may also be in a related branch where I'm testing alternate functionality) using, f.e: git rebase -i HEAD^^ s/^(pick) (49fe52|3f98634)/edit \1/ while $SOME_FILE; do vim $SOME_FILE git commit --amend git rebase --continue done Once the patch-set works and has positive feedback I can squash cherry-picked commits into a new branch for the final: git request-pull branch_x -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] network: Bridge - use IPv4 and IPv6 active DHCP stanza pointers
On 28/02/13 19:51, Laine Stump wrote:
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
Rather than iterate through virNetworkIPDef arrays multiple times
use the new virNetworkDef ipv4_dhcp and ipv6_dhcp active stanza
pointers.
Signed-off-by: TJ li...@iam.tj
---
src/network/bridge_driver.c | 63
+++--
1 file changed, 15 insertions(+), 48 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0932cf8..8410c93 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -810,24 +810,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
}
}
-/* Find the first dhcp for both IPv4 and IPv6 */
-for (ii = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false;
Your refactoring has eliminated the initialization of ipv6SLAAC to false.
- (ipdef = virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, ii));
- ii++) {
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET)) {
-if (ipdef-nranges || ipdef-nhosts) {
-if (ipv4def) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(For IPv4, multiple DHCP definitions
- cannot be specified.));
-goto cleanup;
-} else {
-ipv4def = ipdef;
-}
-}
-}
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET6)) {
-if (ipdef-nranges || ipdef-nhosts) {
I'm starting to warm to the idea of a convenience pointer, but still
dislike the fact that it could get out of sync due to careless
programming. How about a convenience *function* instead. It would be
just as inefficient, but the code would look cleaner (and the
inefficiency isn't a big problem anyway, since it's very rarely done,
and takes a miniscule amount of time anyway).
Actually that is where I started, see patch 06 network: Bridge - Add support
for DHCP relay agent and
static virNetworkGetActiveDhcp().
Later I wanted to eliminate the iterations over the ip structures that I had
moved into this function since it was called from several places,
and ended up replacing it with code in the XML parser that sets the
-ipv{4,6}_dhcp pointers once.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] network: Bridge - Add support for a DHCP Relay Agent
On 28/02/13 19:56, Laine Stump wrote: On 02/27/2013 09:57 PM, TJ wrote: From: TJ li...@iam.tj A DHCP relay daemon will be started that will forward all DHCP/BOOTP requests on the bridge network via the first declared forward interface. Okay, I think we've got our first candidate for something we might want to configure. Picking the forward interface would probably be correct 95% of the time, but it seems reasonable someone might want to forward it somewhere else. Also there's the fact that most people don't define any forward interface for their networks, just leaving it up to the host's IP routing to determine the appropriate egress for every packet. (I don't really like what's done with the forward interface anyway - it doesn't do anything about routing to force traffic egress via that particular interface, just rejects any traffic that would have gone out a different interface). Are you thinking in terms of dhcp relay='yes' forward='ethX'/ ? Should it be always explicit or use the current 'pick a sensible default' but allow this property to over-ride it? The relay is broadcast rather than routed so no IP address is needed on the bridge. The XML relay element's relay property of the active DHCP stanza defaults to 'no'. Set it to 'yes' to enable the relay: ip ... dhcp relay='yes'/ /ip The relay will not be started if the enable property is 'no': ip ... dhcp enable='no' relay='yes'/ I don't see the utility of that - it's the same as simply omitting the dhcp element altogether. The utility of the enabled='(yes|no)' properties is one of convenience for the sysadmin, and was something I felt in need of whilst testing many scenarios of dnsmaq *before* I embarked on this DHCP relay functionality, and again when I was testing dhcp-helper. Without needing to delete important sub-elements of dhcp (and lose through forgetting them) from an active network XML definition I could selectively switch a dhcp block on/off simply by adding the enabled property to the element. The alternative was continually having to switch amongst several network XML files all of which were permutations of the same settings in order that disabling a stanza in the primary XML didn't require needing to delete, for example, all the host and range sub-elements of dhcp. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] network: Bridge - use IPv4 and IPv6 active DHCP stanza pointers
On 28/02/13 19:51, Laine Stump wrote:
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ li...@iam.tj
Rather than iterate through virNetworkIPDef arrays multiple times
use the new virNetworkDef ipv4_dhcp and ipv6_dhcp active stanza
pointers.
Signed-off-by: TJ li...@iam.tj
---
src/network/bridge_driver.c | 63
+++--
1 file changed, 15 insertions(+), 48 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0932cf8..8410c93 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -810,24 +810,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
}
}
-/* Find the first dhcp for both IPv4 and IPv6 */
-for (ii = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false;
Your refactoring has eliminated the initialization of ipv6SLAAC to false.
- (ipdef = virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, ii));
- ii++) {
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET)) {
-if (ipdef-nranges || ipdef-nhosts) {
-if (ipv4def) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(For IPv4, multiple DHCP definitions
- cannot be specified.));
-goto cleanup;
-} else {
-ipv4def = ipdef;
-}
-}
-}
-if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET6)) {
-if (ipdef-nranges || ipdef-nhosts) {
I'm starting to warm to the idea of a convenience pointer, but still
dislike the fact that it could get out of sync due to careless
programming. How about a convenience *function* instead. It would be
just as inefficient, but the code would look cleaner (and the
inefficiency isn't a big problem anyway, since it's very rarely done,
and takes a miniscule amount of time anyway).
Actually that is where I started, see patch 06 network: Bridge - Add support
for DHCP relay agent and
static virNetworkGetActiveDhcp().
Later I wanted to eliminate the iterations over the ip structures that I had
moved into this function since it was called from several places,
and ended up replacing it with code in the XML parser that sets the
-ipv{4,6}_dhcp pointers once.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
Eric Blake wrote: On 02/27/2013 04:51 PM, Jim Fehlig wrote: With the apparmor security driver enabled, qemu instances fail to start # grep ^security_driver /etc/libvirt/qemu.conf security_driver = apparmor # virsh start test-kvm error: Failed to start domain test-kvm error: internal error security label already defined for VM The model field of virSecurityLabelDef object is always populated by virDomainDefGetSecurityLabelDef(), so remove the check for a NULL model when verifying if a label is already defined for the instance. Checking for a NULL model and populating it later in AppArmorGenSecurityLabel() has been left in the code to be consistent with virSecuritySELinuxGenSecurityLabel(). --- src/security/security_apparmor.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK; and safe for 1.0.3. Thanks, pushed now. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/4] add pci-bridge support
Hi, any other comments? 在 2013-02-25一的 09:38 +0800，li guang写道： ping ... 在 2013-02-19二的 10:25 +0800，liguang写道： Now, it's impossible to arrange devices into multi-pci-bus, for example: sound model='ac97' address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /sound video model type='cirrus' vram='9216' heads='1'/ address type='pci' domain='0x' bus='0x1' slot='0x02' function='0x0'/ /video libvirt will complain about bus != 0, fortunately, qemu supports pci-to-pci bridge, if we want to use multi-pci-bus, we can define 2 pci bridge controllers, then attach 1 to the other as a subordinate pci-bus, so, 2 pci-buses appear. for example: controller type='pci-bridge' index='0'/ controller type='pci-bridge' index='1' address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ /controller sound model='ac97' address type='pci' domain='0x' bus='0x01' slot='0x02' function='0x0'/ /sound video model type='cirrus' vram='9216' heads='1'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /video src/conf/domain_conf.c| 98 - src/conf/domain_conf.h|1 + docs/schemas/domaincommon.rng |1 + src/qemu/qemu_capabilities.c |4 + src/qemu/qemu_capabilities.h |1 + src/qemu/qemu_command.c | 43 - 6 files changed, 126 insertions(+), 22 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix starting qemu instances when apparmor driver is enabled
On 03/01/2013 08:37 AM, Jim Fehlig wrote: Eric Blake wrote: On 02/27/2013 04:51 PM, Jim Fehlig wrote: With the apparmor security driver enabled, qemu instances fail to start # grep ^security_driver /etc/libvirt/qemu.conf security_driver = apparmor # virsh start test-kvm error: Failed to start domain test-kvm error: internal error security label already defined for VM The model field of virSecurityLabelDef object is always populated by virDomainDefGetSecurityLabelDef(), so remove the check for a NULL model when verifying if a label is already defined for the instance. Checking for a NULL model and populating it later in AppArmorGenSecurityLabel() has been left in the code to be consistent with virSecuritySELinuxGenSecurityLabel(). --- src/security/security_apparmor.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK; and safe for 1.0.3. Thanks, pushed now. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Hi Jim In selinux, libvirt added a label for tapfd. Do you think this patch makes sense for apparmor? https://www.redhat.com/archives/libvir-list/2012-October/msg01461.html Gunannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Add cpuset cgroup support for LXC
This patchset intend to add cpuset cgroup support for LXC. in order to don't create too many redundant codes, this patchset also rename some functions and structure. Gao feng (4): rename qemuGetNumadAdvice to virGetNumadAdvice LXC: allow uses advisory nodeset from querying numad remove the redundant codes LXC: add cpuset cgroup support for lxc po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/domain_conf.h | 23 +-- src/libvirt_private.syms | 4 ++ src/lxc/lxc_cgroup.c | 57 +++- src/lxc/lxc_cgroup.h | 2 +- src/lxc/lxc_controller.c | 156 +++--- src/qemu/qemu_process.c | 154 + src/util/virnuma.c | 174 +++ src/util/virnuma.h | 52 ++ 10 files changed, 348 insertions(+), 276 deletions(-) create mode 100644 src/util/virnuma.c create mode 100644 src/util/virnuma.h -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] rename qemuGetNumadAdvice to virGetNumadAdvice
qemuGetNumadAdvice will be used by LXC driver,rename
it to virGetNumaAdvice and move it to virnuma.c
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 3 +++
src/qemu/qemu_process.c | 33 ++
src/util/virnuma.c | 60
src/util/virnuma.h | 28 ++
6 files changed, 95 insertions(+), 31 deletions(-)
create mode 100644 src/util/virnuma.c
create mode 100644 src/util/virnuma.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index bd2c02e..ee8ff86 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -164,6 +164,7 @@ src/util/virnetdevtap.c
src/util/virnetdevvportprofile.c
src/util/virnetlink.c
src/util/virnodesuspend.c
+src/util/virnuma.c
src/util/virobject.c
src/util/virpci.c
src/util/virpidfile.c
diff --git a/src/Makefile.am b/src/Makefile.am
index c1659a4..21eb84a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -103,6 +103,7 @@ UTIL_SOURCES =
\
util/virnetdevvportprofile.h util/virnetdevvportprofile.c \
util/virnetlink.c util/virnetlink.h \
util/virnodesuspend.c util/virnodesuspend.h \
+ util/virnuma.c util/virnuma.h \
util/virobject.c util/virobject.h \
util/virpci.c util/virpci.h \
util/virpidfile.c util/virpidfile.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ed46479..6aee6fa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1565,6 +1565,9 @@ nodeSuspendForDuration;
virNodeSuspendGetTargetMask;
+# util/virnuma.h
+virGetNumadAdvice;
+
# util/virobject.h
virClassForObject;
virClassForObjectLockable;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index db95d6e..20d41e3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -70,6 +70,7 @@
#include virnetdevtap.h
#include virbitmap.h
#include viratomic.h
+#include virnuma.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -1981,36 +1982,6 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm,
}
#endif
-#if HAVE_NUMAD
-static char *
-qemuGetNumadAdvice(virDomainDefPtr def)
-{
-virCommandPtr cmd = NULL;
-char *output = NULL;
-
-cmd = virCommandNewArgList(NUMAD, -w, NULL);
-virCommandAddArgFormat(cmd, %d:%llu, def-vcpus,
- VIR_DIV_UP(def-mem.cur_balloon, 1024));
-
-virCommandSetOutputBuffer(cmd, output);
-
-if (virCommandRun(cmd, NULL) 0)
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Failed to query numad for the
- advisory nodeset));
-
-virCommandFree(cmd);
-return output;
-}
-#else
-static char *
-qemuGetNumadAdvice(virDomainDefPtr def ATTRIBUTE_UNUSED)
-{
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(numad is not available on this host));
-return NULL;
-}
-#endif
/* Helper to prepare cpumap for affinity setting, convert
* NUMA nodeset into cpuset if @nodemask is not NULL, otherwise
@@ -3721,7 +3692,7 @@ int qemuProcessStart(virConnectPtr conn,
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) ||
(vm-def-numatune.memory.placement_mode ==
VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)) {
-nodeset = qemuGetNumadAdvice(vm-def);
+nodeset = virGetNumadAdvice(vm-def-vcpus, vm-def-mem.cur_balloon);
if (!nodeset)
goto cleanup;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
new file mode 100644
index 000..37931fe
--- /dev/null
+++ b/src/util/virnuma.c
@@ -0,0 +1,60 @@
+/*
+ * virnuma.c: helper APIS for managing numa
+ *
+ * Copyright (C) 2011-2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#include config.h
+
+#include virnuma.h
+#include vircommand.h
+#include virerror.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+#if HAVE_NUMAD
+char *virGetNumadAdvice(unsigned short vcpus,
+unsigned long long balloon)
+{
+virCommandPtr cmd = NULL;
+char *output = NULL;
+
+cmd = virCommandNewArgList(NUMAD, -w, NULL);
+virCommandAddArgFormat(cmd, %d:%llu,
[libvirt] [PATCH 2/4] LXC: allow uses advisory nodeset from querying numad
Allow lxc using the advisory nodeset from querying numad,
this means if user doesn't specify the numa nodes that
the lxc domain should assign to, libvirt will automatically
bind the lxc domain to the advisory nodeset which queried from
numad.
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
src/lxc/lxc_controller.c | 84 ++--
1 file changed, 74 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 15aa334..b6c1fe8 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -69,6 +69,7 @@
#include nodeinfo.h
#include virrandom.h
#include virprocess.h
+#include virnuma.h
#include rpc/virnetserver.h
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -409,7 +410,8 @@ cleanup:
}
#if WITH_NUMACTL
-static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl)
+static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl,
+ virBitmapPtr nodemask)
{
nodemask_t mask;
int mode = -1;
@@ -418,9 +420,22 @@ static int
virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl)
int i = 0;
int maxnode = 0;
bool warned = false;
-
-if (!ctrl-def-numatune.memory.nodemask)
+virDomainNumatuneDef numatune = ctrl-def-numatune;
+virBitmapPtr tmp_nodemask = NULL;
+
+if (numatune.memory.placement_mode ==
+VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
+if (!numatune.memory.nodemask)
+return 0;
+VIR_DEBUG(Set NUMA memory policy with specified nodeset);
+tmp_nodemask = numatune.memory.nodemask;
+} else if (numatune.memory.placement_mode ==
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
+VIR_DEBUG(Set NUMA memory policy with advisory nodeset from numad);
+tmp_nodemask = nodemask;
+} else {
return 0;
+}
VIR_DEBUG(Setting NUMA memory policy);
@@ -435,7 +450,7 @@ static int
virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl)
/* Convert nodemask to NUMA bitmask. */
nodemask_zero(mask);
i = -1;
-while ((i = virBitmapNextSetBit(ctrl-def-numatune.memory.nodemask, i))
= 0) {
+while ((i = virBitmapNextSetBit(tmp_nodemask, i)) = 0) {
if (i NUMA_NUM_NODES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Host cannot support NUMA node %d), i);
@@ -488,7 +503,8 @@ cleanup:
return ret;
}
#else
-static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl)
+static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl,
+ virBitmapPtr nodemask
ATTRIBUTE_UNUSED)
{
if (ctrl-def-numatune.memory.nodemask) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -549,6 +565,40 @@ static int
virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl)
}
+static int virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl,
+ virBitmapPtr *mask)
+{
+virBitmapPtr nodemask = NULL;
+char *nodeset;
+int ret = -1;
+
+/* Get the advisory nodeset from numad if 'placement' of
+ * either vcpu or numatune is 'auto'.
+ */
+if ((ctrl-def-placement_mode ==
+ VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) ||
+(ctrl-def-numatune.memory.placement_mode ==
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)) {
+nodeset = virGetNumadAdvice(ctrl-def-vcpus,
+ctrl-def-mem.cur_balloon);
+if (!nodeset)
+goto cleanup;
+
+VIR_DEBUG(Nodeset returned from numad: %s, nodeset);
+
+ret = virBitmapParse(nodeset, 0, nodemask, VIR_DOMAIN_CPUMASK_LEN);
+if (ret 0)
+goto cleanup;
+}
+ret = 0;
+*mask = nodemask;
+
+cleanup:
+VIR_FREE(nodeset);
+return ret;
+}
+
+
/**
* virLXCControllerSetupResourceLimits
* @ctrl: the controller state
@@ -560,14 +610,28 @@ static int
virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl)
*/
static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
{
+virBitmapPtr nodemask = NULL;
+int ret;
-if (virLXCControllerSetupCpuAffinity(ctrl) 0)
-return -1;
+ret = virLXCControllerGetNumadAdvice(ctrl, nodemask);
+if (ret 0)
+goto cleanup;
-if (virLXCControllerSetupNUMAPolicy(ctrl) 0)
-return -1;
+ret = virLXCControllerSetupCpuAffinity(ctrl);
+if (ret 0)
+goto cleanup;
+
+ret = virLXCControllerSetupNUMAPolicy(ctrl, nodemask);
+if (ret 0)
+goto cleanup;
-return virLXCCgroupSetup(ctrl-def);
+ret = virLXCCgroupSetup(ctrl-def);
+if (ret 0)
+goto cleanup;
+
+cleanup:
+virBitmapFree(nodemask);
+return ret;
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] LXC: add cpuset cgroup support for lxc
This patch adds cpuset cgroup support for LXC.
also set cpuset cgroup before setting cpu
affinity and numa policy.
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
src/lxc/lxc_cgroup.c | 57 +++-
src/lxc/lxc_cgroup.h | 2 +-
src/lxc/lxc_controller.c | 6 ++---
3 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index a075335..f94b914 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -68,6 +68,58 @@ cleanup:
}
+static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
+ virCgroupPtr cgroup,
+ virBitmapPtr nodemask)
+{
+int rc = 0;
+char *mask = NULL;
+
+if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO
+def-cpumask) {
+mask = virBitmapFormat(def-cpumask);
+if (!mask) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(failed to convert cpumask));
+return -1;
+}
+
+rc = virCgroupSetCpusetCpus(cgroup, mask);
+VIR_FREE(mask);
+if (rc 0) {
+virReportSystemError(-rc, %s,
+ _(Unable to set cpuset.cpus));
+}
+}
+
+if ((def-numatune.memory.nodemask ||
+ (def-numatune.memory.placement_mode ==
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO))
+ def-numatune.memory.mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+if (def-numatune.memory.placement_mode ==
+VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)
+mask = virBitmapFormat(nodemask);
+else
+mask = virBitmapFormat(def-numatune.memory.nodemask);
+
+if (!mask) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(failed to convert memory nodemask));
+return -1;
+}
+
+rc = virCgroupSetCpusetMems(cgroup, mask);
+VIR_FREE(mask);
+if (rc 0) {
+virReportSystemError(-rc, %s,
+ _(Unable to set cpuset.mems));
+}
+}
+
+return rc;
+}
+
+
static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def,
virCgroupPtr cgroup)
{
@@ -472,7 +524,7 @@ cleanup:
}
-int virLXCCgroupSetup(virDomainDefPtr def)
+int virLXCCgroupSetup(virDomainDefPtr def, virBitmapPtr nodemask)
{
virCgroupPtr driver = NULL;
virCgroupPtr cgroup = NULL;
@@ -497,6 +549,9 @@ int virLXCCgroupSetup(virDomainDefPtr def)
if (virLXCCgroupSetupCpuTune(def, cgroup) 0)
goto cleanup;
+if (virLXCCgroupSetupCpusetTune(def, cgroup, nodemask) 0)
+goto cleanup;
+
if (virLXCCgroupSetupBlkioTune(def, cgroup) 0)
goto cleanup;
diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h
index fff554b..29f21d6 100644
--- a/src/lxc/lxc_cgroup.h
+++ b/src/lxc/lxc_cgroup.h
@@ -26,7 +26,7 @@
# include lxc_fuse.h
# include virusb.h
-int virLXCCgroupSetup(virDomainDefPtr def);
+int virLXCCgroupSetup(virDomainDefPtr def, virBitmapPtr nodemask);
int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo);
int
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 3db0a88..75e2fe4 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -505,15 +505,15 @@ static int
virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
if (ret 0)
goto cleanup;
-ret = virLXCControllerSetupCpuAffinity(ctrl);
+ret = virLXCCgroupSetup(ctrl-def, nodemask);
if (ret 0)
goto cleanup;
-ret = virSetupNumaMemoryPolicy(ctrl-def-numatune, nodemask);
+ret = virLXCControllerSetupCpuAffinity(ctrl);
if (ret 0)
goto cleanup;
-ret = virLXCCgroupSetup(ctrl-def);
+ret = virSetupNumaMemoryPolicy(ctrl-def-numatune, nodemask);
if (ret 0)
goto cleanup;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] remove the redundant codes
Intend to reduce the redundant code,use virSetupNumaMemoryPolicy
to replace virLXCControllerSetupNUMAPolicy and
qemuProcessInitNumaMemoryPolicy.
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
src/conf/domain_conf.h | 23 +
src/libvirt_private.syms | 1 +
src/lxc/lxc_controller.c | 114 +---
src/qemu/qemu_process.c | 121 +--
src/util/virnuma.c | 114
src/util/virnuma.h | 24 ++
6 files changed, 143 insertions(+), 254 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5828ae2..2a8dff3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -47,6 +47,7 @@
# include device_conf.h
# include virbitmap.h
# include virstoragefile.h
+# include virnuma.h
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -1589,14 +1590,6 @@ enum virDomainCpuPlacementMode {
VIR_DOMAIN_CPU_PLACEMENT_MODE_LAST
};
-enum virDomainNumatuneMemPlacementMode {
-VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
-VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC,
-VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO,
-
-VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST
-};
-
typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef;
typedef virDomainTimerCatchupDef *virDomainTimerCatchupDefPtr;
struct _virDomainTimerCatchupDef {
@@ -1685,18 +1678,6 @@ virDomainVcpuPinDefPtr
virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
int nvcpupin,
int vcpu);
-typedef struct _virDomainNumatuneDef virDomainNumatuneDef;
-typedef virDomainNumatuneDef *virDomainNumatuneDefPtr;
-struct _virDomainNumatuneDef {
-struct {
-virBitmapPtr nodemask;
-int mode;
-int placement_mode; /* enum virDomainNumatuneMemPlacementMode */
-} memory;
-
-/* Future NUMA tuning related stuff should go here. */
-};
-
typedef struct _virBlkioDeviceWeight virBlkioDeviceWeight;
typedef virBlkioDeviceWeight *virBlkioDeviceWeightPtr;
struct _virBlkioDeviceWeight {
@@ -1784,7 +1765,7 @@ struct _virDomainDef {
virDomainVcpuPinDefPtr emulatorpin;
} cputune;
-virDomainNumatuneDef numatune;
+virNumaTuneParams numatune;
/* These 3 are based on virDomainLifeCycleAction enum flags */
int onReboot;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6aee6fa..56c466a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1567,6 +1567,7 @@ virNodeSuspendGetTargetMask;
# util/virnuma.h
virGetNumadAdvice;
+virSetupNumaMemoryPolicy;
# util/virobject.h
virClassForObject;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index b6c1fe8..3db0a88 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -46,11 +46,6 @@
# include cap-ng.h
#endif
-#if WITH_NUMACTL
-# define NUMA_VERSION1_COMPATIBILITY 1
-# include numa.h
-#endif
-
#include virerror.h
#include virlog.h
#include virutil.h
@@ -409,113 +404,6 @@ cleanup:
return ret;
}
-#if WITH_NUMACTL
-static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl,
- virBitmapPtr nodemask)
-{
-nodemask_t mask;
-int mode = -1;
-int node = -1;
-int ret = -1;
-int i = 0;
-int maxnode = 0;
-bool warned = false;
-virDomainNumatuneDef numatune = ctrl-def-numatune;
-virBitmapPtr tmp_nodemask = NULL;
-
-if (numatune.memory.placement_mode ==
-VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
-if (!numatune.memory.nodemask)
-return 0;
-VIR_DEBUG(Set NUMA memory policy with specified nodeset);
-tmp_nodemask = numatune.memory.nodemask;
-} else if (numatune.memory.placement_mode ==
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
-VIR_DEBUG(Set NUMA memory policy with advisory nodeset from numad);
-tmp_nodemask = nodemask;
-} else {
-return 0;
-}
-
-VIR_DEBUG(Setting NUMA memory policy);
-
-if (numa_available() 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- %s, _(Host kernel is not aware of NUMA.));
-return -1;
-}
-
-maxnode = numa_max_node() + 1;
-
-/* Convert nodemask to NUMA bitmask. */
-nodemask_zero(mask);
-i = -1;
-while ((i = virBitmapNextSetBit(tmp_nodemask, i)) = 0) {
-if (i NUMA_NUM_NODES) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Host cannot support NUMA node %d), i);
-return -1;
-}
-if (i maxnode !warned) {
-VIR_WARN(nodeset is out of range, there is only %d NUMA
- nodes on host, maxnode);
-warned = true;
-}
-nodemask_set(mask, i);
-
Re: [libvirt] [PATCH 4/4] python: fix fd leak in generator.py
On 03/01/2013 06:26 AM, Eric Blake wrote: On 02/28/2013 03:03 AM, Guannan Ren wrote: --- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) At first, I was worried that this is a leak in the generated code. But it looks like it is only a leak in the generator, and that the generated code is safe. So it's not an essential bug fix, and more of a judgment call on whether to include in 1.0.3. At any rate, the fix appears obvious enough that I'm fine giving: ACK. Thanks for this review. I pushed these two ack'ed patches before sending out another bug-fixing patch about generator.py. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Make sure qemuProcessStart is run within a job
On Thu, Feb 28, 2013 at 14:29:59 -0700, Eric Blake wrote: On 02/28/2013 06:22 AM, Jiri Denemark wrote: qemuProcessStart expects to be run with a job already set and every caller except for qemuMigrationPrepareAny use it correctly. This bug can be observed in libvirtd logs during incoming migration as warning : qemuDomainObjEnterMonitorInternal:979 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous --- src/qemu/qemu_domain.c| 35 --- src/qemu/qemu_domain.h| 4 src/qemu/qemu_migration.c | 11 +++ 3 files changed, 35 insertions(+), 15 deletions(-) ACK. Qualifies as a bug fix for 1.0.3. Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
